From 3fa4cccb695e1752addf105580fb7cc77882f0d8 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 08:35:40 +0200 Subject: [PATCH] fix(auth): decode the base64-encoded password in setupAdmin The frontend base64-encodes the password sent to setup-admin, as it does for login, but setupAdmin forwarded the encoded string straight to the auth service, which bcrypt-hashed the base64 text. Every subsequent login (which does decode) then compared the real plaintext against that hash and failed, locking the bootstrap admin out permanently. Decode with decodeBase64Password before calling auth.SetupAdmin, exactly like login and resetPassword, and drop the now-stale setupAdmin exemption from the base64-password-guard test. Closes #224 --- internal/api/base64_password_guard_test.go | 22 +++--- internal/api/handler_auth.go | 9 +++ internal/api/handler_auth_test.go | 5 +- internal/api/handler_coverage_test.go | 6 +- internal/server/adapter_test.go | 79 ++++++++++++++++++++++ 5 files changed, 108 insertions(+), 13 deletions(-) diff --git a/internal/api/base64_password_guard_test.go b/internal/api/base64_password_guard_test.go index 9e33f8552..cc17c3c19 100644 --- a/internal/api/base64_password_guard_test.go +++ b/internal/api/base64_password_guard_test.go @@ -21,9 +21,10 @@ // // For each such function the test verifies that the function body also contains // a call to decodeBase64Password or one of its known delegate helpers -// (decodeChangePasswordRequest). Functions that are legitimately exempt (e.g., -// setupAdmin, which is bootstrapped separately) must be listed in -// knownExemptFunctions with an explanation. +// (decodeChangePasswordRequest). A function that is legitimately exempt for a +// documented reason must be listed in knownExemptFunctions with an +// explanation; there are currently no such functions (see issue #224, which +// removed the previous setupAdmin exemption). // // # Synthetic-regression sub-test // @@ -54,14 +55,13 @@ import ( // decoding the password (e.g., the endpoint uses a different auth convention or // is not exposed to the frontend encode/decode contract). Every entry here // should be accompanied by a comment explaining why. -var knownExemptFunctions = map[string]string{ - // setupAdmin: the bootstrap endpoint. The frontend does base64-encode the - // password (see frontend/src/api/auth.ts:setupAdmin), but the backend - // currently forwards the raw value to the auth service which handles - // hashing internally. Tracked as a separate concern; do not expand this - // exemption to other handlers. - "setupAdmin": "bootstrap endpoint — see comment in knownExemptFunctions for details", -} +// knownExemptFunctions intentionally starts empty. Add an entry here only +// when a handler has a documented reason for not decoding a password field +// (e.g., the endpoint uses a different auth convention or is not exposed to +// the frontend encode/decode contract), with a comment explaining why. +// setupAdmin was previously (wrongly) exempted here; see issue #224. It now +// decodes like every other password handler. +var knownExemptFunctions = map[string]string{} // decodeHelpers is the set of function-call names that constitute a compliant // alternative to calling decodeBase64Password directly. A handler that diff --git a/internal/api/handler_auth.go b/internal/api/handler_auth.go index 3040098b5..e0d868651 100644 --- a/internal/api/handler_auth.go +++ b/internal/api/handler_auth.go @@ -240,6 +240,15 @@ func (h *Handler) setupAdmin(ctx context.Context, req *events.LambdaFunctionURLR return nil, NewClientError(400, "invalid request body") } + // Decode base64-encoded password (the frontend encodes it identically to + // login/resetPassword; skipping this hashes the base64 text and locks the + // bootstrap admin out on first login — see issue #224). + decoded, err := decodeBase64Password(setupReq.Password) + if err != nil { + return nil, err + } + setupReq.Password = decoded + response, err := h.auth.SetupAdmin(ctx, setupReq) if err != nil { // Share the sentinel→ClientError mapping with /api/users (issue #349) diff --git a/internal/api/handler_auth_test.go b/internal/api/handler_auth_test.go index 6b27fb29c..053eaa485 100644 --- a/internal/api/handler_auth_test.go +++ b/internal/api/handler_auth_test.go @@ -431,13 +431,16 @@ func TestHandler_setupAdmin_Success(t *testing.T) { handler := &Handler{auth: mockAuth} + // Password must be base64-encoded in the request body, exactly as the + // frontend encodes it (issue #224). + encodedPassword := base64.StdEncoding.EncodeToString([]byte("admin123")) req := &events.LambdaFunctionURLRequest{ RequestContext: events.LambdaFunctionURLRequestContext{ HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{ SourceIP: "127.0.0.1", }, }, - Body: `{"email": "admin@example.com", "password": "admin123"}`, + Body: `{"email": "admin@example.com", "password": "` + encodedPassword + `"}`, } result, err := handler.setupAdmin(ctx, req) diff --git a/internal/api/handler_coverage_test.go b/internal/api/handler_coverage_test.go index b64feff96..d0daf328d 100644 --- a/internal/api/handler_coverage_test.go +++ b/internal/api/handler_coverage_test.go @@ -2,6 +2,7 @@ package api import ( "context" + "encoding/base64" "errors" "testing" @@ -239,8 +240,11 @@ func TestRouter_Handlers_Coverage(t *testing.T) { h := &Handler{auth: mockAuth} router := NewRouter(h) + // Password must be base64-encoded, exactly as the frontend encodes it + // (issue #224) - setupAdmin decodes it before forwarding to auth. + encodedPassword := base64.StdEncoding.EncodeToString([]byte("pass123")) req := &events.LambdaFunctionURLRequest{ - Body: `{"email": "admin@example.com", "password": "pass123"}`, + Body: `{"email": "admin@example.com", "password": "` + encodedPassword + `"}`, } result, err := router.setupAdminHandler(ctx, req, nil) diff --git a/internal/server/adapter_test.go b/internal/server/adapter_test.go index 0d7b15758..ecc99b302 100644 --- a/internal/server/adapter_test.go +++ b/internal/server/adapter_test.go @@ -3,12 +3,14 @@ package server import ( "context" "crypto/sha256" + "encoding/base64" "encoding/hex" "testing" "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/api" "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" + "github.com/aws/aws-lambda-go/events" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -480,3 +482,80 @@ func TestAuthServiceAdapter_SetupAdmin(t *testing.T) { assert.NotNil(t, resp) assert.Equal(t, "admin@example.com", resp.User.Email) } + +// TestSetupAdminThenLogin_RoundTrip is a regression test for issue #224: the +// frontend base64-encodes the setup-admin password exactly as it does for +// login (frontend/src/api/auth.ts), but the setupAdmin handler used to +// forward the still-encoded string straight through the adapter to +// auth.Service.SetupAdmin, which bcrypt-hashed the base64 text. Every later +// login (whose handler does decode) then compared the real plaintext +// against that hash and failed, locking the bootstrap admin out +// permanently. +// +// This drives the full stack an HTTP request would: api.Router.Route -> +// api.Handler.setupAdmin/login -> authServiceAdapter -> the real +// *auth.Service, backed by an auth.MockStore that persists the user +// setupAdmin creates and hands it back to login. It fails on the pre-fix +// setupAdmin handler (which skips decodeBase64Password) and passes once it +// decodes like every other password handler. +func TestSetupAdminThenLogin_RoundTrip(t *testing.T) { + ctx := context.Background() + + mockStore := new(auth.MockStore) + realAuth := auth.NewService(auth.ServiceConfig{ + Store: mockStore, + SessionDuration: time.Hour, + CSRFKey: auth.TestCSRFKey(), + DashboardURL: "http://localhost:3000", + }) + adapter := newAuthServiceAdapter(realAuth) + handler := api.NewHandler(api.HandlerConfig{AuthService: adapter}) + router := api.NewRouter(handler) + + const email = "admin@example.com" + const plaintextPassword = "SecureBootstrapPass@1" + encodedPassword := base64.StdEncoding.EncodeToString([]byte(plaintextPassword)) + body := `{"email": "` + email + `", "password": "` + encodedPassword + `"}` + + mockStore.On("AdminExists", ctx).Return(false, nil).Once() + + var createdUser *auth.User + mockStore.On("CreateAdminIfNone", ctx, mock.AnythingOfType("*auth.User")). + Run(func(args mock.Arguments) { + u, ok := args.Get(1).(*auth.User) + require.True(t, ok) + createdUser = u + }). + Return(true, nil).Once() + mockStore.On("CreateSession", ctx, mock.AnythingOfType("*auth.Session")).Return(nil).Twice() + mockStore.On("UpdateUser", ctx, mock.AnythingOfType("*auth.User")).Return(nil).Maybe() + + setupReq := &events.LambdaFunctionURLRequest{ + RequestContext: events.LambdaFunctionURLRequestContext{ + HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{SourceIP: "127.0.0.1"}, + }, + Body: body, + } + _, err := router.Route(ctx, "POST", "/api/auth/setup-admin", setupReq) + require.NoError(t, err) + require.NotNil(t, createdUser, "setupAdmin must have created the admin user") + + // login looks the user up by email; hand back the user setupAdmin + // created, carrying whatever password hash setupAdmin actually produced. + mockStore.On("GetUserByEmail", ctx, email).Return(createdUser, nil).Once() + + loginReq := &events.LambdaFunctionURLRequest{ + RequestContext: events.LambdaFunctionURLRequestContext{ + HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{SourceIP: "127.0.0.1"}, + }, + Body: body, + } + result, err := router.Route(ctx, "POST", "/api/auth/login", loginReq) + require.NoError(t, err, "the bootstrap admin must be able to log in with the same frontend-encoded password used at setup") + + resp, ok := result.(*api.LoginResponse) + require.True(t, ok) + assert.Equal(t, email, resp.User.Email) + + mockStore.AssertExpectations(t) +}