From c1c9ed5dec5c4948c90549ce3f12b7308f62119b Mon Sep 17 00:00:00 2001 From: Jan Dittberner Date: Mon, 26 Sep 2022 11:58:36 +0200 Subject: [PATCH] Fix golangci-lint warnings --- .golangci.yml | 3 +- cmd/boardvoting/config.go | 4 +- cmd/boardvoting/handlers.go | 104 ++++++++++++++--------------- cmd/boardvoting/handlers_test.go | 4 ++ cmd/boardvoting/helpers.go | 34 +++++----- cmd/boardvoting/jobs.go | 1 - cmd/boardvoting/main.go | 11 +-- cmd/boardvoting/middleware.go | 9 ++- cmd/boardvoting/middleware_test.go | 11 +++ internal/forms/forms.go | 3 - internal/models/audit.go | 1 + internal/models/motions.go | 20 ++---- internal/models/users.go | 91 ++++++++++++++----------- internal/validator/validator.go | 10 +-- 14 files changed, 159 insertions(+), 147 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 05aa8eb..837e5e4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,5 +1,6 @@ --- run: + go: "1.17" skip-files: - boardvoting/assets.go @@ -34,7 +35,7 @@ linters-settings: goimports: local-prefixes: git.cacert.org/cacert-boardvoting misspell: - locale: en-AU + locale: "US" ignore-words: - CAcert diff --git a/cmd/boardvoting/config.go b/cmd/boardvoting/config.go index 203dbb6..90c6f16 100644 --- a/cmd/boardvoting/config.go +++ b/cmd/boardvoting/config.go @@ -19,7 +19,7 @@ package main import ( "fmt" - "io/ioutil" + "os" "time" "gopkg.in/yaml.v2" @@ -65,7 +65,7 @@ type Config struct { } func parseConfig(configFile string) (*Config, error) { - source, err := ioutil.ReadFile(configFile) + source, err := os.ReadFile(configFile) if err != nil { return nil, fmt.Errorf("could not read configuration file %s: %w", configFile, err) } diff --git a/cmd/boardvoting/handlers.go b/cmd/boardvoting/handlers.go index d4e20b9..ff3c05d 100644 --- a/cmd/boardvoting/handlers.go +++ b/cmd/boardvoting/handlers.go @@ -100,7 +100,7 @@ func (app *application) motionList(w http.ResponseWriter, r *http.Request) { panic(err) } - templateData := app.newTemplateData(r, "motions", "all-motions") + templateData := app.newTemplateData(r.Context(), r, "motions", "all-motions") if listOptions.UnvotedOnly { templateData.ActiveSubNav = subLevelNavMotionsUnvoted @@ -163,12 +163,12 @@ func (app *application) calculateMotionListOptions(r *http.Request) (*models.Mot } func (app *application) motionDetails(w http.ResponseWriter, r *http.Request) { - motion := app.motionFromRequestParam(w, r) + motion := app.motionFromRequestParam(r.Context(), w, r) if motion == nil { return } - data := app.newTemplateData(r, topLevelNavMotions, subLevelNavMotionsAll) + data := app.newTemplateData(r.Context(), r, topLevelNavMotions, subLevelNavMotionsAll) data.Motion = motion @@ -176,7 +176,7 @@ func (app *application) motionDetails(w http.ResponseWriter, r *http.Request) { } func (app *application) newMotionForm(w http.ResponseWriter, r *http.Request) { - data := app.newTemplateData(r, topLevelNavMotions, subLevelNavMotionsAll) + data := app.newTemplateData(r.Context(), r, topLevelNavMotions, subLevelNavMotionsAll) data.Form = &forms.EditMotionForm{ Type: models.VoteTypeMotion, } @@ -201,7 +201,7 @@ func (app *application) newMotionSubmit(w http.ResponseWriter, r *http.Request) } if !form.Valid() { - data := app.newTemplateData(r, topLevelNavMotions, subLevelNavMotionsAll) + data := app.newTemplateData(r.Context(), r, topLevelNavMotions, subLevelNavMotionsAll) data.Form = &form app.render(w, http.StatusUnprocessableEntity, "create_motion.html", data) @@ -246,7 +246,7 @@ func (app *application) newMotionSubmit(w http.ResponseWriter, r *http.Request) app.jobScheduler.Reschedule(JobIDCloseDecisions, JobIDRemindVoters) - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashSuccess, Title: "New motion started", Message: fmt.Sprintf("Started new motion %s: %s", decision.Tag, decision.Title), @@ -256,12 +256,12 @@ func (app *application) newMotionSubmit(w http.ResponseWriter, r *http.Request) } func (app *application) editMotionForm(w http.ResponseWriter, r *http.Request) { - motion := app.motionFromRequestParam(w, r) + motion := app.motionFromRequestParam(r.Context(), w, r) if motion == nil { return } - data := app.newTemplateData(r, topLevelNavMotions, subLevelNavMotionsAll) + data := app.newTemplateData(r.Context(), r, topLevelNavMotions, subLevelNavMotionsAll) data.Motion = motion @@ -275,7 +275,7 @@ func (app *application) editMotionForm(w http.ResponseWriter, r *http.Request) { } func (app *application) editMotionSubmit(w http.ResponseWriter, r *http.Request) { - motion := app.motionFromRequestParam(w, r) + motion := app.motionFromRequestParam(r.Context(), w, r) if motion == nil { return } @@ -294,7 +294,7 @@ func (app *application) editMotionSubmit(w http.ResponseWriter, r *http.Request) } if !form.Valid() { - data := app.newTemplateData(r, topLevelNavMotions, subLevelNavMotionsAll) + data := app.newTemplateData(r.Context(), r, topLevelNavMotions, subLevelNavMotionsAll) data.Form = &form app.render(w, http.StatusUnprocessableEntity, "edit_motion.html", data) @@ -340,7 +340,7 @@ func (app *application) editMotionSubmit(w http.ResponseWriter, r *http.Request) app.jobScheduler.Reschedule(JobIDCloseDecisions, JobIDRemindVoters) - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashInfo, Title: "Motion modified", Message: fmt.Sprintf("The motion %s has been modified!", decision.Tag), @@ -350,14 +350,14 @@ func (app *application) editMotionSubmit(w http.ResponseWriter, r *http.Request) } func (app *application) withdrawMotionForm(w http.ResponseWriter, r *http.Request) { - motion := app.motionFromRequestParam(w, r) + motion := app.motionFromRequestParam(r.Context(), w, r) if motion == nil { app.notFound(w) return } - data := app.newTemplateData(r, topLevelNavMotions, subLevelNavMotionsAll) + data := app.newTemplateData(r.Context(), r, topLevelNavMotions, subLevelNavMotionsAll) data.Motion = motion @@ -365,7 +365,7 @@ func (app *application) withdrawMotionForm(w http.ResponseWriter, r *http.Reques } func (app *application) withdrawMotionSubmit(w http.ResponseWriter, r *http.Request) { - motion := app.motionFromRequestParam(w, r) + motion := app.motionFromRequestParam(r.Context(), w, r) if motion == nil { app.notFound(w) @@ -389,7 +389,7 @@ func (app *application) withdrawMotionSubmit(w http.ResponseWriter, r *http.Requ app.jobScheduler.Reschedule(JobIDCloseDecisions, JobIDRemindVoters) - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashWarning, Title: "Motion withdrawn", Message: fmt.Sprintf("The motion %s has been withdrawn!", motion.Tag), @@ -399,7 +399,7 @@ func (app *application) withdrawMotionSubmit(w http.ResponseWriter, r *http.Requ } func (app *application) voteForm(w http.ResponseWriter, r *http.Request) { - motion := app.motionFromRequestParam(w, r) + motion := app.motionFromRequestParam(r.Context(), w, r) if motion == nil { app.notFound(w) @@ -413,7 +413,7 @@ func (app *application) voteForm(w http.ResponseWriter, r *http.Request) { return } - data := app.newTemplateData(r, topLevelNavMotions, subLevelNavMotionsAll) + data := app.newTemplateData(r.Context(), r, topLevelNavMotions, subLevelNavMotionsAll) data.Motion = motion @@ -425,7 +425,7 @@ func (app *application) voteForm(w http.ResponseWriter, r *http.Request) { } func (app *application) voteSubmit(w http.ResponseWriter, r *http.Request) { - motion := app.motionFromRequestParam(w, r) + motion := app.motionFromRequestParam(r.Context(), w, r) if motion == nil { return } @@ -459,7 +459,7 @@ func (app *application) voteSubmit(w http.ResponseWriter, r *http.Request) { Decision: motion, User: user, Choice: choice, }) - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashSuccess, Title: "Vote registered", Message: fmt.Sprintf("Your vote for motion %s has been registered.", motion.Tag), @@ -469,12 +469,12 @@ func (app *application) voteSubmit(w http.ResponseWriter, r *http.Request) { } func (app *application) proxyVoteForm(w http.ResponseWriter, r *http.Request) { - motion := app.motionFromRequestParam(w, r) + motion := app.motionFromRequestParam(r.Context(), w, r) if motion == nil { return } - data := app.newTemplateData(r, topLevelNavMotions, subLevelNavMotionsAll) + data := app.newTemplateData(r.Context(), r, topLevelNavMotions, subLevelNavMotionsAll) data.Motion = motion @@ -491,7 +491,7 @@ func (app *application) proxyVoteForm(w http.ResponseWriter, r *http.Request) { } func (app *application) proxyVoteSubmit(w http.ResponseWriter, r *http.Request) { - motion := app.motionFromRequestParam(w, r) + motion := app.motionFromRequestParam(r.Context(), w, r) if motion == nil { return } @@ -510,7 +510,7 @@ func (app *application) proxyVoteSubmit(w http.ResponseWriter, r *http.Request) } if !form.Valid() { - data := app.newTemplateData(r, topLevelNavMotions, subLevelNavMotionsAll) + data := app.newTemplateData(r.Context(), r, topLevelNavMotions, subLevelNavMotionsAll) data.Motion = motion potentialVoters, err := app.users.Voters(r.Context()) @@ -540,7 +540,7 @@ func (app *application) proxyVoteSubmit(w http.ResponseWriter, r *http.Request) panic(err) } - voter := app.getVoter(w, r, form.Voter.ID) + voter := app.getVoter(r.Context(), w, form.Voter.ID) if voter == nil { return } @@ -557,7 +557,7 @@ func (app *application) proxyVoteSubmit(w http.ResponseWriter, r *http.Request) Decision: motion, User: user, Voter: voter, Choice: form.Choice, Justification: form.Justification, }) - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashSuccess, Title: "Proxy vote registered", Message: fmt.Sprintf( @@ -571,7 +571,7 @@ func (app *application) proxyVoteSubmit(w http.ResponseWriter, r *http.Request) } func (app *application) userList(w http.ResponseWriter, r *http.Request) { - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) users, err := app.users.List( r.Context(), @@ -588,7 +588,7 @@ func (app *application) userList(w http.ResponseWriter, r *http.Request) { } func (app *application) editUserForm(w http.ResponseWriter, r *http.Request) { - userToEdit := app.userFromRequestParam(w, r, app.users.WithRoles(), app.users.WithEmailAddresses()) + userToEdit := app.userFromRequestParam(r.Context(), w, r, app.users.WithRoles(), app.users.WithEmailAddresses()) if userToEdit == nil { return } @@ -608,7 +608,7 @@ func (app *application) editUserForm(w http.ResponseWriter, r *http.Request) { roleNames[i] = roles[i].Name } - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) data.Form = &forms.EditUserForm{ User: userToEdit, @@ -623,7 +623,7 @@ func (app *application) editUserForm(w http.ResponseWriter, r *http.Request) { } func (app *application) editUserSubmit(w http.ResponseWriter, r *http.Request) { - userToEdit := app.userFromRequestParam(w, r, app.users.WithRoles(), app.users.WithEmailAddresses()) + userToEdit := app.userFromRequestParam(r.Context(), w, r, app.users.WithRoles(), app.users.WithEmailAddresses()) if userToEdit == nil { app.notFound(w) @@ -645,7 +645,7 @@ func (app *application) editUserSubmit(w http.ResponseWriter, r *http.Request) { panic(err) } - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) if !form.Valid() { roles, err := userToEdit.Roles() @@ -680,7 +680,7 @@ func (app *application) editUserSubmit(w http.ResponseWriter, r *http.Request) { panic(err) } - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashInfo, Title: "User modified", Message: fmt.Sprintf("User %s has been modified.", userToEdit.Name), @@ -690,7 +690,7 @@ func (app *application) editUserSubmit(w http.ResponseWriter, r *http.Request) { } func (app *application) userAddEmailForm(w http.ResponseWriter, r *http.Request) { - userToEdit := app.userFromRequestParam(w, r, app.users.WithEmailAddresses()) + userToEdit := app.userFromRequestParam(r.Context(), w, r, app.users.WithEmailAddresses()) if userToEdit == nil { app.notFound(w) @@ -702,7 +702,7 @@ func (app *application) userAddEmailForm(w http.ResponseWriter, r *http.Request) panic(err) } - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) data.Form = &forms.AddEmailForm{ User: userToEdit, @@ -713,7 +713,7 @@ func (app *application) userAddEmailForm(w http.ResponseWriter, r *http.Request) } func (app *application) userAddEmailSubmit(w http.ResponseWriter, r *http.Request) { - userToEdit := app.userFromRequestParam(w, r, app.users.WithEmailAddresses()) + userToEdit := app.userFromRequestParam(r.Context(), w, r, app.users.WithEmailAddresses()) if userToEdit == nil { app.notFound(w) @@ -734,7 +734,7 @@ func (app *application) userAddEmailSubmit(w http.ResponseWriter, r *http.Reques panic(err) } - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) if form.Valid() { emailExists, err := app.users.EmailExists(r.Context(), form.EmailAddress) @@ -770,7 +770,7 @@ func (app *application) userAddEmailSubmit(w http.ResponseWriter, r *http.Reques panic(err) } - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashSuccess, Title: "Email address added", Message: fmt.Sprintf( @@ -784,7 +784,7 @@ func (app *application) userAddEmailSubmit(w http.ResponseWriter, r *http.Reques } func (app *application) userDeleteEmailForm(w http.ResponseWriter, r *http.Request) { - userToEdit, emailAddress, err := app.deleteEmailParams(w, r) + userToEdit, emailAddress, err := app.deleteEmailParams(r.Context(), w, r) if err != nil { panic(err) } @@ -802,7 +802,7 @@ func (app *application) userDeleteEmailForm(w http.ResponseWriter, r *http.Reque return } - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) data.Form = &forms.DeleteEmailForm{ User: userToEdit, @@ -813,7 +813,7 @@ func (app *application) userDeleteEmailForm(w http.ResponseWriter, r *http.Reque } func (app *application) userDeleteEmailSubmit(w http.ResponseWriter, r *http.Request) { - userToEdit, emailAddress, err := app.deleteEmailParams(w, r) + userToEdit, emailAddress, err := app.deleteEmailParams(r.Context(), w, r) if err != nil { panic(err) } @@ -854,7 +854,7 @@ func (app *application) userDeleteEmailSubmit(w http.ResponseWriter, r *http.Req form.EmailAddress = emailAddress form.User = userToEdit - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) data.Form = &form @@ -869,7 +869,7 @@ func (app *application) userDeleteEmailSubmit(w http.ResponseWriter, r *http.Req panic(err) } - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashWarning, Title: "Email address deleted", Message: fmt.Sprintf( @@ -883,7 +883,7 @@ func (app *application) userDeleteEmailSubmit(w http.ResponseWriter, r *http.Req } func (app *application) newUserForm(w http.ResponseWriter, r *http.Request) { - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) data.Form = &forms.NewUserForm{} @@ -911,7 +911,7 @@ func (app *application) newUserSubmit(w http.ResponseWriter, r *http.Request) { } if !form.Valid() { - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) data.Form = &form @@ -935,7 +935,7 @@ func (app *application) newUserSubmit(w http.ResponseWriter, r *http.Request) { panic(err) } - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashSuccess, Title: "User created", Message: fmt.Sprintf("Created new user %s", form.Name), @@ -945,7 +945,7 @@ func (app *application) newUserSubmit(w http.ResponseWriter, r *http.Request) { } func (app *application) deleteUserForm(w http.ResponseWriter, r *http.Request) { - userToDelete := app.userFromRequestParam(w, r, app.users.CanDelete()) + userToDelete := app.userFromRequestParam(r.Context(), w, r, app.users.CanDelete()) if userToDelete == nil { return } @@ -956,7 +956,7 @@ func (app *application) deleteUserForm(w http.ResponseWriter, r *http.Request) { return } - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) data.Form = &forms.DeleteUserForm{ User: userToDelete, @@ -966,7 +966,7 @@ func (app *application) deleteUserForm(w http.ResponseWriter, r *http.Request) { } func (app *application) deleteUserSubmit(w http.ResponseWriter, r *http.Request) { - userToDelete := app.userFromRequestParam(w, r, app.users.CanDelete()) + userToDelete := app.userFromRequestParam(r.Context(), w, r, app.users.CanDelete()) if userToDelete == nil { return } @@ -992,7 +992,7 @@ func (app *application) deleteUserSubmit(w http.ResponseWriter, r *http.Request) panic(err) } - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavUsers) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavUsers) if !form.Valid() { data.Form = &form @@ -1008,7 +1008,7 @@ func (app *application) deleteUserSubmit(w http.ResponseWriter, r *http.Request) panic(err) } - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashWarning, Title: "User deleted", Message: fmt.Sprintf("User %s has been deleted.", userToDelete.Name), @@ -1018,7 +1018,7 @@ func (app *application) deleteUserSubmit(w http.ResponseWriter, r *http.Request) } func (app *application) chooseVotersForm(w http.ResponseWriter, r *http.Request) { - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavVoters) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavVoters) allUsers, err := app.users.List(r.Context(), app.users.WithRoles()) if err != nil { @@ -1069,7 +1069,7 @@ func (app *application) chooseVotersSubmit(w http.ResponseWriter, r *http.Reques } if !form.Valid() { - data := app.newTemplateData(r, topLevelNavUsers, subLevelNavVoters) + data := app.newTemplateData(r.Context(), r, topLevelNavUsers, subLevelNavVoters) allUsers, err := app.users.List(r.Context(), app.users.WithRoles()) if err != nil { @@ -1091,7 +1091,7 @@ func (app *application) chooseVotersSubmit(w http.ResponseWriter, r *http.Reques panic(err) } - app.addFlash(r, &FlashMessage{ + app.addFlash(r.Context(), &FlashMessage{ Variant: flashSuccess, Title: "Voters selected", Message: "A new list of voters has been selected", diff --git a/cmd/boardvoting/handlers_test.go b/cmd/boardvoting/handlers_test.go index d60cdb1..4278fa4 100644 --- a/cmd/boardvoting/handlers_test.go +++ b/cmd/boardvoting/handlers_test.go @@ -104,6 +104,8 @@ func TestApplication_healthCheck(t *testing.T) { rs := rr.Result() + defer func() { _ = rs.Body.Close() }() + assert.Equal(t, http.StatusOK, rs.StatusCode) }) @@ -133,6 +135,8 @@ func TestApplication_healthCheck(t *testing.T) { rs := rr.Result() + defer func() { _ = rs.Body.Close() }() + assert.Equal(t, http.StatusInternalServerError, rs.StatusCode) }) } diff --git a/cmd/boardvoting/helpers.go b/cmd/boardvoting/helpers.go index f832f80..ccebe7f 100644 --- a/cmd/boardvoting/helpers.go +++ b/cmd/boardvoting/helpers.go @@ -19,6 +19,7 @@ package main import ( "bytes" + "context" "crypto/x509" "encoding/pem" "errors" @@ -30,12 +31,12 @@ import ( "strconv" "strings" - "git.cacert.org/cacert-boardvoting/internal/forms" "github.com/Masterminds/sprig/v3" "github.com/go-chi/chi/v5" "github.com/go-playground/form/v4" "github.com/justinas/nosurf" + "git.cacert.org/cacert-boardvoting/internal/forms" "git.cacert.org/cacert-boardvoting/internal/models" "git.cacert.org/cacert-boardvoting/ui" ) @@ -126,6 +127,7 @@ type templateData struct { } func (app *application) newTemplateData( + ctx context.Context, r *http.Request, nav topLevelNavItem, subNav subLevelNavItem, @@ -137,7 +139,7 @@ func (app *application) newTemplateData( User: user, ActiveNav: nav, ActiveSubNav: subNav, - Flashes: app.flashes(r), + Flashes: app.flashes(ctx), CSRFToken: nosurf.Token(r), } } @@ -161,12 +163,13 @@ func (app *application) render(w http.ResponseWriter, status int, page string, d } func (app *application) motionFromRequestParam( + ctx context.Context, w http.ResponseWriter, r *http.Request, ) *models.Motion { withVotes := r.URL.Query().Has("showvotes") - motion, err := app.motions.ByTag(r.Context(), chi.URLParam(r, "tag"), withVotes) + motion, err := app.motions.ByTag(ctx, chi.URLParam(r, "tag"), withVotes) if err != nil { panic(err) } @@ -181,6 +184,7 @@ func (app *application) motionFromRequestParam( } func (app *application) userFromRequestParam( + ctx context.Context, w http.ResponseWriter, r *http.Request, options ...models.UserListOption, ) *models.User { userID, err := strconv.Atoi(chi.URLParam(r, "id")) @@ -190,7 +194,7 @@ func (app *application) userFromRequestParam( return nil } - user, err := app.users.ByID(r.Context(), int64(userID), options...) + user, err := app.users.ByID(ctx, int64(userID), options...) if err != nil { panic(err) } @@ -221,8 +225,12 @@ func (app *application) emailFromRequestParam(r *http.Request, user *models.User return "", nil } -func (app *application) deleteEmailParams(w http.ResponseWriter, r *http.Request) (*models.User, string, error) { - userToEdit := app.userFromRequestParam(w, r, app.users.WithEmailAddresses()) +func (app *application) deleteEmailParams( + ctx context.Context, + w http.ResponseWriter, + r *http.Request, +) (*models.User, string, error) { + userToEdit := app.userFromRequestParam(ctx, w, r, app.users.WithEmailAddresses()) if userToEdit == nil { return nil, "", nil } @@ -278,20 +286,16 @@ type FlashMessage struct { Message string } -func (app *application) addFlash(r *http.Request, message *FlashMessage) { - flashes := app.flashes(r) +func (app *application) addFlash(ctx context.Context, message *FlashMessage) { + flashes := app.flashes(ctx) flashes = append(flashes, *message) - app.sessionManager.Put( - r.Context(), - "flashes", - flashes, - ) + app.sessionManager.Put(ctx, "flashes", flashes) } -func (app *application) flashes(r *http.Request) []FlashMessage { - flashInstance := app.sessionManager.Pop(r.Context(), "flashes") +func (app *application) flashes(ctx context.Context) []FlashMessage { + flashInstance := app.sessionManager.Pop(ctx, "flashes") if flashInstance != nil { flashes, ok := flashInstance.([]FlashMessage) diff --git a/cmd/boardvoting/jobs.go b/cmd/boardvoting/jobs.go index 26461f8..52c3592 100644 --- a/cmd/boardvoting/jobs.go +++ b/cmd/boardvoting/jobs.go @@ -69,7 +69,6 @@ func (r *RemindVotersJob) Schedule() { potentialRun := time.Date(year, month, day, 0, 0, 0, 0, time.UTC) if potentialRun.Before(time.Now().UTC()) { - r.infoLog.Printf("potential reminder time %s is in the past, not scheduling ReminderJob", potentialRun) return diff --git a/cmd/boardvoting/main.go b/cmd/boardvoting/main.go index 8f67f10..73ec633 100644 --- a/cmd/boardvoting/main.go +++ b/cmd/boardvoting/main.go @@ -27,7 +27,6 @@ import ( "flag" "fmt" "html/template" - "io/ioutil" "log" "net/http" "net/url" @@ -212,8 +211,8 @@ func (app *application) startHTTPSServer(config *Config) error { return nil } -func (app *application) getVoter(w http.ResponseWriter, r *http.Request, voterID int64) *models.User { - voter, err := app.users.ByID(r.Context(), voterID, app.users.WithRoles()) +func (app *application) getVoter(ctx context.Context, w http.ResponseWriter, voterID int64) *models.User { + voter, err := app.users.ByID(ctx, voterID, app.users.WithRoles()) if err != nil { panic(err) } @@ -233,10 +232,6 @@ func (app *application) getVoter(w http.ResponseWriter, r *http.Request, voterID return voter } -type logEntry struct { - request *http.Request -} - func setupHTTPRedirect(config *Config, errChan chan error) { redirect := &http.Server{ Addr: config.HTTPAddress, @@ -269,7 +264,7 @@ func setupHTTPRedirect(config *Config, errChan chan error) { } func setupTLSConfig(config *Config) (*tls.Config, error) { - caCert, err := ioutil.ReadFile(config.ClientCACertificates) + caCert, err := os.ReadFile(config.ClientCACertificates) if err != nil { return nil, fmt.Errorf("could not read client certificate CAs %w", err) } diff --git a/cmd/boardvoting/middleware.go b/cmd/boardvoting/middleware.go index 7361785..0ecd204 100644 --- a/cmd/boardvoting/middleware.go +++ b/cmd/boardvoting/middleware.go @@ -50,7 +50,10 @@ func secureHeaders(next http.Handler) http.Handler { }) } -func (app *application) authenticateRequest(r *http.Request) (*models.User, *x509.Certificate, error) { +func (app *application) authenticateRequest( + ctx context.Context, + r *http.Request, +) (*models.User, *x509.Certificate, error) { if r.TLS == nil { return nil, nil, nil } @@ -78,7 +81,7 @@ func (app *application) authenticateRequest(r *http.Request) (*models.User, *x50 emails := clientCert.EmailAddresses - user, err := app.users.ByEmails(r.Context(), emails) + user, err := app.users.ByEmails(ctx, emails) if err != nil { return nil, nil, fmt.Errorf("could not get user information from database: %w", err) } @@ -88,7 +91,7 @@ func (app *application) authenticateRequest(r *http.Request) (*models.User, *x50 func (app *application) tryAuthenticate(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - user, cert, err := app.authenticateRequest(r) + user, cert, err := app.authenticateRequest(r.Context(), r) if err != nil { panic(err) } diff --git a/cmd/boardvoting/middleware_test.go b/cmd/boardvoting/middleware_test.go index ae83085..503251c 100644 --- a/cmd/boardvoting/middleware_test.go +++ b/cmd/boardvoting/middleware_test.go @@ -49,6 +49,8 @@ func Test_secureHeaders(t *testing.T) { rs := rr.Result() + defer func() { _ = rs.Body.Close() }() + assert.Equal(t, "default-src 'self'; font-src 'self' data:", rs.Header.Get("Content-Security-Policy")) assert.Equal(t, "origin-when-cross-origin", rs.Header.Get("Referrer-Policy")) assert.Equal(t, "nosniff", rs.Header.Get("X-Content-Type-Options")) @@ -103,6 +105,9 @@ func TestApplication_tryAuthenticate(t *testing.T) { app.tryAuthenticate(next).ServeHTTP(rr, r) rs := rr.Result() + + defer func() { _ = rs.Body.Close() }() + assert.Equal(t, http.StatusOK, rs.StatusCode) assert.Nil(t, nextCtx.Value(ctxUser)) }) @@ -118,6 +123,9 @@ func TestApplication_tryAuthenticate(t *testing.T) { app.tryAuthenticate(next).ServeHTTP(rr, r) rs := rr.Result() + + defer func() { _ = rs.Body.Close() }() + assert.Equal(t, http.StatusOK, rs.StatusCode) assert.Nil(t, nextCtx.Value(ctxUser)) }) @@ -136,6 +144,9 @@ func TestApplication_tryAuthenticate(t *testing.T) { app.tryAuthenticate(next).ServeHTTP(rr, r) rs := rr.Result() + + defer func() { _ = rs.Body.Close() }() + assert.Equal(t, http.StatusOK, rs.StatusCode) user := nextCtx.Value(ctxUser) diff --git a/internal/forms/forms.go b/internal/forms/forms.go index ca9204b..ae693ed 100644 --- a/internal/forms/forms.go +++ b/internal/forms/forms.go @@ -60,7 +60,6 @@ func validateReasoning(v *validator.Validator, reasoning string, field string) { field, fmt.Sprintf("This field must be at least %d characters long", minimumJustificationLen), ) - } func validateMotionTitle(v *validator.Validator, title string, field string) { @@ -79,7 +78,6 @@ func validateMotionTitle(v *validator.Validator, title string, field string) { field, fmt.Sprintf("This field must be at most %d characters long", maximumTitleLength), ) - } func validateMotionContent(v *validator.Validator, content string, field string) { @@ -98,7 +96,6 @@ func validateMotionContent(v *validator.Validator, content string, field string) field, fmt.Sprintf("This field must be at most %d characters long", maximumContentLength), ) - } type EditMotionForm struct { diff --git a/internal/models/audit.go b/internal/models/audit.go index 5fc311f..8216be0 100644 --- a/internal/models/audit.go +++ b/internal/models/audit.go @@ -89,6 +89,7 @@ func userChangeInfo(before, after *User) any { Addresses []string `json:"addresses"` Reminder string `json:"reminder"` } + details := struct { Before userInfo `json:"before"` After userInfo `json:"after"` diff --git a/internal/models/motions.go b/internal/models/motions.go index 0d269fc..1c606bf 100644 --- a/internal/models/motions.go +++ b/internal/models/motions.go @@ -318,9 +318,7 @@ WHERE decisions.status=0 AND :now > due`, struct{ Now time.Time }{Now: time.Now return nil, fmt.Errorf("fetching closable decisions failed: %w", err) } - defer func(rows *sqlx.Rows) { - _ = rows.Close() - }(rows) + defer func() { _ = rows.Close() }() decisions := make([]*Motion, 0) @@ -411,9 +409,7 @@ WHERE due < ? AND status=? AND NOT EXISTS(SELECT * FROM votes WHERE decision = d return nil, errCouldNotExecuteQuery(err) } - defer func(rows *sqlx.Rows) { - _ = rows.Close() - }(rows) + defer func() { _ = rows.Close() }() result := make([]*Motion, 0) @@ -444,9 +440,7 @@ func sumsForDecision(ctx context.Context, tx *sqlx.Tx, d *Motion) (*VoteSums, er return nil, fmt.Errorf("fetching vote sums for motion %s failed: %w", d.Tag, err) } - defer func(voteRows *sqlx.Rows) { - _ = voteRows.Close() - }(voteRows) + defer func() { _ = voteRows.Close() }() sums := &VoteSums{} @@ -636,9 +630,7 @@ GROUP BY v.decision, v.vote`, return errCouldNotExecuteQuery(err) } - defer func(rows *sql.Rows) { - _ = rows.Close() - }(rows) + defer func() { _ = rows.Close() }() for rows.Next() { if err = rows.Err(); err != nil { @@ -849,9 +841,7 @@ ORDER BY voters.name`, return fmt.Errorf("could not fetch rows: %w", err) } - defer func(rows *sqlx.Rows) { - _ = rows.Close() - }(rows) + defer func() { _ = rows.Close() }() for rows.Next() { if err := rows.Err(); err != nil { diff --git a/internal/models/users.go b/internal/models/users.go index 6288f31..3670ab9 100644 --- a/internal/models/users.go +++ b/internal/models/users.go @@ -181,9 +181,7 @@ WHERE ur.role = ? return nil, fmt.Errorf("could not run query: %w", err) } - defer func(rows *sqlx.Rows) { - _ = rows.Close() - }(rows) + defer func() { _ = rows.Close() }() result := make([]*User, 0) @@ -227,9 +225,7 @@ WHERE e.address IN (?)`, emails) return nil, fmt.Errorf("could not run query: %w", err) } - defer func(rows *sqlx.Rows) { - _ = rows.Close() - }(rows) + defer func() { _ = rows.Close() }() var ( user User @@ -268,9 +264,7 @@ func (m *UserModel) Roles(ctx context.Context, user *User) ([]*Role, error) { return nil, fmt.Errorf("could not query roles for %s: %w", user.Name, err) } - defer func(rows *sqlx.Rows) { - _ = rows.Close() - }(rows) + defer func() { _ = rows.Close() }() result := make([]*Role, 0) @@ -370,9 +364,7 @@ ORDER BY voters.name`, return nil, fmt.Errorf("could not execute query: %w", err) } - defer func(rows *sqlx.Rows) { - _ = rows.Close() - }(rows) + defer func() { _ = rows.Close() }() result := make([]*User, 0) @@ -435,9 +427,7 @@ func (m *UserModel) List(ctx context.Context, options ...UserListOption) ([]*Use return nil, errCouldNotExecuteQuery(err) } - defer func(rows *sqlx.Rows) { - _ = rows.Close() - }(rows) + defer func() { _ = rows.Close() }() users := make([]*User, 0) @@ -583,6 +573,7 @@ func userIDsFromUsers(users []*User) []int64 { for idx := range users { userIDs[idx] = users[idx].ID } + return userIDs } @@ -642,9 +633,7 @@ WHERE id IN (?) return errCouldNotExecuteQuery(err) } - defer func(rows *sqlx.Rows) { - _ = rows.Close() - }(rows) + defer func() { _ = rows.Close() }() for rows.Next() { if err := rows.Err(); err != nil { @@ -727,35 +716,41 @@ func (m *UserModel) EditUser(ctx context.Context, edit *User, admin *User, roles _ = tx.Rollback() }(tx) + err = m.updateChangedUser(ctx, edit, userBefore, tx) + if err != nil { + return err + } + + if err = AuditLog(ctx, tx, admin, AuditEditUser, reasoning, userChangeInfo(userBefore, edit)); err != nil { + return err + } + + if err = tx.Commit(); err != nil { + return errCouldNotCommitTransaction(err) + } + + return nil +} + +func (m *UserModel) updateChangedUser(ctx context.Context, edit, userBefore *User, tx *sqlx.Tx) error { if edit.Name != userBefore.Name { - if _, err = tx.NamedExecContext(ctx, `UPDATE voters SET name=:name WHERE id=:id`, edit); err != nil { + if _, err := tx.NamedExecContext(ctx, `UPDATE voters SET name=:name WHERE id=:id`, edit); err != nil { return errCouldNotExecuteQuery(err) } } if !userBefore.rolesEqual(edit) { - if err = updateRoles(ctx, tx, edit); err != nil { + if err := updateRoles(ctx, tx, edit); err != nil { return err } } if !userBefore.reminderEqual(edit) { - if err = updateReminder(ctx, tx, edit); err != nil { + if err := updateReminder(ctx, tx, edit); err != nil { return err } } - if err = AuditLog( - ctx, tx, admin, AuditEditUser, reasoning, - userChangeInfo(userBefore, edit), - ); err != nil { - return err - } - - if err = tx.Commit(); err != nil { - return errCouldNotCommitTransaction(err) - } - return nil } @@ -775,7 +770,7 @@ func (m *UserModel) EmailExists(ctx context.Context, address string) (bool, erro return exists, nil } -func (m *UserModel) AddEmail(ctx context.Context, user *User, admin *User, emailAddress string, reasoning string) error { +func (m *UserModel) AddEmail(ctx context.Context, user, admin *User, emailAddress, reasoning string) error { userBefore, err := m.ByID(ctx, user.ID, m.WithEmailAddresses()) if err != nil { return err @@ -824,7 +819,12 @@ func (m *UserModel) DeleteEmail(ctx context.Context, user, admin *User, emailAdd _ = tx.Rollback() }(tx) - if _, err := tx.ExecContext(ctx, `DELETE FROM emails WHERE reminder=FALSE AND address=? AND voter=?`, emailAddress, user.ID); err != nil { + if _, err := tx.ExecContext( + ctx, + `DELETE FROM emails WHERE reminder=FALSE AND address=? AND voter=?`, + emailAddress, + user.ID, + ); err != nil { return errCouldNotExecuteQuery(err) } @@ -833,7 +833,7 @@ func (m *UserModel) DeleteEmail(ctx context.Context, user, admin *User, emailAdd return errCouldNotExecuteQuery(err) } - defer rows.Close() + defer func() { _ = rows.Close() }() emailAddresses := make([]string, 0, len(userBefore.emailAddresses)-1) @@ -934,7 +934,9 @@ func (m *UserModel) ChooseVoters(ctx context.Context, admin *User, voterIDs []in return errCouldNotStartTransaction(err) } - defer tx.Rollback() + defer func(tx *sqlx.Tx) { + _ = tx.Rollback() + }(tx) votersBefore, err := allVoterNames(ctx, tx) if err != nil { @@ -971,7 +973,14 @@ WHERE id IN (?) AND NOT EXISTS(SELECT * FROM user_roles WHERE role = ? AND voter return err } - if err := AuditLog(ctx, tx, admin, AuditChangeVoters, reasoning, voterChangeInfo(votersBefore, votersAfter)); err != nil { + if err := AuditLog( + ctx, + tx, + admin, + AuditChangeVoters, + reasoning, + voterChangeInfo(votersBefore, votersAfter), + ); err != nil { return err } @@ -983,12 +992,16 @@ WHERE id IN (?) AND NOT EXISTS(SELECT * FROM user_roles WHERE role = ? AND voter } func allVoterNames(ctx context.Context, tx *sqlx.Tx) ([]string, error) { - rows, err := tx.QueryxContext(ctx, `SELECT name FROM voters JOIN user_roles ur ON voters.id = ur.voter_id WHERE role=? ORDER BY name`, RoleVoter) + rows, err := tx.QueryxContext(ctx, `SELECT name +FROM voters + JOIN user_roles ur ON voters.id = ur.voter_id +WHERE role = ? +ORDER BY name`, RoleVoter) if err != nil { return nil, errCouldNotExecuteQuery(err) } - defer rows.Close() + defer func() { _ = rows.Close() }() names := make([]string, 0) diff --git a/internal/validator/validator.go b/internal/validator/validator.go index d9d054d..83fe252 100644 --- a/internal/validator/validator.go +++ b/internal/validator/validator.go @@ -21,21 +21,14 @@ import ( "net" "net/mail" "reflect" - "regexp" "strings" "unicode/utf8" ) type Validator struct { FieldErrors map[string]string - rxEmail *regexp.Regexp } -var rxEmail = regexp.MustCompile( - "^[a-zA-Z0-9.!#$%&'*+\\\\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?" + - "(?:\\\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$", -) - func (v *Validator) Valid() bool { return len(v.FieldErrors) == 0 } @@ -95,7 +88,7 @@ func PermittedString(value string, permittedValues ...string) bool { } func PermittedStringSet(value []string, permittedValues []string) bool { - if value == nil || len(value) == 0 { + if len(value) == 0 { return true } @@ -120,6 +113,7 @@ func IsEmail(value string) bool { } parts := strings.SplitN(addr.Address, "@", 2) + mxs, err := net.LookupMX(parts[1]) if err != nil || len(mxs) < 1 { return false