diff --git a/docs/AUTHENTICATION.md b/docs/AUTHENTICATION.md index 385b643..42ca149 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -56,7 +56,16 @@ and printed once in the logs: WARN Initial admin account created with a generated password. Change it at first login. username=admin password=... ``` -The account is flagged `mustChangePassword`. Change it right away: +The account is flagged `mustChangePassword`, and the server enforces it: until +the password is changed, the account can only read `/auth/me` and +`/auth/config`, sign out and change its password. Every other API call is +refused: HTTP 403 with `password change required`, or gRPC `PermissionDenied` +(counted as `tracker_auth_requests_total{result="password_change_required"}`). +The web assets, `/config.js` and the API docs stay reachable, as they serve no +data. The same applies to any local user created or reset by an administrator. +On a reset, the user's existing sessions are signed out (the session version is +bumped, they answer 401) and the restriction applies from the next login. API +keys are never affected. Change the password right away: ```bash curl -c jar -X POST http://localhost:8080/api/v1alpha1/auth/login \ @@ -67,6 +76,9 @@ curl -b jar -c jar -X POST http://localhost:8080/api/v1alpha1/auth/password \ -d '{"currentPassword":"","newPassword":""}' ``` +Changing the password reissues the session cookie (the cookie jar above keeps +the new one), so no new login is needed afterwards. + Passwords are hashed with Argon2id and must be 12 to 128 characters long. ## Sessions @@ -475,7 +487,7 @@ the anonymous permissions. `tracker_auth_requests_total{principal,result}` counts authorization decisions, with `principal` in `anonymous`, `user`, `apikey` and `result` -in `allowed`, `unauthenticated`, `denied`. +in `allowed`, `unauthenticated`, `denied`, `password_change_required`. `tracker_auth_logins_total{method,result}` counts login attempts, with `method` in `local`, `oidc` and `result` in `success`, `failure`, diff --git a/internal/auth/authz/authz.go b/internal/auth/authz/authz.go index 062f66c..4424f14 100644 --- a/internal/auth/authz/authz.go +++ b/internal/auth/authz/authz.go @@ -92,6 +92,8 @@ func Check(p auth.Principal, method string) error { return CheckPermission(p, perm) } +const passwordChangeRequired = "password change required" + // CheckPermission is the pure decision for a principal and a permission. func CheckPermission(p auth.Principal, perm auth.Permission) error { // A credential that was presented and refused loses even public routes. @@ -101,6 +103,12 @@ func CheckPermission(p auth.Principal, perm auth.Permission) error { if p.CredentialRejected { return status.Error(codes.Unauthenticated, "invalid or expired credentials") } + // A local user flagged for a password change may only reach public + // routes (Me, GetAuthConfig) and the hand-written login, logout and + // change-password handlers, which are not guarded by a permission. + if p.MustChangePassword && perm != auth.PermPublic { + return status.Error(codes.PermissionDenied, passwordChangeRequired) + } switch perm { case auth.PermPublic: return nil @@ -146,8 +154,11 @@ func observe(p auth.Principal, method string, err error) { result := "allowed" if err != nil { result = "denied" - if status.Code(err) == codes.Unauthenticated { + switch { + case status.Code(err) == codes.Unauthenticated: result = "unauthenticated" + case status.Code(err) == codes.PermissionDenied && status.Convert(err).Message() == passwordChangeRequired: + result = "password_change_required" } slog.Warn("authz denied", "method", method, "principal", p.Username, "kind", p.Kind, "reason", status.Convert(err).Message()) } diff --git a/internal/auth/authz/authz_test.go b/internal/auth/authz/authz_test.go index 709be2c..86b024e 100644 --- a/internal/auth/authz/authz_test.go +++ b/internal/auth/authz/authz_test.go @@ -140,3 +140,48 @@ func TestRequireHTTPRefusesRejectedCredential(t *testing.T) { assert.Equal(t, http.StatusUnauthorized, w.Code) assert.False(t, called, "the handler must not run") } + +// A user whose password must be changed can only reach public routes: the +// server enforces the forced change, the web UI redirect is a convenience. +func TestMustChangePasswordOnlyReachesPublicRoutes(t *testing.T) { + flagged := auth.Principal{ + Kind: auth.KindUser, + Username: "admin", + Permissions: auth.NewPermissionSet(auth.PermEventRead, auth.PermLinksRead, auth.PermAccessManage), + IsAdmin: true, + MustChangePassword: true, + } + + for _, perm := range []auth.Permission{auth.PermEventRead, auth.PermLinksRead, auth.PermAccessManage, auth.PermAuthenticated} { + err := CheckPermission(flagged, perm) + assert.Equal(t, codes.PermissionDenied, status.Code(err), "permission %s", perm) + assert.Equal(t, "password change required", status.Convert(err).Message(), "permission %s", perm) + } + assert.NoError(t, CheckPermission(flagged, auth.PermPublic)) + assert.NoError(t, Check(flagged, getAuthConfig)) + assert.Equal(t, codes.PermissionDenied, status.Code(Check(flagged, listEvents))) + + // A rejected credential still wins with a 401. + rejected := flagged + rejected.CredentialRejected = true + assert.Equal(t, codes.Unauthenticated, status.Code(CheckPermission(rejected, auth.PermPublic))) + assert.Equal(t, codes.Unauthenticated, status.Code(CheckPermission(rejected, auth.PermEventRead))) + + // The same principal without the flag is served normally. + flagged.MustChangePassword = false + assert.NoError(t, CheckPermission(flagged, auth.PermEventRead)) +} + +func TestRequireHTTPRefusesMustChangePassword(t *testing.T) { + called := false + h := RequireHTTP(auth.PermLinksRead, func(w http.ResponseWriter, r *http.Request, _ map[string]string) { called = true }) + + ctx := auth.WithPrincipal(context.Background(), auth.Principal{ + Kind: auth.KindUser, Permissions: auth.NewPermissionSet(auth.PermLinksRead), MustChangePassword: true, + }) + rec := httptest.NewRecorder() + h(rec, httptest.NewRequest(http.MethodGet, "/api/links", nil).WithContext(ctx), nil) + assert.Equal(t, http.StatusForbidden, rec.Code) + assert.JSONEq(t, `{"error":"password change required"}`, rec.Body.String()) + assert.False(t, called) +} diff --git a/internal/auth/identity/resolver.go b/internal/auth/identity/resolver.go index eb27038..99f1107 100644 --- a/internal/auth/identity/resolver.go +++ b/internal/auth/identity/resolver.go @@ -193,12 +193,13 @@ func (r *Resolver) PrincipalForUser(ctx context.Context, user *store.User) (auth teamIDs = append(teamIDs, t.ID.Hex()) } return auth.Principal{ - Kind: auth.KindUser, - UserID: user.ID.Hex(), - Username: user.Username, - TeamIDs: teamIDs, - Permissions: perms, - Scope: scope, - IsAdmin: admin, + Kind: auth.KindUser, + UserID: user.ID.Hex(), + Username: user.Username, + TeamIDs: teamIDs, + Permissions: perms, + Scope: scope, + IsAdmin: admin, + MustChangePassword: user.MustChangePassword, }, nil } diff --git a/internal/auth/identity/resolver_test.go b/internal/auth/identity/resolver_test.go index 9567d19..538fb60 100644 --- a/internal/auth/identity/resolver_test.go +++ b/internal/auth/identity/resolver_test.go @@ -205,3 +205,24 @@ func TestResolveAPIKeyOnAdministratorsTeam(t *testing.T) { } assert.False(t, r.Resolve(context.Background(), auth.Credentials{APIKey: teamKey.Secret}).IsAdmin) } + +func TestPrincipalForUserCarriesMustChangePassword(t *testing.T) { + r, user, _, _ := newFixture(t) + + p, err := r.PrincipalForUser(context.Background(), user) + require.NoError(t, err) + assert.False(t, p.MustChangePassword) + + user.MustChangePassword = true + p, err = r.PrincipalForUser(context.Background(), user) + require.NoError(t, err) + assert.True(t, p.MustChangePassword) + + token, _, _ := r.Sessions.Issue(user.ID.Hex(), user.SessionVersion) + assert.True(t, r.Resolve(context.Background(), auth.Credentials{SessionToken: token}).MustChangePassword) + + // An API key never carries the flag, whoever created it. + gen, _ := auth.GenerateAPIKey() + r.Keys.(*fakeKeys).byPrefix[gen.Prefix] = &store.APIKey{ID: primitive.NewObjectID(), Prefix: gen.Prefix, Hash: gen.Hash} + assert.False(t, r.Resolve(context.Background(), auth.Credentials{APIKey: gen.Secret}).MustChangePassword) +} diff --git a/internal/auth/principal.go b/internal/auth/principal.go index 17cb491..4cdba16 100644 --- a/internal/auth/principal.go +++ b/internal/auth/principal.go @@ -27,6 +27,11 @@ type Principal struct { // credential that could not be honoured. Authorization turns it into a // 401 whatever the permission asked for, including a public one. CredentialRejected bool + // MustChangePassword is true for a local user whose password must be + // changed before anything else. Authorization then refuses every + // non-public permission. API keys, anonymous and OIDC principals never + // carry it. + MustChangePassword bool } // Anonymous returns the principal used for unauthenticated requests. diff --git a/server/auth_password_required_test.go b/server/auth_password_required_test.go new file mode 100644 index 0000000..8641ebe --- /dev/null +++ b/server/auth_password_required_test.go @@ -0,0 +1,142 @@ +package server + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + authv1 "github.com/bananaops/tracker/generated/proto/auth/v1alpha1" + eventv1 "github.com/bananaops/tracker/generated/proto/event/v1alpha1" + "github.com/bananaops/tracker/internal/auth" + "github.com/bananaops/tracker/internal/auth/authz" + "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// newPasswordRequiredServer mounts the real auth routes, the real AuthService +// and EventService gateways and the HTTP middleware. The two guarded probe +// routes use authz.RequireHTTP exactly as /api/links does, without needing the +// global MongoDB collections the links and event stores connect to. The Event +// service has no store: a denied call returns before reaching it. +func newPasswordRequiredServer(t *testing.T, f *authFixture) http.Handler { + t.Helper() + mux := runtime.NewServeMux() + NewAuthHTTP(f.users, f.sessions, f.cfg).Register(mux) + require.NoError(t, authv1.RegisterAuthServiceHandlerServer(context.Background(), mux, newAuthService(f))) + require.NoError(t, eventv1.RegisterEventServiceHandlerServer(context.Background(), mux, &Event{})) + ok := func(w http.ResponseWriter, _ *http.Request, _ map[string]string) { w.WriteHeader(http.StatusOK) } + require.NoError(t, mux.HandlePath(http.MethodGet, "/api/links", authz.RequireHTTP(auth.PermLinksRead, ok))) + require.NoError(t, mux.HandlePath(http.MethodGet, "/api/probe/events", authz.RequireHTTP(auth.PermEventRead, ok))) + return auth.HTTPMiddleware(f.resolver, f.cfg)(recoverAsServerError(mux)) +} + +// recoverAsServerError turns a panic into a 500. The Event service has no +// store, so if the authz rule ever stopped refusing a flagged user the call +// would reach the nil store and panic, aborting the whole test binary and +// hiding every test scheduled after it. A 500 makes the assertion fail +// cleanly instead. +func recoverAsServerError(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer func() { + if recover() != nil { + w.WriteHeader(http.StatusInternalServerError) + } + }() + next.ServeHTTP(w, r) + }) +} + +func get(h http.Handler, path string, cookie *http.Cookie, bearer string) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodGet, path, nil) + if cookie != nil { + req.AddCookie(cookie) + } + if bearer != "" { + req.Header.Set("Authorization", "Bearer "+bearer) + } + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec +} + +func TestForcedPasswordChangeIsEnforcedByTheServer(t *testing.T) { + f := newAuthFixture(t) + require.True(t, f.admin.MustChangePassword, "the bootstrap admin is flagged") + h := newPasswordRequiredServer(t, f) + + login := post(h, "/api/v1alpha1/auth/login", `{"username":"admin","password":"admin-password-123"}`, nil) + require.Equal(t, http.StatusNoContent, login.Code, login.Body.String()) + cookie := sessionCookie(t, login) + + // Every protected route is refused, whatever the transport. + for _, path := range []string{"/api/v1alpha1/events/list", "/api/links", "/api/probe/events"} { + rec := get(h, path, cookie, "") + assert.Equal(t, http.StatusForbidden, rec.Code, path) + assert.Contains(t, rec.Body.String(), "password change required", path) + } + rec := get(h, "/api/v1alpha1/auth/users", cookie, "") + assert.Equal(t, http.StatusForbidden, rec.Code, "access:manage route") + assert.Contains(t, rec.Body.String(), "password change required") + + // The public routes stay reachable: the UI needs Me to learn it must redirect. + rec = get(h, "/api/v1alpha1/auth/me", cookie, "") + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"mustChangePassword":true`) + assert.Equal(t, http.StatusOK, get(h, "/api/v1alpha1/auth/config", cookie, "").Code) + + // The password can be changed, and the new session is served normally. + rec = post(h, "/api/v1alpha1/auth/password", `{"currentPassword":"admin-password-123","newPassword":"brand-new-password-1"}`, cookie) + require.Equal(t, http.StatusNoContent, rec.Code, rec.Body.String()) + fresh := sessionCookie(t, rec) + + // The change bumps the session version: the old cookie is dead, the + // reissued one works without logging in again. + assert.Equal(t, http.StatusUnauthorized, get(h, "/api/probe/events", cookie, "").Code, "old session") + assert.Equal(t, http.StatusOK, get(h, "/api/probe/events", fresh, "").Code) + assert.Equal(t, http.StatusOK, get(h, "/api/links", fresh, "").Code) + rec = get(h, "/api/v1alpha1/auth/me", fresh, "") + assert.Contains(t, rec.Body.String(), `"mustChangePassword":false`) +} + +func TestLogoutStillWorksWhileFlagged(t *testing.T) { + f := newAuthFixture(t) + h := newPasswordRequiredServer(t, f) + + login := post(h, "/api/v1alpha1/auth/login", `{"username":"admin","password":"admin-password-123"}`, nil) + cookie := sessionCookie(t, login) + + rec := post(h, "/api/v1alpha1/auth/logout", `{}`, cookie) + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.Equal(t, -1, sessionCookie(t, rec).MaxAge) +} + +// An API key is never affected by the flag of the user who created it. +func TestAPIKeyIgnoresCreatorMustChangePassword(t *testing.T) { + f := newAuthFixture(t) + svc := newAuthService(f) + h := newPasswordRequiredServer(t, f) + + // A flagged admin cannot create a key through the service: it is refused. + flagged := f.flaggedPrincipalOf(t, f.admin) + require.True(t, flagged.MustChangePassword) + _, err := svc.CreateApiKey(rpcCtx(flagged, "CreateApiKey"), &authv1.CreateApiKeyRequest{Name: "denied"}) + require.Error(t, err) + assert.Equal(t, codes.PermissionDenied, status.Code(err)) + assert.Equal(t, "password change required", status.Convert(err).Message()) + + // The key exists, created by an admin (here the same one with the flag + // lifted on the principal only), while the stored user is still flagged. + created, err := svc.CreateApiKey(rpcCtx(f.principalOf(t, f.admin), "CreateApiKey"), &authv1.CreateApiKeyRequest{Name: "ci"}) + require.NoError(t, err) + + stored, err := f.users.GetByID(context.Background(), f.admin.ID) + require.NoError(t, err) + require.True(t, stored.MustChangePassword) + + assert.Equal(t, http.StatusOK, get(h, "/api/probe/events", nil, created.Secret).Code) + assert.Equal(t, http.StatusOK, get(h, "/api/links", nil, created.Secret).Code) +} diff --git a/server/auth_testing_test.go b/server/auth_testing_test.go index 7854c78..0a207af 100644 --- a/server/auth_testing_test.go +++ b/server/auth_testing_test.go @@ -67,8 +67,20 @@ func newAuthFixture(t *testing.T) *authFixture { return f } -// principalOf resolves the principal of a stored user through the real resolver. +// principalOf resolves the principal of a stored user through the real +// resolver. The bootstrap admin is flagged mustChangePassword, which authz +// rightly refuses everywhere; the service tests that use this helper exercise +// the service rules, not that gate, so the flag is lifted on the principal +// only (the stored user keeps it). Use flaggedPrincipalOf to keep it. func (f *authFixture) principalOf(t *testing.T, u *store.User) auth.Principal { + t.Helper() + p := f.flaggedPrincipalOf(t, u) + p.MustChangePassword = false + return p +} + +// flaggedPrincipalOf is principalOf without lifting mustChangePassword. +func (f *authFixture) flaggedPrincipalOf(t *testing.T, u *store.User) auth.Principal { t.Helper() p, err := f.resolver.PrincipalForUser(context.Background(), u) require.NoError(t, err)