Skip to content

Commit f210558

Browse files
sumnerevansclaude
andcommitted
auth: replace copy-paste JWT checks with real middleware
Add a shared parseTokenByIssuer helper and VolunteerAuthMiddleware, consolidate all protected admin routes into a single subrouter wrapped with AdminAuthMiddleware, and remove the duplicated cookie+token checks that were scattered across handler and template functions. Closes #73 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d3ccbc8 commit f210558

4 files changed

Lines changed: 87 additions & 135 deletions

File tree

internal/admin.go

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package internal
33
import (
44
"context"
55
"encoding/csv"
6-
"errors"
76
"fmt"
87
"net/http"
98
"time"
@@ -24,47 +23,7 @@ func (a *Application) createAdminLoginJWT(email string) *jwt.Token {
2423
})
2524
}
2625

27-
func (a *Application) isAdminByToken(tokenStr string) (bool, error) {
28-
if tokenStr == "" {
29-
return false, errors.New("no token")
30-
}
31-
32-
token, err := jwt.ParseWithClaims(tokenStr, &jwt.RegisteredClaims{}, func(token *jwt.Token) (any, error) {
33-
// Don't forget to validate the alg is what you expect:
34-
if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
35-
return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
36-
}
37-
38-
return a.Config.ReadSecretKey(), nil
39-
})
40-
if err != nil {
41-
return false, fmt.Errorf("failed to parse admin token: %w", err)
42-
}
43-
44-
claims, ok := token.Claims.(*jwt.RegisteredClaims)
45-
if !token.Valid || !ok {
46-
return false, fmt.Errorf("failed to validate token: %w", err)
47-
}
48-
49-
if claims.Issuer != string(IssuerAdminLogin) {
50-
return false, fmt.Errorf("invalid student verify token: %w", err)
51-
}
52-
53-
return true, nil
54-
}
55-
5626
func (a *Application) GetAdminTeamsTemplate(r *http.Request) map[string]any {
57-
tok, err := r.Cookie("admin_token")
58-
if err != nil {
59-
a.Log.Warn().Err(err).Msg("failed to get admin token")
60-
return nil
61-
}
62-
63-
if isAdmin, err := a.isAdminByToken(tok.Value); err != nil || !isAdmin {
64-
a.Log.Warn().Err(err).Msg("user is not admin!")
65-
return nil
66-
}
67-
6827
teamsWithTeachers, err := a.DB.GetAdminTeamsWithTeacherName(r.Context())
6928
if err != nil {
7029
a.Log.Err(err).Msg("failed to get teams")
@@ -153,17 +112,6 @@ func (a *Application) GetAdminTeamsTemplate(r *http.Request) map[string]any {
153112
}
154113

155114
func (a *Application) GetAdminDietaryRestrictionsTemplate(r *http.Request) map[string]any {
156-
tok, err := r.Cookie("admin_token")
157-
if err != nil {
158-
a.Log.Warn().Err(err).Msg("failed to get admin token")
159-
return nil
160-
}
161-
162-
if isAdmin, err := a.isAdminByToken(tok.Value); err != nil || !isAdmin {
163-
a.Log.Warn().Err(err).Msg("user is not admin!")
164-
return nil
165-
}
166-
167115
dietaryRestrictions, err := a.DB.GetAllDietaryRestrictions(r.Context())
168116
if err != nil {
169117
a.Log.Err(err).Msg("failed to get dietary restrictions")
@@ -179,7 +127,7 @@ func (a *Application) HandleAdminEmailLogin(w http.ResponseWriter, r *http.Reque
179127
tok := r.URL.Query().Get("tok")
180128
log := zerolog.Ctx(r.Context())
181129
log.Info().Str("token", tok).Msg("got token")
182-
isAdmin, err := a.isAdminByToken(tok)
130+
isAdmin, err := a.parseTokenByIssuer(tok, IssuerAdminLogin)
183131
if err != nil || !isAdmin {
184132
log.Warn().Msg("failed to get admin")
185133
w.WriteHeader(http.StatusBadRequest)

internal/application.go

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package internal
33
import (
44
"fmt"
55
"html/template"
6+
"maps"
67
"net/http"
78
"regexp"
89
"strings"
@@ -66,9 +67,7 @@ func (a *Application) ServeTemplateExtra(logger *zerolog.Logger, templateName st
6667
if data == nil {
6768
data = map[string]any{}
6869
}
69-
for k, v := range extraData {
70-
data[k] = v
71-
}
70+
maps.Copy(data, extraData)
7271
user, err := a.GetLoggedInTeacher(r)
7372
if err == nil {
7473
data["Username"] = user.Name
@@ -217,36 +216,43 @@ func (a *Application) Start() {
217216
router.HandleFunc("POST "+path, renderFn(fn))
218217
}
219218

220-
// Admin pages
221-
router.HandleFunc("GET /admin", a.ServeTemplate(a.Log, "adminhome.html", noArgs))
219+
// Admin pages (unprotected — these exact patterns beat the /admin/ prefix below)
222220
router.HandleFunc("GET /admin/login", a.ServeTemplate(a.Log, "adminlogin.html", noArgs))
223-
router.HandleFunc("GET /admin/dietaryrestrictions", a.ServeTemplate(a.Log, "admindietaryrestrictions.html", a.GetAdminDietaryRestrictionsTemplate))
224-
router.HandleFunc("GET /admin/teams", a.ServeTemplate(a.Log, "adminteams.html", a.GetAdminTeamsTemplate))
225221
router.HandleFunc("GET /admin/emaillogin", a.HandleAdminEmailLogin)
226222
router.HandleFunc("POST /admin/emaillogin", a.HandleAdminLogin)
227223

228-
adminAPIRouter := http.NewServeMux()
229-
adminAPIRouter.HandleFunc("GET /resendstudentemail", a.HandleResendStudentEmail)
230-
adminAPIRouter.HandleFunc("GET /resendparentemail", a.HandleResendParentEmail)
231-
adminAPIRouter.HandleFunc("GET /confirmationlink/student", a.HandleGetStudentEmailConfirmationLink)
232-
adminAPIRouter.HandleFunc("GET /confirmationlink/parent", a.HandleGetParentEmailConfirmationLink)
233-
adminAPIRouter.HandleFunc("GET /sendemailconfirmationreminders", a.HandleSendEmailConfirmationReminders)
234-
adminAPIRouter.HandleFunc("GET /sendparentreminders", a.HandleSendParentReminders)
235-
adminAPIRouter.HandleFunc("GET /sendqrcodes", a.HandleSendQRCodes)
236-
adminAPIRouter.HandleFunc("GET /kattis/teams", a.HandleKattisTeamsExport)
237-
adminAPIRouter.HandleFunc("GET /kattis/participants", a.HandleKattisParticipantsExport)
238-
adminAPIRouter.HandleFunc("GET /zoom/breakout", a.HandleZoomBreakoutExport)
239-
adminAPIRouter.HandleFunc("GET /manualcheckin", a.HandleManualCheckin)
240-
adminAPIRouter.HandleFunc("GET /team-list", a.HandleTeamList)
241-
router.Handle("/admin/api/", http.StripPrefix("/admin/api", a.AdminAuthMiddleware(adminAPIRouter)))
242-
243-
// Volunteer pages
224+
// Admin pages (protected) — subrouter consolidates all protected admin routes
225+
adminRouter := http.NewServeMux()
226+
adminRouter.HandleFunc("GET /{$}", a.ServeTemplate(a.Log, "adminhome.html", noArgs))
227+
adminRouter.HandleFunc("GET /dietaryrestrictions", a.ServeTemplate(a.Log, "admindietaryrestrictions.html", a.GetAdminDietaryRestrictionsTemplate))
228+
adminRouter.HandleFunc("GET /teams", a.ServeTemplate(a.Log, "adminteams.html", a.GetAdminTeamsTemplate))
229+
adminRouter.HandleFunc("GET /api/resendstudentemail", a.HandleResendStudentEmail)
230+
adminRouter.HandleFunc("GET /api/resendparentemail", a.HandleResendParentEmail)
231+
adminRouter.HandleFunc("GET /api/confirmationlink/student", a.HandleGetStudentEmailConfirmationLink)
232+
adminRouter.HandleFunc("GET /api/confirmationlink/parent", a.HandleGetParentEmailConfirmationLink)
233+
adminRouter.HandleFunc("GET /api/sendemailconfirmationreminders", a.HandleSendEmailConfirmationReminders)
234+
adminRouter.HandleFunc("GET /api/sendparentreminders", a.HandleSendParentReminders)
235+
adminRouter.HandleFunc("GET /api/sendqrcodes", a.HandleSendQRCodes)
236+
adminRouter.HandleFunc("GET /api/kattis/teams", a.HandleKattisTeamsExport)
237+
adminRouter.HandleFunc("GET /api/kattis/participants", a.HandleKattisParticipantsExport)
238+
adminRouter.HandleFunc("GET /api/zoom/breakout", a.HandleZoomBreakoutExport)
239+
adminRouter.HandleFunc("GET /api/manualcheckin", a.HandleManualCheckin)
240+
adminRouter.HandleFunc("GET /api/team-list", a.HandleTeamList)
241+
router.Handle("/admin/", http.StripPrefix("/admin", a.AdminAuthMiddleware(adminRouter)))
242+
router.Handle("GET /admin", a.AdminAuthMiddleware(
243+
http.HandlerFunc(a.ServeTemplate(a.Log, "adminhome.html", noArgs))))
244+
245+
// Volunteer pages (unprotected)
244246
router.HandleFunc("GET /volunteer", a.ServeTemplate(a.Log, "volunteerhome.html", noArgs))
245247
router.HandleFunc("GET /volunteer/login", a.ServeTemplate(a.Log, "volunteerlogin.html", noArgs))
246248
router.HandleFunc("GET /volunteer/emaillogin", a.HandleVolunteerEmailLogin)
247249
router.HandleFunc("POST /volunteer/emaillogin", a.HandleVolunteerLogin)
248-
router.HandleFunc("GET /volunteer/scan", a.ServeTemplate(a.Log, "volunteerscan.html", a.GetVolunteerScanTemplate))
249-
router.HandleFunc("GET /volunteer/checkin", a.HandleVolunteerCheckIn)
250+
251+
// Volunteer pages (protected)
252+
router.Handle("GET /volunteer/scan", a.VolunteerAuthMiddleware(
253+
http.HandlerFunc(a.ServeTemplate(a.Log, "volunteerscan.html", a.GetVolunteerScanTemplate))))
254+
router.Handle("GET /volunteer/checkin", a.VolunteerAuthMiddleware(
255+
http.HandlerFunc(a.HandleVolunteerCheckIn)))
250256

251257
var handler http.Handler = router
252258
handler = hlog.RequestIDHandler("request_id", "RequestID")(handler)

internal/middleware.go

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,66 @@
11
package internal
22

3-
import "net/http"
3+
import (
4+
"fmt"
5+
"net/http"
6+
7+
"github.com/golang-jwt/jwt/v4"
8+
)
9+
10+
func (a *Application) parseTokenByIssuer(tokenStr string, issuer Issuer) (bool, error) {
11+
if tokenStr == "" {
12+
return false, fmt.Errorf("no token")
13+
}
14+
15+
token, err := jwt.ParseWithClaims(tokenStr, &jwt.RegisteredClaims{}, func(token *jwt.Token) (any, error) {
16+
if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
17+
return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
18+
}
19+
return a.Config.ReadSecretKey(), nil
20+
})
21+
if err != nil {
22+
return false, fmt.Errorf("failed to parse token: %w", err)
23+
}
24+
25+
claims, ok := token.Claims.(*jwt.RegisteredClaims)
26+
if !token.Valid || !ok {
27+
return false, fmt.Errorf("invalid token")
28+
}
29+
30+
if claims.Issuer != string(issuer) {
31+
return false, fmt.Errorf("wrong issuer: got %s, want %s", claims.Issuer, issuer)
32+
}
33+
34+
return true, nil
35+
}
436

537
func (a *Application) AdminAuthMiddleware(next http.Handler) http.Handler {
638
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
739
tok, err := r.Cookie("admin_token")
840
if err != nil {
9-
http.Redirect(w, r, "/", http.StatusSeeOther)
41+
http.Redirect(w, r, "/admin/login", http.StatusSeeOther)
42+
return
43+
}
44+
45+
if isAdmin, err := a.parseTokenByIssuer(tok.Value, IssuerAdminLogin); err != nil || !isAdmin {
46+
http.Redirect(w, r, "/admin/login", http.StatusSeeOther)
47+
return
48+
}
49+
50+
next.ServeHTTP(w, r)
51+
})
52+
}
53+
54+
func (a *Application) VolunteerAuthMiddleware(next http.Handler) http.Handler {
55+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
56+
tok, err := r.Cookie("volunteer_token")
57+
if err != nil {
58+
http.Redirect(w, r, "/volunteer/login", http.StatusSeeOther)
1059
return
1160
}
1261

13-
if isAdmin, err := a.isAdminByToken(tok.Value); err != nil || !isAdmin {
14-
http.Redirect(w, r, "/", http.StatusSeeOther)
62+
if isVolunteer, err := a.parseTokenByIssuer(tok.Value, IssuerVolunteerLogin); err != nil || !isVolunteer {
63+
http.Redirect(w, r, "/volunteer/login", http.StatusSeeOther)
1564
return
1665
}
1766

internal/volunteer.go

Lines changed: 2 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -22,39 +22,10 @@ func (a *Application) createVolunteerLoginJWT(email string) *jwt.Token {
2222
})
2323
}
2424

25-
func (a *Application) isVolunteerByToken(tokenStr string) (bool, error) {
26-
if tokenStr == "" {
27-
return false, errors.New("no token")
28-
}
29-
30-
token, err := jwt.ParseWithClaims(tokenStr, &jwt.RegisteredClaims{}, func(token *jwt.Token) (any, error) {
31-
// Don't forget to validate the alg is what you expect:
32-
if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
33-
return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
34-
}
35-
36-
return a.Config.ReadSecretKey(), nil
37-
})
38-
if err != nil {
39-
return false, fmt.Errorf("failed to parse admin token: %w", err)
40-
}
41-
42-
claims, ok := token.Claims.(*jwt.RegisteredClaims)
43-
if !token.Valid || !ok {
44-
return false, fmt.Errorf("failed to validate token: %w", err)
45-
}
46-
47-
if claims.Issuer != string(IssuerVolunteerLogin) {
48-
return false, fmt.Errorf("invalid student verify token: %w", err)
49-
}
50-
51-
return true, nil
52-
}
53-
5425
func (a *Application) HandleVolunteerEmailLogin(w http.ResponseWriter, r *http.Request) {
5526
tok := r.URL.Query().Get("tok")
5627
zerolog.Ctx(r.Context()).Info().Str("token", tok).Msg("got token")
57-
isVolunteer, err := a.isVolunteerByToken(tok)
28+
isVolunteer, err := a.parseTokenByIssuer(tok, IssuerVolunteerLogin)
5829
if err != nil || !isVolunteer {
5930
a.Log.Warn().Msg("failed to get volunteer")
6031
w.WriteHeader(http.StatusBadRequest)
@@ -121,23 +92,13 @@ func (a *Application) HandleVolunteerLogin(w http.ResponseWriter, r *http.Reques
12192

12293
func (a *Application) GetVolunteerScanTemplate(r *http.Request) map[string]any {
12394
ctx := r.Context()
124-
tok, err := r.Cookie("volunteer_token")
125-
if err != nil {
126-
a.Log.Warn().Err(err).Msg("failed to get volunteer token")
127-
return nil
128-
}
129-
130-
if isVolunteer, err := a.isVolunteerByToken(tok.Value); err != nil || !isVolunteer {
131-
a.Log.Warn().Err(err).Msg("user is not volunteer!")
132-
return nil
133-
}
13495

13596
res := map[string]any{
13697
"LoggedInAsVolunteer": true,
13798
}
13899

139100
if adminTok, err := r.Cookie("admin_token"); err == nil {
140-
if isAdmin, err := a.isAdminByToken(adminTok.Value); err == nil && isAdmin {
101+
if isAdmin, err := a.parseTokenByIssuer(adminTok.Value, IssuerAdminLogin); err == nil && isAdmin {
141102
res["LoggedInAsAdmin"] = true
142103
}
143104
}
@@ -174,18 +135,6 @@ func (a *Application) GetVolunteerScanTemplate(r *http.Request) map[string]any {
174135

175136
func (a *Application) HandleVolunteerCheckIn(w http.ResponseWriter, r *http.Request) {
176137
ctx := r.Context()
177-
tok, err := r.Cookie("volunteer_token")
178-
if err != nil {
179-
a.Log.Warn().Err(err).Msg("failed to get volunteer token")
180-
w.WriteHeader(http.StatusBadRequest)
181-
return
182-
}
183-
184-
if isVolunteer, err := a.isVolunteerByToken(tok.Value); err != nil || !isVolunteer {
185-
a.Log.Warn().Err(err).Msg("user is not volunteer!")
186-
w.WriteHeader(http.StatusBadRequest)
187-
return
188-
}
189138

190139
studentSignInToken := r.URL.Query().Get("tok")
191140
student, err := a.getStudentByQRToken(ctx, studentSignInToken)

0 commit comments

Comments
 (0)