From 9d6d09290b718161f186c5b26a43bad20a67ffc4 Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Mon, 6 Jul 2026 16:51:55 +0530 Subject: [PATCH 1/3] fix(couchbase): persist json:"-" fields (User.Password) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Couchbase (de)serializes whole schema structs via encoding/json (gocb default transcoder), which honors json:"-". User.Password is tagged json:"-" for API safety, so it was silently dropped on both write and read — password auth never worked on Couchbase, and no migration existed. - add structToDocument / decodeDocument helpers that re-add json:"-" fields under their bson key; route all Insert/Upsert through the write helper and User reads through the read helper - assert password round-trips in the cross-DB storage provider test (would have caught this) plus unit tests for the helpers --- internal/storage/db/couchbase/audit_log.go | 6 +- .../storage/db/couchbase/authenticator.go | 6 +- .../storage/db/couchbase/email_template.go | 6 +- internal/storage/db/couchbase/env.go | 6 +- internal/storage/db/couchbase/otp.go | 6 +- internal/storage/db/couchbase/session.go | 6 +- .../storage/db/couchbase/session_token.go | 18 ++- internal/storage/db/couchbase/shared.go | 99 ++++++++++++++ internal/storage/db/couchbase/shared_test.go | 124 ++++++++++++++++++ internal/storage/db/couchbase/user.go | 51 ++++--- .../db/couchbase/verification_requests.go | 6 +- internal/storage/db/couchbase/webhook.go | 6 +- internal/storage/db/couchbase/webhook_log.go | 6 +- internal/storage/provider_test.go | 7 + 14 files changed, 326 insertions(+), 27 deletions(-) create mode 100644 internal/storage/db/couchbase/shared_test.go diff --git a/internal/storage/db/couchbase/audit_log.go b/internal/storage/db/couchbase/audit_log.go index c48b3a581..7bd8e4682 100644 --- a/internal/storage/db/couchbase/audit_log.go +++ b/internal/storage/db/couchbase/audit_log.go @@ -24,7 +24,11 @@ func (p *provider) AddAuditLog(ctx context.Context, auditLog *schemas.AuditLog) insertOpt := gocb.InsertOptions{ Context: ctx, } - _, err := p.db.Collection(schemas.Collections.AuditLog).Insert(auditLog.ID, auditLog, &insertOpt) + doc, err := structToDocument(auditLog) + if err != nil { + return err + } + _, err = p.db.Collection(schemas.Collections.AuditLog).Insert(auditLog.ID, doc, &insertOpt) if err != nil { return err } diff --git a/internal/storage/db/couchbase/authenticator.go b/internal/storage/db/couchbase/authenticator.go index 05bce1c1a..78e2612fe 100644 --- a/internal/storage/db/couchbase/authenticator.go +++ b/internal/storage/db/couchbase/authenticator.go @@ -28,7 +28,11 @@ func (p *provider) AddAuthenticator(ctx context.Context, authenticators *schemas insertOpt := gocb.InsertOptions{ Context: ctx, } - _, err := p.db.Collection(schemas.Collections.Authenticators).Insert(authenticators.ID, authenticators, &insertOpt) + doc, err := structToDocument(authenticators) + if err != nil { + return nil, err + } + _, err = p.db.Collection(schemas.Collections.Authenticators).Insert(authenticators.ID, doc, &insertOpt) if err != nil { return nil, err } diff --git a/internal/storage/db/couchbase/email_template.go b/internal/storage/db/couchbase/email_template.go index deebfac15..ec3bf691b 100644 --- a/internal/storage/db/couchbase/email_template.go +++ b/internal/storage/db/couchbase/email_template.go @@ -29,7 +29,11 @@ func (p *provider) AddEmailTemplate(ctx context.Context, emailTemplate *schemas. Context: ctx, } - _, err := p.db.Collection(schemas.Collections.EmailTemplate).Insert(emailTemplate.ID, emailTemplate, &insertOpt) + doc, err := structToDocument(emailTemplate) + if err != nil { + return nil, err + } + _, err = p.db.Collection(schemas.Collections.EmailTemplate).Insert(emailTemplate.ID, doc, &insertOpt) if err != nil { return nil, err } diff --git a/internal/storage/db/couchbase/env.go b/internal/storage/db/couchbase/env.go index 14b20ef2a..d25e7f12f 100644 --- a/internal/storage/db/couchbase/env.go +++ b/internal/storage/db/couchbase/env.go @@ -23,7 +23,11 @@ func (p *provider) AddEnv(ctx context.Context, env *schemas.Env) (*schemas.Env, insertOpt := gocb.InsertOptions{ Context: ctx, } - _, err := p.db.Collection(schemas.Collections.Env).Insert(env.ID, env, &insertOpt) + doc, err := structToDocument(env) + if err != nil { + return nil, err + } + _, err = p.db.Collection(schemas.Collections.Env).Insert(env.ID, doc, &insertOpt) if err != nil { return nil, err } diff --git a/internal/storage/db/couchbase/otp.go b/internal/storage/db/couchbase/otp.go index 1e9261043..c864f5b0b 100644 --- a/internal/storage/db/couchbase/otp.go +++ b/internal/storage/db/couchbase/otp.go @@ -49,7 +49,11 @@ func (p *provider) UpsertOTP(ctx context.Context, otpParam *schemas.OTP) (*schem insertOpt := gocb.InsertOptions{ Context: ctx, } - _, err := p.db.Collection(schemas.Collections.OTP).Insert(otp.ID, otp, &insertOpt) + doc, err := structToDocument(otp) + if err != nil { + return nil, err + } + _, err = p.db.Collection(schemas.Collections.OTP).Insert(otp.ID, doc, &insertOpt) if err != nil { return nil, err } diff --git a/internal/storage/db/couchbase/session.go b/internal/storage/db/couchbase/session.go index 74531a34c..025c7d12f 100644 --- a/internal/storage/db/couchbase/session.go +++ b/internal/storage/db/couchbase/session.go @@ -21,7 +21,11 @@ func (p *provider) AddSession(ctx context.Context, session *schemas.Session) err insertOpt := gocb.InsertOptions{ Context: ctx, } - _, err := p.db.Collection(schemas.Collections.Session).Insert(session.ID, session, &insertOpt) + doc, err := structToDocument(session) + if err != nil { + return err + } + _, err = p.db.Collection(schemas.Collections.Session).Insert(session.ID, doc, &insertOpt) if err != nil { return err } diff --git a/internal/storage/db/couchbase/session_token.go b/internal/storage/db/couchbase/session_token.go index 1f903b760..376903ea6 100644 --- a/internal/storage/db/couchbase/session_token.go +++ b/internal/storage/db/couchbase/session_token.go @@ -22,7 +22,11 @@ func (p *provider) AddSessionToken(ctx context.Context, token *schemas.SessionTo if token.UpdatedAt == 0 { token.UpdatedAt = time.Now().Unix() } - _, err := p.db.Collection(schemas.Collections.SessionToken).Insert(token.ID, token, &gocb.InsertOptions{Context: ctx}) + doc, err := structToDocument(token) + if err != nil { + return err + } + _, err = p.db.Collection(schemas.Collections.SessionToken).Insert(token.ID, doc, &gocb.InsertOptions{Context: ctx}) return err } @@ -180,7 +184,11 @@ func (p *provider) AddMFASession(ctx context.Context, session *schemas.MFASessio if session.UpdatedAt == 0 { session.UpdatedAt = time.Now().Unix() } - _, err := p.db.Collection(schemas.Collections.MFASession).Insert(session.ID, session, &gocb.InsertOptions{Context: ctx}) + doc, err := structToDocument(session) + if err != nil { + return err + } + _, err = p.db.Collection(schemas.Collections.MFASession).Insert(session.ID, doc, &gocb.InsertOptions{Context: ctx}) return err } @@ -328,7 +336,11 @@ func (p *provider) AddOAuthState(ctx context.Context, state *schemas.OAuthState) _, _ = p.db.Collection(schemas.Collections.OAuthState).Remove(row.ID, &gocb.RemoveOptions{Context: ctx}) } } - _, err := p.db.Collection(schemas.Collections.OAuthState).Insert(state.ID, state, &gocb.InsertOptions{Context: ctx}) + doc, err := structToDocument(state) + if err != nil { + return err + } + _, err = p.db.Collection(schemas.Collections.OAuthState).Insert(state.ID, doc, &gocb.InsertOptions{Context: ctx}) return err } diff --git a/internal/storage/db/couchbase/shared.go b/internal/storage/db/couchbase/shared.go index 504172664..c1444df69 100644 --- a/internal/storage/db/couchbase/shared.go +++ b/internal/storage/db/couchbase/shared.go @@ -2,6 +2,7 @@ package couchbase import ( "context" + "encoding/json" "fmt" "reflect" "strings" @@ -9,6 +10,104 @@ import ( "github.com/couchbase/gocb/v2" ) +// structToDocument converts a schema struct into a map for Couchbase persistence. +// +// Couchbase is the only storage provider that (de)serializes whole schema structs +// through encoding/json (the gocb default JSON transcoder). encoding/json honors +// `json:"-"`, which is set on secret fields such as User.Password purely to keep them +// out of API/GraphQL JSON responses. As a side effect those fields were silently +// dropped from the persisted document. structToDocument reproduces the exact JSON +// document shape for every normally-tagged field, then re-adds any `json:"-"` field +// under its `bson` tag key so it is actually persisted. All Insert/Upsert call sites +// pass their document through this helper. +// +// ponytail: paired with decodeDocument. If a schema gains a new `json:"-"` field, its +// read path MUST decode via decodeDocument or the field will load empty. Today only +// User.Password is affected, and only User's read methods use decodeDocument. +func structToDocument(v interface{}) (map[string]interface{}, error) { + data, err := json.Marshal(v) + if err != nil { + return nil, err + } + doc := map[string]interface{}{} + if err := json.Unmarshal(data, &doc); err != nil { + return nil, err + } + rv := reflect.Indirect(reflect.ValueOf(v)) + if rv.Kind() != reflect.Struct { + return doc, nil + } + rt := rv.Type() + for i := 0; i < rt.NumField(); i++ { + f := rt.Field(i) + if f.Tag.Get("json") != "-" { + continue + } + key := persistKey(f) + if key == "" { + continue + } + doc[key] = rv.Field(i).Interface() + } + return doc, nil +} + +// decodeDocument unmarshals a Couchbase row/document into dest, then populates any +// `json:"-"` field from its `bson` tag key (encoding/json ignores such fields on +// unmarshal too). dest must be a non-nil pointer to a struct. See structToDocument. +func decodeDocument(data []byte, dest interface{}) error { + if err := json.Unmarshal(data, dest); err != nil { + return err + } + rv := reflect.Indirect(reflect.ValueOf(dest)) + if rv.Kind() != reflect.Struct { + return nil + } + rt := rv.Type() + var rawFields map[string]json.RawMessage + for i := 0; i < rt.NumField(); i++ { + f := rt.Field(i) + if f.Tag.Get("json") != "-" { + continue + } + key := persistKey(f) + if key == "" { + continue + } + fv := rv.Field(i) + if !fv.CanSet() { + continue + } + if rawFields == nil { + rawFields = map[string]json.RawMessage{} + if err := json.Unmarshal(data, &rawFields); err != nil { + return err + } + } + raw, ok := rawFields[key] + if !ok { + continue + } + nv := reflect.New(f.Type) + if err := json.Unmarshal(raw, nv.Interface()); err != nil { + return err + } + fv.Set(nv.Elem()) + } + return nil +} + +// persistKey returns the document key a `json:"-"` field is stored under. Couchbase +// document keys mirror the bson/json tag names, so it uses the field's `bson` tag, +// falling back to the lowercased field name. +func persistKey(f reflect.StructField) string { + name := strings.Split(f.Tag.Get("bson"), ",")[0] + if name == "" || name == "-" { + return strings.ToLower(f.Name) + } + return name +} + // GetSetFields to get set fields func GetSetFields(webhookMap map[string]interface{}) (string, map[string]interface{}) { params := make(map[string]interface{}, 1) diff --git a/internal/storage/db/couchbase/shared_test.go b/internal/storage/db/couchbase/shared_test.go new file mode 100644 index 000000000..c76506916 --- /dev/null +++ b/internal/storage/db/couchbase/shared_test.go @@ -0,0 +1,124 @@ +package couchbase + +import ( + "encoding/json" + "testing" + + "github.com/authorizerdev/authorizer/internal/refs" + "github.com/authorizerdev/authorizer/internal/storage/schemas" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestStructToDocumentPersistsSecretFields proves the write side no longer drops +// json:"-" fields. gocb Insert/Upsert marshal the document via encoding/json, which +// honors json:"-" and previously omitted User.Password from the stored document. +func TestStructToDocumentPersistsSecretFields(t *testing.T) { + user := &schemas.User{ + ID: "user-1", + Email: refs.NewStringRef("a@b.com"), + Password: refs.NewStringRef("hashed-secret"), + SignupMethods: "basic_auth", + } + + // Baseline: plain json.Marshal (what gocb did before the fix) drops the password. + plain := map[string]interface{}{} + raw, err := json.Marshal(user) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(raw, &plain)) + _, hadPassword := plain["password"] + assert.False(t, hadPassword, "sanity: encoding/json must drop json:\"-\" password (this is the bug)") + + // Fixed path: structToDocument re-adds the password under its bson key. + doc, err := structToDocument(user) + require.NoError(t, err) + require.Contains(t, doc, "password", "structToDocument must persist the password field") + assert.Equal(t, "hashed-secret", refs.StringValue(doc["password"].(*string))) + + // Shape preservation: every key/value the old json.Marshal produced must be + // unchanged; only the previously-dropped "password" key is added. + docJSON, err := json.Marshal(doc) + require.NoError(t, err) + roundTripped := map[string]interface{}{} + require.NoError(t, json.Unmarshal(docJSON, &roundTripped)) + for k, v := range plain { + assert.Equal(t, v, roundTripped[k], "key %q must serialize identically to before", k) + } + delete(roundTripped, "password") + assert.Equal(t, plain, roundTripped, "no keys other than password may be added or changed") +} + +// TestDecodeDocumentPopulatesSecretFields proves the read side no longer drops +// json:"-" fields: Row/One unmarshal via encoding/json, which ignores json:"-". +func TestDecodeDocumentPopulatesSecretFields(t *testing.T) { + // Simulate a document as persisted by structToDocument -> gocb (JSON bytes). + stored := []byte(`{"_id":"user-1","email":"a@b.com","password":"hashed-secret","signup_methods":"basic_auth"}`) + + // Baseline: plain unmarshal drops the password. + var plain schemas.User + require.NoError(t, json.Unmarshal(stored, &plain)) + assert.Nil(t, plain.Password, "sanity: encoding/json must ignore json:\"-\" password on read") + + // Fixed path: decodeDocument populates it from the bson key. + var got schemas.User + require.NoError(t, decodeDocument(stored, &got)) + require.NotNil(t, got.Password) + assert.Equal(t, "hashed-secret", *got.Password) + assert.Equal(t, "a@b.com", refs.StringValue(got.Email)) +} + +// TestSecretFieldFullRoundTrip exercises the exact write->read cycle end to end. +func TestSecretFieldFullRoundTrip(t *testing.T) { + in := &schemas.User{ + ID: "user-1", + Email: refs.NewStringRef("a@b.com"), + Password: refs.NewStringRef("hashed-secret"), + } + doc, err := structToDocument(in) + require.NoError(t, err) + persisted, err := json.Marshal(doc) // gocb Insert serialization + require.NoError(t, err) + + var out schemas.User + require.NoError(t, decodeDocument(persisted, &out)) + require.NotNil(t, out.Password) + assert.Equal(t, *in.Password, *out.Password) +} + +// TestNilSecretFieldRoundTrip ensures a nil password persists as null and decodes +// back to nil (clearable-on-nil semantics must match other providers). +func TestNilSecretFieldRoundTrip(t *testing.T) { + in := &schemas.User{ID: "user-1", Email: refs.NewStringRef("a@b.com")} + doc, err := structToDocument(in) + require.NoError(t, err) + require.Contains(t, doc, "password") + assert.Nil(t, doc["password"]) + + persisted, err := json.Marshal(doc) + require.NoError(t, err) + assert.Contains(t, string(persisted), `"password":null`) + + var out schemas.User + require.NoError(t, decodeDocument(persisted, &out)) + assert.Nil(t, out.Password) +} + +// TestStructToDocumentNoOpForPlainStructs guarantees entities without json:"-" +// fields serialize byte-identically to the previous raw-struct path. +func TestStructToDocumentNoOpForPlainStructs(t *testing.T) { + wh := &schemas.Webhook{ID: "wh-1", EventName: "user.created", EndPoint: "https://x/y", Enabled: true} + + doc, err := structToDocument(wh) + require.NoError(t, err) + got, err := json.Marshal(doc) + require.NoError(t, err) + + want, err := json.Marshal(wh) + require.NoError(t, err) + + // Compare as maps (key order independent). + var gotMap, wantMap map[string]interface{} + require.NoError(t, json.Unmarshal(got, &gotMap)) + require.NoError(t, json.Unmarshal(want, &wantMap)) + assert.Equal(t, wantMap, gotMap, "plain structs must serialize identically to raw json.Marshal") +} diff --git a/internal/storage/db/couchbase/user.go b/internal/storage/db/couchbase/user.go index b40253edb..9191eaed5 100644 --- a/internal/storage/db/couchbase/user.go +++ b/internal/storage/db/couchbase/user.go @@ -2,6 +2,7 @@ package couchbase import ( "context" + "encoding/json" "fmt" "log" "strings" @@ -40,7 +41,11 @@ func (p *provider) AddUser(ctx context.Context, user *schemas.User) (*schemas.Us insertOpt := gocb.InsertOptions{ Context: ctx, } - _, err := p.db.Collection(schemas.Collections.User).Insert(user.ID, user, &insertOpt) + doc, err := structToDocument(user) + if err != nil { + return nil, err + } + _, err = p.db.Collection(schemas.Collections.User).Insert(user.ID, doc, &insertOpt) if err != nil { return nil, err } @@ -53,7 +58,11 @@ func (p *provider) UpdateUser(ctx context.Context, user *schemas.User) (*schemas upsertOpt := gocb.UpsertOptions{ Context: ctx, } - _, err := p.db.Collection(schemas.Collections.User).Upsert(user.ID, user, &upsertOpt) + doc, err := structToDocument(user) + if err != nil { + return nil, err + } + _, err = p.db.Collection(schemas.Collections.User).Upsert(user.ID, doc, &upsertOpt) if err != nil { return nil, err } @@ -91,12 +100,15 @@ func (p *provider) ListUsers(ctx context.Context, pagination *model.Pagination) } paginationClone.Total = total for queryResult.Next() { - var user schemas.User - err := queryResult.Row(&user) - if err != nil { + var raw json.RawMessage + if err := queryResult.Row(&raw); err != nil { + log.Fatal(err) + } + user := &schemas.User{} + if err := decodeDocument(raw, user); err != nil { log.Fatal(err) } - users = append(users, &user) + users = append(users, user) } if err := queryResult.Err(); err != nil { return nil, nil, err @@ -106,7 +118,6 @@ func (p *provider) ListUsers(ctx context.Context, pagination *model.Pagination) // GetUserByEmail to get user information from database using email address func (p *provider) GetUserByEmail(ctx context.Context, email string) (*schemas.User, error) { - var user *schemas.User query := fmt.Sprintf("SELECT _id, email, email_verified_at, `password`, signup_methods, given_name, family_name, middle_name, nickname, birthdate, phone_number, phone_number_verified_at, picture, `roles`, revoked_timestamp, is_multi_factor_auth_enabled, app_data, created_at, updated_at FROM %s.%s WHERE email = $1 LIMIT 1", p.scopeName, schemas.Collections.User) q, err := p.db.Query(query, &gocb.QueryOptions{ ScanConsistency: gocb.QueryScanConsistencyRequestPlus, @@ -116,8 +127,12 @@ func (p *provider) GetUserByEmail(ctx context.Context, email string) (*schemas.U if err != nil { return nil, err } - err = q.One(&user) - if err != nil { + var raw json.RawMessage + if err := q.One(&raw); err != nil { + return nil, err + } + user := &schemas.User{} + if err := decodeDocument(raw, user); err != nil { return nil, err } return user, nil @@ -125,7 +140,6 @@ func (p *provider) GetUserByEmail(ctx context.Context, email string) (*schemas.U // GetUserByID to get user information from database using user ID func (p *provider) GetUserByID(ctx context.Context, id string) (*schemas.User, error) { - var user *schemas.User query := fmt.Sprintf("SELECT _id, email, email_verified_at, `password`, signup_methods, given_name, family_name, middle_name, nickname, birthdate, phone_number, phone_number_verified_at, picture, `roles`, revoked_timestamp, is_multi_factor_auth_enabled, app_data, created_at, updated_at FROM %s.%s WHERE _id = $1 LIMIT 1", p.scopeName, schemas.Collections.User) q, err := p.db.Query(query, &gocb.QueryOptions{ ScanConsistency: gocb.QueryScanConsistencyRequestPlus, @@ -135,8 +149,12 @@ func (p *provider) GetUserByID(ctx context.Context, id string) (*schemas.User, e if err != nil { return nil, err } - err = q.One(&user) - if err != nil { + var raw json.RawMessage + if err := q.One(&raw); err != nil { + return nil, err + } + user := &schemas.User{} + if err := decodeDocument(raw, user); err != nil { return nil, err } return user, nil @@ -178,7 +196,6 @@ func (p *provider) UpdateUsers(ctx context.Context, data map[string]interface{}, // GetUserByPhoneNumber to get user information from database using phone number func (p *provider) GetUserByPhoneNumber(ctx context.Context, phoneNumber string) (*schemas.User, error) { - var user *schemas.User query := fmt.Sprintf("SELECT _id, email, email_verified_at, `password`, signup_methods, given_name, family_name, middle_name, nickname, birthdate, phone_number, phone_number_verified_at, picture, `roles`, revoked_timestamp, is_multi_factor_auth_enabled, app_data, created_at, updated_at FROM %s.%s WHERE phone_number = $1 LIMIT 1", p.scopeName, schemas.Collections.User) q, err := p.db.Query(query, &gocb.QueryOptions{ ScanConsistency: gocb.QueryScanConsistencyRequestPlus, @@ -188,8 +205,12 @@ func (p *provider) GetUserByPhoneNumber(ctx context.Context, phoneNumber string) if err != nil { return nil, err } - err = q.One(&user) - if err != nil { + var raw json.RawMessage + if err := q.One(&raw); err != nil { + return nil, err + } + user := &schemas.User{} + if err := decodeDocument(raw, user); err != nil { return nil, err } return user, nil diff --git a/internal/storage/db/couchbase/verification_requests.go b/internal/storage/db/couchbase/verification_requests.go index 582af178b..0866179a5 100644 --- a/internal/storage/db/couchbase/verification_requests.go +++ b/internal/storage/db/couchbase/verification_requests.go @@ -24,7 +24,11 @@ func (p *provider) AddVerificationRequest(ctx context.Context, verificationReque insertOpt := gocb.InsertOptions{ Context: ctx, } - _, err := p.db.Collection(schemas.Collections.VerificationRequest).Insert(verificationRequest.ID, verificationRequest, &insertOpt) + doc, err := structToDocument(verificationRequest) + if err != nil { + return nil, err + } + _, err = p.db.Collection(schemas.Collections.VerificationRequest).Insert(verificationRequest.ID, doc, &insertOpt) if err != nil { return nil, err } diff --git a/internal/storage/db/couchbase/webhook.go b/internal/storage/db/couchbase/webhook.go index f4678decc..75da2854d 100644 --- a/internal/storage/db/couchbase/webhook.go +++ b/internal/storage/db/couchbase/webhook.go @@ -28,7 +28,11 @@ func (p *provider) AddWebhook(ctx context.Context, webhook *schemas.Webhook) (*s insertOpt := gocb.InsertOptions{ Context: ctx, } - _, err := p.db.Collection(schemas.Collections.Webhook).Insert(webhook.ID, webhook, &insertOpt) + doc, err := structToDocument(webhook) + if err != nil { + return nil, err + } + _, err = p.db.Collection(schemas.Collections.Webhook).Insert(webhook.ID, doc, &insertOpt) if err != nil { return nil, err } diff --git a/internal/storage/db/couchbase/webhook_log.go b/internal/storage/db/couchbase/webhook_log.go index 883347c34..013f59057 100644 --- a/internal/storage/db/couchbase/webhook_log.go +++ b/internal/storage/db/couchbase/webhook_log.go @@ -24,7 +24,11 @@ func (p *provider) AddWebhookLog(ctx context.Context, webhookLog *schemas.Webhoo insertOpt := gocb.InsertOptions{ Context: ctx, } - _, err := p.db.Collection(schemas.Collections.WebhookLog).Insert(webhookLog.ID, webhookLog, &insertOpt) + doc, err := structToDocument(webhookLog) + if err != nil { + return nil, err + } + _, err = p.db.Collection(schemas.Collections.WebhookLog).Insert(webhookLog.ID, doc, &insertOpt) if err != nil { return nil, err } diff --git a/internal/storage/provider_test.go b/internal/storage/provider_test.go index 8e28a8cc2..a3ea00517 100644 --- a/internal/storage/provider_test.go +++ b/internal/storage/provider_test.go @@ -204,6 +204,13 @@ func testUserOperations(t *testing.T, ctx context.Context, provider Provider) { require.NotNil(t, fetchedUser) assert.Equal(t, user.Email, fetchedUser.Email) + // Password is tagged json:"-" for API safety but must still persist to and load + // from the database. Couchbase (de)serializes structs via encoding/json, which + // honors json:"-" and previously dropped the password entirely. + // Regression guard: fix/couchbase-secret-json-tag-persistence. + require.NotNil(t, fetchedUser.Password, "stored password must round-trip from the database") + assert.Equal(t, *user.Password, *fetchedUser.Password) + // Test UpdateUser fetchedUser.GivenName = refs.NewStringRef("Updated") updatedUser, err := provider.UpdateUser(ctx, fetchedUser) From c00db321e7be2b6261f71506a548fbb82dc4310a Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Mon, 6 Jul 2026 16:59:10 +0530 Subject: [PATCH 2/3] fix(auth): guard nil password hash in login to prevent panic CompareHashAndPassword dereferenced user.Password unconditionally. A basic_auth user with no persisted hash (e.g. a pre-fix Couchbase record, since that provider silently dropped json:"-" fields on write) would nil-pointer-panic instead of failing the login cleanly. Treats a nil hash the same as a wrong password, using the existing dummy-bcrypt timing-equalisation helper. --- internal/integration_tests/login_test.go | 29 ++++++++++++++++++++++++ internal/service/login.go | 10 +++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/internal/integration_tests/login_test.go b/internal/integration_tests/login_test.go index 8d4c11fa6..a5d14ea62 100644 --- a/internal/integration_tests/login_test.go +++ b/internal/integration_tests/login_test.go @@ -165,6 +165,35 @@ func TestLogin(t *testing.T) { assert.Nil(t, res) }) + t.Run("should fail cleanly (not panic) when stored password hash is nil", func(t *testing.T) { + noHashEmail := "no_password_hash_" + uuid.New().String() + "@authorizer.dev" + signupReq3 := &model.SignUpRequest{ + Email: &noHashEmail, + Password: password, + ConfirmPassword: password, + } + signupRes, err := ts.GraphQLProvider.SignUp(ctx, signupReq3) + assert.NoError(t, err) + assert.NotNil(t, signupRes) + + // Simulate a record whose password hash was never persisted (e.g. the + // pre-fix Couchbase json:"-" bug) by nulling it out directly. + user, err := ts.StorageProvider.GetUserByEmail(ctx, noHashEmail) + assert.NoError(t, err) + user.Password = nil + _, err = ts.StorageProvider.UpdateUser(ctx, user) + assert.NoError(t, err) + + loginReq := &model.LoginRequest{ + Email: &noHashEmail, + Password: password, + } + res, err := ts.GraphQLProvider.Login(ctx, loginReq) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid credentials") + assert.Nil(t, res) + }) + t.Run("mobile login", func(t *testing.T) { mobile := fmt.Sprintf("%d", time.Now().Add(10*time.Second).Unix()) signUpReq := &model.SignUpRequest{ diff --git a/internal/service/login.go b/internal/service/login.go index bbe73c5ed..e547e52d1 100644 --- a/internal/service/login.go +++ b/internal/service/login.go @@ -246,7 +246,15 @@ func (p *provider) Login(ctx context.Context, meta RequestMetadata, params *mode } } } - err = bcrypt.CompareHashAndPassword([]byte(*user.Password), []byte(params.Password)) + if user.Password == nil { + // A basic_auth user with no stored hash (e.g. a pre-fix Couchbase + // record that never persisted one) must fail the same way as a + // wrong password, not nil-pointer-dereference. + loginPerformDummyPasswordCheck(params.Password) + err = bcrypt.ErrMismatchedHashAndPassword + } else { + err = bcrypt.CompareHashAndPassword([]byte(*user.Password), []byte(params.Password)) + } if err != nil { log.Debug().Str("reason", "bad_password").Msg("login failed") metrics.RecordAuthEvent(metrics.EventLogin, metrics.StatusFailure) From 5186050e69c45ee429bd8da32f510fce22bfa76f Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Mon, 6 Jul 2026 17:05:42 +0530 Subject: [PATCH 3/3] fix(couchbase): return decode errors from ListUsers instead of log.Fatal log.Fatal calls os.Exit(1), so one malformed row would crash the whole server on an admin list call. Flagged independently by both review passes on this PR since it now also covers decodeDocument failures. --- internal/storage/db/couchbase/user.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/storage/db/couchbase/user.go b/internal/storage/db/couchbase/user.go index 9191eaed5..04cb32fc6 100644 --- a/internal/storage/db/couchbase/user.go +++ b/internal/storage/db/couchbase/user.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "log" "strings" "time" @@ -102,11 +101,11 @@ func (p *provider) ListUsers(ctx context.Context, pagination *model.Pagination) for queryResult.Next() { var raw json.RawMessage if err := queryResult.Row(&raw); err != nil { - log.Fatal(err) + return nil, nil, err } user := &schemas.User{} if err := decodeDocument(raw, user); err != nil { - log.Fatal(err) + return nil, nil, err } users = append(users, user) }