diff --git a/internal/audit/detail.go b/internal/audit/detail.go new file mode 100644 index 0000000..93d23bc --- /dev/null +++ b/internal/audit/detail.go @@ -0,0 +1,33 @@ +// Package audit provides helpers for constructing audit log detail strings. +package audit + +import "encoding/json" + +// JSON builds a JSON details string from key-value pairs for audit logging. +// Uses json.Marshal for safe encoding rather than fmt.Sprintf with %q, +// which is fragile for edge-case Unicode. +func JSON(pairs ...string) string { + if len(pairs)%2 != 0 { + return "{}" + } + m := make(map[string]string, len(pairs)/2) + for i := 0; i < len(pairs); i += 2 { + m[pairs[i]] = pairs[i+1] + } + b, err := json.Marshal(m) + if err != nil { + return "{}" + } + return string(b) +} + +// JSONWithRoles builds a JSON details string that includes a "roles" key +// mapped to a string slice. This produces a proper JSON array for the value. +func JSONWithRoles(roles []string) string { + m := map[string][]string{"roles": roles} + b, err := json.Marshal(m) + if err != nil { + return "{}" + } + return string(b) +} diff --git a/internal/audit/detail_test.go b/internal/audit/detail_test.go new file mode 100644 index 0000000..3afa5fd --- /dev/null +++ b/internal/audit/detail_test.go @@ -0,0 +1,163 @@ +package audit + +import ( + "encoding/json" + "testing" +) + +func TestJSON(t *testing.T) { + tests := []struct { + name string + pairs []string + verify func(t *testing.T, result string) + }{ + { + name: "single pair", + pairs: []string{"username", "alice"}, + verify: func(t *testing.T, result string) { + var m map[string]string + if err := json.Unmarshal([]byte(result), &m); err != nil { + t.Fatalf("invalid JSON: %v", err) + } + if m["username"] != "alice" { + t.Fatalf("expected alice, got %s", m["username"]) + } + }, + }, + { + name: "multiple pairs", + pairs: []string{"jti", "abc-123", "reason", "logout"}, + verify: func(t *testing.T, result string) { + var m map[string]string + if err := json.Unmarshal([]byte(result), &m); err != nil { + t.Fatalf("invalid JSON: %v", err) + } + if m["jti"] != "abc-123" { + t.Fatalf("expected abc-123, got %s", m["jti"]) + } + if m["reason"] != "logout" { + t.Fatalf("expected logout, got %s", m["reason"]) + } + }, + }, + { + name: "special characters in values", + pairs: []string{"username", "user\"with\\quotes"}, + verify: func(t *testing.T, result string) { + var m map[string]string + if err := json.Unmarshal([]byte(result), &m); err != nil { + t.Fatalf("invalid JSON for special chars: %v", err) + } + if m["username"] != "user\"with\\quotes" { + t.Fatalf("unexpected value: %s", m["username"]) + } + }, + }, + { + name: "unicode edge cases", + pairs: []string{"username", "user\u2028line\u2029sep"}, + verify: func(t *testing.T, result string) { + var m map[string]string + if err := json.Unmarshal([]byte(result), &m); err != nil { + t.Fatalf("invalid JSON for unicode: %v", err) + } + if m["username"] != "user\u2028line\u2029sep" { + t.Fatalf("unexpected value: %s", m["username"]) + } + }, + }, + { + name: "null bytes in value", + pairs: []string{"data", "before\x00after"}, + verify: func(t *testing.T, result string) { + var m map[string]string + if err := json.Unmarshal([]byte(result), &m); err != nil { + t.Fatalf("invalid JSON for null bytes: %v", err) + } + if m["data"] != "before\x00after" { + t.Fatalf("unexpected value: %q", m["data"]) + } + }, + }, + { + name: "odd number of args returns empty object", + pairs: []string{"key"}, + verify: func(t *testing.T, result string) { + if result != "{}" { + t.Fatalf("expected {}, got %s", result) + } + }, + }, + { + name: "no args returns empty object", + pairs: nil, + verify: func(t *testing.T, result string) { + if result != "{}" { + t.Fatalf("expected {}, got %s", result) + } + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := JSON(tc.pairs...) + tc.verify(t, result) + }) + } +} + +func TestJSONWithRoles(t *testing.T) { + tests := []struct { + name string + roles []string + verify func(t *testing.T, result string) + }{ + { + name: "multiple roles", + roles: []string{"admin", "editor"}, + verify: func(t *testing.T, result string) { + var m map[string][]string + if err := json.Unmarshal([]byte(result), &m); err != nil { + t.Fatalf("invalid JSON: %v", err) + } + if len(m["roles"]) != 2 || m["roles"][0] != "admin" || m["roles"][1] != "editor" { + t.Fatalf("unexpected roles: %v", m["roles"]) + } + }, + }, + { + name: "empty roles", + roles: []string{}, + verify: func(t *testing.T, result string) { + var m map[string][]string + if err := json.Unmarshal([]byte(result), &m); err != nil { + t.Fatalf("invalid JSON: %v", err) + } + if len(m["roles"]) != 0 { + t.Fatalf("expected empty roles, got %v", m["roles"]) + } + }, + }, + { + name: "roles with special characters", + roles: []string{"role\"special"}, + verify: func(t *testing.T, result string) { + var m map[string][]string + if err := json.Unmarshal([]byte(result), &m); err != nil { + t.Fatalf("invalid JSON: %v", err) + } + if m["roles"][0] != "role\"special" { + t.Fatalf("unexpected role: %s", m["roles"][0]) + } + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := JSONWithRoles(tc.roles) + tc.verify(t, result) + }) + } +} diff --git a/internal/grpcserver/auth.go b/internal/grpcserver/auth.go index 64a7cf7..374c364 100644 --- a/internal/grpcserver/auth.go +++ b/internal/grpcserver/auth.go @@ -13,6 +13,7 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" mciasv1 "git.wntrmute.dev/kyle/mcias/gen/mcias/v1" + "git.wntrmute.dev/kyle/mcias/internal/audit" "git.wntrmute.dev/kyle/mcias/internal/auth" "git.wntrmute.dev/kyle/mcias/internal/crypto" "git.wntrmute.dev/kyle/mcias/internal/model" @@ -42,7 +43,7 @@ func (a *authServiceServer) Login(ctx context.Context, req *mciasv1.LoginRequest // Security: run dummy Argon2 to equalise timing for unknown users. _, _ = auth.VerifyPassword("dummy", auth.DummyHash()) a.s.db.WriteAuditEvent(model.EventLoginFail, nil, nil, ip, //nolint:errcheck // audit failure is non-fatal - fmt.Sprintf(`{"username":%q,"reason":"unknown_user"}`, req.Username)) + audit.JSON("username", req.Username, "reason", "unknown_user")) return nil, status.Error(codes.Unauthenticated, "invalid credentials") } @@ -129,7 +130,7 @@ func (a *authServiceServer) Login(ctx context.Context, req *mciasv1.LoginRequest a.s.db.WriteAuditEvent(model.EventLoginOK, &acct.ID, nil, ip, "") //nolint:errcheck a.s.db.WriteAuditEvent(model.EventTokenIssued, &acct.ID, nil, ip, //nolint:errcheck - fmt.Sprintf(`{"jti":%q}`, claims.JTI)) + audit.JSON("jti", claims.JTI)) return &mciasv1.LoginResponse{ Token: tokenStr, @@ -145,7 +146,7 @@ func (a *authServiceServer) Logout(ctx context.Context, _ *mciasv1.LogoutRequest return nil, status.Error(codes.Internal, "internal error") } a.s.db.WriteAuditEvent(model.EventTokenRevoked, nil, nil, peerIP(ctx), //nolint:errcheck - fmt.Sprintf(`{"jti":%q,"reason":"logout"}`, claims.JTI)) + audit.JSON("jti", claims.JTI, "reason", "logout")) return &mciasv1.LogoutResponse{}, nil } @@ -186,7 +187,7 @@ func (a *authServiceServer) RenewToken(ctx context.Context, _ *mciasv1.RenewToke } a.s.db.WriteAuditEvent(model.EventTokenRenewed, &acct.ID, nil, peerIP(ctx), //nolint:errcheck - fmt.Sprintf(`{"old_jti":%q,"new_jti":%q}`, claims.JTI, newClaims.JTI)) + audit.JSON("old_jti", claims.JTI, "new_jti", newClaims.JTI)) return &mciasv1.RenewTokenResponse{ Token: newTokenStr, diff --git a/internal/server/server.go b/internal/server/server.go index aa135a1..71155ee 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -19,6 +19,7 @@ import ( "net" "net/http" + "git.wntrmute.dev/kyle/mcias/internal/audit" "git.wntrmute.dev/kyle/mcias/internal/auth" "git.wntrmute.dev/kyle/mcias/internal/config" "git.wntrmute.dev/kyle/mcias/internal/crypto" @@ -214,7 +215,7 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) { // Security: return a generic error whether the user exists or not. // Always run a dummy Argon2 check to prevent timing-based user enumeration. _, _ = auth.VerifyPassword("dummy", auth.DummyHash()) - s.writeAudit(r, model.EventLoginFail, nil, nil, fmt.Sprintf(`{"username":%q,"reason":"unknown_user"}`, req.Username)) + s.writeAudit(r, model.EventLoginFail, nil, nil, audit.JSON("username", req.Username, "reason", "unknown_user")) middleware.WriteError(w, http.StatusUnauthorized, "invalid credentials", "unauthorized") return } @@ -315,7 +316,7 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) { } s.writeAudit(r, model.EventLoginOK, &acct.ID, nil, "") - s.writeAudit(r, model.EventTokenIssued, &acct.ID, nil, fmt.Sprintf(`{"jti":%q}`, claims.JTI)) + s.writeAudit(r, model.EventTokenIssued, &acct.ID, nil, audit.JSON("jti", claims.JTI)) writeJSON(w, http.StatusOK, loginResponse{ Token: tokenStr, @@ -330,7 +331,7 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) { middleware.WriteError(w, http.StatusInternalServerError, "internal error", "internal_error") return } - s.writeAudit(r, model.EventTokenRevoked, nil, nil, fmt.Sprintf(`{"jti":%q,"reason":"logout"}`, claims.JTI)) + s.writeAudit(r, model.EventTokenRevoked, nil, nil, audit.JSON("jti", claims.JTI, "reason", "logout")) w.WriteHeader(http.StatusNoContent) } @@ -376,7 +377,7 @@ func (s *Server) handleRenew(w http.ResponseWriter, r *http.Request) { return } - s.writeAudit(r, model.EventTokenRenewed, &acct.ID, nil, fmt.Sprintf(`{"old_jti":%q,"new_jti":%q}`, claims.JTI, newClaims.JTI)) + s.writeAudit(r, model.EventTokenRenewed, &acct.ID, nil, audit.JSON("old_jti", claims.JTI, "new_jti", newClaims.JTI)) writeJSON(w, http.StatusOK, loginResponse{ Token: newTokenStr, @@ -482,7 +483,7 @@ func (s *Server) handleTokenIssue(w http.ResponseWriter, r *http.Request) { actorID = &a.ID } } - s.writeAudit(r, model.EventTokenIssued, actorID, &acct.ID, fmt.Sprintf(`{"jti":%q}`, claims.JTI)) + s.writeAudit(r, model.EventTokenIssued, actorID, &acct.ID, audit.JSON("jti", claims.JTI)) writeJSON(w, http.StatusOK, loginResponse{ Token: tokenStr, @@ -502,7 +503,7 @@ func (s *Server) handleTokenRevoke(w http.ResponseWriter, r *http.Request) { return } - s.writeAudit(r, model.EventTokenRevoked, nil, nil, fmt.Sprintf(`{"jti":%q}`, jti)) + s.writeAudit(r, model.EventTokenRevoked, nil, nil, audit.JSON("jti", jti)) w.WriteHeader(http.StatusNoContent) } @@ -597,7 +598,7 @@ func (s *Server) handleCreateAccount(w http.ResponseWriter, r *http.Request) { return } - s.writeAudit(r, model.EventAccountCreated, nil, &acct.ID, fmt.Sprintf(`{"username":%q}`, acct.Username)) + s.writeAudit(r, model.EventAccountCreated, nil, &acct.ID, audit.JSON("username", acct.Username)) writeJSON(w, http.StatusCreated, accountToResponse(acct)) } @@ -712,7 +713,7 @@ func (s *Server) handleSetRoles(w http.ResponseWriter, r *http.Request) { return } - s.writeAudit(r, model.EventRoleGranted, grantedBy, &acct.ID, fmt.Sprintf(`{"roles":%v}`, req.Roles)) + s.writeAudit(r, model.EventRoleGranted, grantedBy, &acct.ID, audit.JSONWithRoles(req.Roles)) w.WriteHeader(http.StatusNoContent) } @@ -745,7 +746,7 @@ func (s *Server) handleGrantRole(w http.ResponseWriter, r *http.Request) { return } - s.writeAudit(r, model.EventRoleGranted, grantedBy, &acct.ID, fmt.Sprintf(`{"role":"%s"}`, req.Role)) + s.writeAudit(r, model.EventRoleGranted, grantedBy, &acct.ID, audit.JSON("role", req.Role)) w.WriteHeader(http.StatusNoContent) } @@ -774,7 +775,7 @@ func (s *Server) handleRevokeRole(w http.ResponseWriter, r *http.Request) { return } - s.writeAudit(r, model.EventRoleRevoked, revokedBy, &acct.ID, fmt.Sprintf(`{"role":"%s"}`, role)) + s.writeAudit(r, model.EventRoleRevoked, revokedBy, &acct.ID, audit.JSON("role", role)) w.WriteHeader(http.StatusNoContent) } diff --git a/internal/ui/handlers_auth.go b/internal/ui/handlers_auth.go index 250341a..47ec1bb 100644 --- a/internal/ui/handlers_auth.go +++ b/internal/ui/handlers_auth.go @@ -1,9 +1,9 @@ package ui import ( - "fmt" "net/http" + "git.wntrmute.dev/kyle/mcias/internal/audit" "git.wntrmute.dev/kyle/mcias/internal/auth" "git.wntrmute.dev/kyle/mcias/internal/crypto" "git.wntrmute.dev/kyle/mcias/internal/model" @@ -59,7 +59,7 @@ func (u *UIServer) handleLoginPost(w http.ResponseWriter, r *http.Request) { // Security: always run dummy Argon2 to prevent timing-based user enumeration. _, _ = auth.VerifyPassword("dummy", u.dummyHash()) u.writeAudit(r, model.EventLoginFail, nil, nil, - fmt.Sprintf(`{"username":%q,"reason":"unknown_user"}`, username)) + audit.JSON("username", username, "reason", "unknown_user")) u.render(w, "login", LoginData{Error: "invalid credentials"}) return } @@ -130,7 +130,7 @@ func (u *UIServer) handleTOTPStep(w http.ResponseWriter, r *http.Request) { accountID, ok := u.consumeTOTPNonce(nonce) if !ok { u.writeAudit(r, model.EventLoginFail, nil, nil, - fmt.Sprintf(`{"username":%q,"reason":"invalid_totp_nonce"}`, username)) + audit.JSON("username", username, "reason", "invalid_totp_nonce")) u.render(w, "login", LoginData{Error: "session expired, please log in again"}) return } @@ -238,7 +238,7 @@ func (u *UIServer) finishLogin(w http.ResponseWriter, r *http.Request, acct *mod u.writeAudit(r, model.EventLoginOK, &acct.ID, nil, "") u.writeAudit(r, model.EventTokenIssued, &acct.ID, nil, - fmt.Sprintf(`{"jti":%q,"via":"ui"}`, claims.JTI)) + audit.JSON("jti", claims.JTI, "via", "ui")) // Redirect to dashboard. if isHTMX(r) { @@ -259,7 +259,7 @@ func (u *UIServer) handleLogout(w http.ResponseWriter, r *http.Request) { u.logger.Warn("revoke token on UI logout", "error", revokeErr) } u.writeAudit(r, model.EventTokenRevoked, nil, nil, - fmt.Sprintf(`{"jti":%q,"reason":"ui_logout"}`, claims.JTI)) + audit.JSON("jti", claims.JTI, "reason", "ui_logout")) } } u.clearSessionCookie(w)