From 8789561b607b8f96e2ebc3042d2e66532f91d62a Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 26 Sep 2026 12:52:20 -0400 Subject: [PATCH] fix(api): consolidate grant deletes and harden API v1 auth after review - GrantRepository keeps three deletes: DeleteForUser, DeleteStaleEpochs (replaces DeleteOtherEpochs and DeleteIfEpoch) and DeleteIdle. Delete(id) is gone; the idle path in ResolveGrant now calls DeleteIdle, so a grant renewed by another node between the read and the delete survives. settleEpoch deletes the user's grants below the snapshot's epoch, which is safe outside the transaction because epochs only move forward. - The "dead grants on an older epoch are only deleted when presented" policy note moves from the repository to core ListGrants. - SQL trace logging no longer prints the args of property writes (signing keys) or user password writes, both encrypted with a key that may be the public default. The SQL statement is still logged. - The spec gate rejects JSON body keys that differ from a declared property only in case. kin-openapi validates exact names while encoding/json decodes case-insensitively, so {"scopes":[],"Scopes":null} minted a token with every scope and a "Client" key skipped maxLength. It also rejects data after the first JSON value, which the handlers' decoder ignores and which let a body skip the alias check. - The liveness cache trims its eviction log on evict, not only on put, so evict-only traffic stays bounded; the floor still drops stale fills. - createAccessToken, login and setupFirstAdmin declare Cache-Control: no-store on their success responses. --- api/bundled/openapi.json | 24 ++++++ api/bundled/openapi.yaml | 15 ++++ .../headers/CacheControlNoStore.yaml | 4 + api/openapi/paths/auth.yaml | 9 ++ core/apiauth/cache.go | 8 +- core/apiauth/cache_test.go | 16 ++++ core/apiauth/service.go | 24 +++--- core/apiauth/service_session_test.go | 6 +- core/apiauth/service_test.go | 41 +++++++++- model/grant.go | 4 +- persistence/grant_repository.go | 13 +-- persistence/grant_repository_test.go | 45 ++++++---- persistence/persistence_suite_test.go | 14 ++++ persistence/property_repository.go | 2 + persistence/property_repository_test.go | 13 +++ persistence/sql_base_repository.go | 21 ++++- persistence/user_repository.go | 3 +- persistence/user_repository_test.go | 22 +++++ server/apiv1/api_gen.go | 42 ++++++++-- server/apiv1/api_test.go | 24 ++++++ server/apiv1/apiv1_suite_test.go | 15 +++- server/apiv1/auth_handlers.go | 4 +- server/apiv1/auth_test.go | 48 ++++++++++- server/apiv1/gate.go | 82 ++++++++++++++++++- server/apiv1/gate_test.go | 62 +++++++++++++- server/apiv1/password_handlers.go | 4 +- 26 files changed, 493 insertions(+), 72 deletions(-) create mode 100644 api/openapi/components/headers/CacheControlNoStore.yaml diff --git a/api/bundled/openapi.json b/api/bundled/openapi.json index c46fef1a7..f3af48f50 100644 --- a/api/bundled/openapi.json +++ b/api/bundled/openapi.json @@ -151,6 +151,11 @@ "responses": { "200": { "description": "The new access token.", + "headers": { + "Cache-Control": { + "$ref": "#/components/headers/CacheControlNoStore" + } + }, "content": { "application/json": { "schema": { @@ -372,6 +377,11 @@ "responses": { "200": { "description": "The new grant.", + "headers": { + "Cache-Control": { + "$ref": "#/components/headers/CacheControlNoStore" + } + }, "content": { "application/json": { "schema": { @@ -423,6 +433,11 @@ "responses": { "201": { "description": "The admin was created.", + "headers": { + "Cache-Control": { + "$ref": "#/components/headers/CacheControlNoStore" + } + }, "content": { "application/json": { "schema": { @@ -1137,6 +1152,15 @@ "schema": { "type": "string" } + }, + "CacheControlNoStore": { + "description": "Always `no-store`, because the response carries a secret or a token.", + "schema": { + "type": "string", + "enum": [ + "no-store" + ] + } } } } diff --git a/api/bundled/openapi.yaml b/api/bundled/openapi.yaml index 5b07e614d..2134268eb 100644 --- a/api/bundled/openapi.yaml +++ b/api/bundled/openapi.yaml @@ -108,6 +108,9 @@ paths: responses: '200': description: The new access token. + headers: + Cache-Control: + $ref: '#/components/headers/CacheControlNoStore' content: application/json: schema: @@ -243,6 +246,9 @@ paths: responses: '200': description: The new grant. + headers: + Cache-Control: + $ref: '#/components/headers/CacheControlNoStore' content: application/json: schema: @@ -276,6 +282,9 @@ paths: responses: '201': description: The admin was created. + headers: + Cache-Control: + $ref: '#/components/headers/CacheControlNoStore' content: application/json: schema: @@ -793,3 +802,9 @@ components: description: Entity tag for `If-None-Match` revalidation. schema: type: string + CacheControlNoStore: + description: Always `no-store`, because the response carries a secret or a token. + schema: + type: string + enum: + - no-store diff --git a/api/openapi/components/headers/CacheControlNoStore.yaml b/api/openapi/components/headers/CacheControlNoStore.yaml new file mode 100644 index 000000000..092a03fda --- /dev/null +++ b/api/openapi/components/headers/CacheControlNoStore.yaml @@ -0,0 +1,4 @@ +description: Always `no-store`, because the response carries a secret or a token. +schema: + type: string + enum: [no-store] diff --git a/api/openapi/paths/auth.yaml b/api/openapi/paths/auth.yaml index 5f7e3d58d..b6d8c1d9c 100644 --- a/api/openapi/paths/auth.yaml +++ b/api/openapi/paths/auth.yaml @@ -17,6 +17,9 @@ token: responses: '200': description: The new access token. + headers: + Cache-Control: + $ref: ../components/headers/CacheControlNoStore.yaml content: application/json: schema: @@ -130,6 +133,9 @@ login: responses: '200': description: The new grant. + headers: + Cache-Control: + $ref: ../components/headers/CacheControlNoStore.yaml content: application/json: schema: @@ -163,6 +169,9 @@ setup: responses: '201': description: The admin was created. + headers: + Cache-Control: + $ref: ../components/headers/CacheControlNoStore.yaml content: application/json: schema: diff --git a/core/apiauth/cache.go b/core/apiauth/cache.go index d19d53dd5..ab4aaa1e1 100644 --- a/core/apiauth/cache.go +++ b/core/apiauth/cache.go @@ -62,10 +62,6 @@ func (c *livenessCache) put(id string, e livenessEntry, now time.Time, started u break } } - if len(c.evicted) >= maxLivenessEntries { - clear(c.evicted) - c.floor = c.gen - } e.expires = now.Add(c.ttl) c.entries[id] = e } @@ -73,6 +69,10 @@ func (c *livenessCache) put(id string, e livenessEntry, now time.Time, started u func (c *livenessCache) evict(id string) { c.mu.Lock() defer c.mu.Unlock() + if len(c.evicted) >= maxLivenessEntries { + clear(c.evicted) + c.floor = c.gen + } c.gen++ c.evicted[id] = c.gen delete(c.entries, id) diff --git a/core/apiauth/cache_test.go b/core/apiauth/cache_test.go index a3429897d..fde2066f5 100644 --- a/core/apiauth/cache_test.go +++ b/core/apiauth/cache_test.go @@ -65,6 +65,22 @@ var _ = Describe("livenessCache", func() { Expect(ok).To(BeFalse()) }) + It("stays bounded under eviction-only traffic and still drops a fill started before the trim", func() { + started := c.begin() + c.evict("g1") + for i := range 3 * maxLivenessEntries { + c.evict(fmt.Sprint("other", i)) + } + Expect(len(c.evicted)).To(BeNumerically("<=", maxLivenessEntries)) + + c.put("g1", livenessEntry{userID: "u1"}, t0, started) + _, ok := c.get("g1", t0) + Expect(ok).To(BeFalse()) + c.put("g1", livenessEntry{userID: "u1"}, t0, c.begin()) + _, ok = c.get("g1", t0) + Expect(ok).To(BeTrue()) + }) + It("records the last use without extending the TTL", func() { c.put("g1", livenessEntry{userID: "u1"}, t0, c.begin()) c.markUsed("g1", t0.Add(10*time.Second)) diff --git a/core/apiauth/service.go b/core/apiauth/service.go index f16fef5fb..08dc53332 100644 --- a/core/apiauth/service.go +++ b/core/apiauth/service.go @@ -155,8 +155,8 @@ func (s *Service) ResolveGrant(ctx context.Context, secret, ip string) (*Princip if err != nil { return nil, err } - if !s.now().Before(g.LastActivity().Add(IdleExpiry)) { - s.dropGrant(ctx, g.ID) + if idleSince := s.now().Add(-IdleExpiry); g.LastActivity().Before(idleSince) { + s.dropIdle(ctx, g.ID, idleSince) return nil, model.ErrInvalidAuth } u, err := s.loadUser(ctx, g.UserID) @@ -194,16 +194,17 @@ func (s *Service) loadUser(ctx context.Context, userID string) (*model.User, err return u, err } -// dropGrant evicts after deleting, so a concurrent fill cannot re-cache the dead grant. -func (s *Service) dropGrant(ctx context.Context, id string) { - if err := s.ds.Grant().Delete(ctx, id); err != nil { - log.Warn(ctx, "API v1: could not delete dead grant", "grant", id, err) +// dropIdle deletes only still-idle grants, sparing one renewed meanwhile, and evicts after deleting so a +// concurrent fill cannot re-cache the dead grant. +func (s *Service) dropIdle(ctx context.Context, id string, idleSince time.Time) { + if _, err := s.ds.Grant().DeleteIdle(ctx, idleSince); err != nil { + log.Warn(ctx, "API v1: could not delete idle grants", "grant", id, err) } s.cache.evict(id) } -// settleEpoch re-reads grant and user in one read transaction: separate reads can straddle a password -// change and make a kept grant look dead. The delete only fires while the grant is on the epoch seen here. +// settleEpoch re-reads grant and user in one read transaction: separate reads can straddle a password change +// and make a kept grant look dead. Deleting below the snapshot's epoch is safe: a later change only moves kept grants up. func (s *Service) settleEpoch(ctx context.Context, grantID string) (*model.Grant, *model.User, error) { var g *model.Grant var u *model.User @@ -224,8 +225,8 @@ func (s *Service) settleEpoch(ctx context.Context, grantID string) (*model.Grant } if g.UserEpoch != u.TokenEpoch { s.cache.evict(grantID) - if err := s.ds.Grant().DeleteIfEpoch(ctx, grantID, g.UserEpoch); err != nil { - log.Warn(ctx, "API v1: could not delete dead grant", "grant", grantID, err) + if err := s.ds.Grant().DeleteStaleEpochs(ctx, u.ID, u.TokenEpoch); err != nil { + log.Warn(ctx, "API v1: could not delete the user's grants from older epochs", "user", u.ID, "grant", grantID, err) } return nil, nil, model.ErrInvalidAuth } @@ -297,6 +298,7 @@ func (s *Service) liveGrant(ctx context.Context, id string, u *model.User) (live return e, u, nil } +// ListGrants shows only the current epoch: grants left on an older one are dead but only deleted when presented. func (s *Service) ListGrants(ctx context.Context, p *Principal, offset, limit int) (model.Grants, int64, error) { idleSince := s.now().Add(-IdleExpiry) grants, err := s.ds.Grant().GetAllForUser(ctx, p.User.ID, p.User.TokenEpoch, idleSince, offset, limit) @@ -374,6 +376,6 @@ func (s *Service) ChangePassword(ctx context.Context, p *Principal, current, new if err := tx.Grant().SetEpoch(ctx, u.ID, oldEpoch, updated.TokenEpoch, keep); err != nil { return err } - return tx.Grant().DeleteOtherEpochs(ctx, u.ID, updated.TokenEpoch) + return tx.Grant().DeleteStaleEpochs(ctx, u.ID, updated.TokenEpoch) }) } diff --git a/core/apiauth/service_session_test.go b/core/apiauth/service_session_test.go index 1a83d2ee4..573173402 100644 --- a/core/apiauth/service_session_test.go +++ b/core/apiauth/service_session_test.go @@ -63,7 +63,7 @@ var _ = Describe("Service: sessions", func() { _, err := svc.Authenticate(ctx, tok.Token, "") Expect(err).ToNot(HaveOccurred()) - Expect(realDS.Grant().Delete(ctx, issued.Grant.ID)).To(Succeed()) // another node + Expect(realDS.Grant().DeleteForUser(ctx, u.ID, issued.Grant.ID)).To(Succeed()) // another node _, err = svc.Authenticate(ctx, tok.Token, "") Expect(err).ToNot(HaveOccurred()) // still cached now = now.Add(cacheTTL) @@ -223,7 +223,7 @@ var _ = Describe("Service: sessions", func() { _, p, tok := login(ctx, svc, u) _, err := svc.Authenticate(ctx, tok.Token, "") Expect(err).ToNot(HaveOccurred()) - Expect(realDS.Grant().Delete(ctx, p.GrantID)).To(Succeed()) // another node + Expect(realDS.Grant().DeleteForUser(ctx, u.ID, p.GrantID)).To(Succeed()) // another node Expect(svc.Logout(ctx, p)).To(Succeed()) _, err = svc.Authenticate(ctx, tok.Token, "") @@ -302,7 +302,7 @@ var _ = Describe("Service: sessions", func() { It("rejects a caller whose grant was revoked before the change ran", func() { u := createUser(ctx, "pw", false) _, p, _ := login(ctx, svc, u) - Expect(realDS.Grant().Delete(ctx, p.GrantID)).To(Succeed()) + Expect(realDS.Grant().DeleteForUser(ctx, u.ID, p.GrantID)).To(Succeed()) err := svc.ChangePassword(request.WithUser(ctx, p.User), p, "pw", "pw2", true) Expect(err).To(MatchError(model.ErrInvalidAuth)) _, err = svc.Login(ctx, u.UserName, "pw", meta, nil) diff --git a/core/apiauth/service_test.go b/core/apiauth/service_test.go index ce6622914..03469e486 100644 --- a/core/apiauth/service_test.go +++ b/core/apiauth/service_test.go @@ -38,6 +38,26 @@ func (p *flakyProps) Get(ctx context.Context, id string) (string, error) { return p.PropertyRepository.Get(ctx, id) } +// renewingDS runs renew right before DeleteIdle, as a node resolving the grant meanwhile would. +type renewingDS struct { + model.DataStore + renew func() +} + +func (d renewingDS) Grant() model.GrantRepository { + return renewingGrants{GrantRepository: d.DataStore.Grant(), renew: d.renew} +} + +type renewingGrants struct { + model.GrantRepository + renew func() +} + +func (g renewingGrants) DeleteIdle(ctx context.Context, idleSince time.Time) (int64, error) { + g.renew() + return g.GrantRepository.DeleteIdle(ctx, idleSince) +} + var meta = ClientMeta{Name: "Living room", Client: "TestApp", ClientVersion: "1.0"} var _ = Describe("Service: grants and tokens", func() { @@ -174,13 +194,32 @@ var _ = Describe("Service: grants and tokens", func() { Expect(err).To(MatchError(model.ErrNotFound)) }) - It("rejects a grant whose epoch is behind the user's", func() { + It("keeps an idle grant that another node renewed before the delete ran", func() { + u := createUser(ctx, "pw", false) + issued, _ := svc.Login(ctx, u.UserName, "pw", meta, nil) + renewedAt := now.Add(IdleExpiry - time.Minute) + racing := New(renewingDS{DataStore: realDS, renew: func() { + Expect(realDS.Grant().Touch(ctx, issued.Grant.ID, "10.0.0.2", renewedAt, renewedAt)).To(Succeed()) + }}) + now = now.Add(IdleExpiry + time.Second) + racing.SetClock(func() time.Time { return now }) + + _, err := racing.ResolveGrant(ctx, issued.Secret, "") + Expect(err).To(MatchError(model.ErrInvalidAuth)) + g, err := realDS.Grant().Get(ctx, issued.Grant.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(g.LastUsedIP).To(Equal("10.0.0.2")) + }) + + It("rejects and deletes a grant whose epoch is behind the user's", func() { u := createUser(ctx, "pw", false) issued, _ := svc.Login(ctx, u.UserName, "pw", meta, nil) u.NewPassword = "changed-elsewhere" Expect(realDS.User().Put(ctx, &u)).To(Succeed()) _, err := svc.ResolveGrant(ctx, issued.Secret, "") Expect(err).To(MatchError(model.ErrInvalidAuth)) + _, err = realDS.Grant().Get(ctx, issued.Grant.ID) + Expect(err).To(MatchError(model.ErrNotFound)) }) }) diff --git a/model/grant.go b/model/grant.go index 1e8fe46e1..ecbf30e41 100644 --- a/model/grant.go +++ b/model/grant.go @@ -59,11 +59,9 @@ type GrantRepository interface { FindBySecretHash(ctx context.Context, hash string) (*Grant, error) GetAllForUser(ctx context.Context, userID string, epoch int, idleSince time.Time, offset, limit int) (Grants, error) CountForUser(ctx context.Context, userID string, epoch int, idleSince time.Time) (int64, error) - Delete(ctx context.Context, id string) error DeleteForUser(ctx context.Context, userID, id string) error - DeleteOtherEpochs(ctx context.Context, userID string, epoch int) error + DeleteStaleEpochs(ctx context.Context, userID string, currentEpoch int) error SetEpoch(ctx context.Context, userID string, fromEpoch, toEpoch int, onlyID string) error - DeleteIfEpoch(ctx context.Context, id string, epoch int) error Touch(ctx context.Context, id, ip string, at, notSince time.Time) error DeleteIdle(ctx context.Context, idleSince time.Time) (int64, error) } diff --git a/persistence/grant_repository.go b/persistence/grant_repository.go index 0e561315c..349099983 100644 --- a/persistence/grant_repository.go +++ b/persistence/grant_repository.go @@ -60,7 +60,6 @@ func (r *grantRepository) findOne(ctx context.Context, cond Sqlizer) (*model.Gra return &g, nil } -// activeForUser skips grants left on an older epoch: they are dead but only deleted when presented. func activeForUser(userID string, epoch int, idleSince time.Time) Sqlizer { return And{Eq{"user_id": userID, "user_epoch": epoch}, Expr(grantLastActivity+" >= ?", idleSince.UTC())} } @@ -78,10 +77,6 @@ func (r *grantRepository) CountForUser(ctx context.Context, userID string, epoch return r.count(ctx, Select().Where(activeForUser(userID, epoch, idleSince))) } -func (r *grantRepository) Delete(ctx context.Context, id string) error { - return r.delete(ctx, Eq{"id": id}) -} - func (r *grantRepository) DeleteForUser(ctx context.Context, userID, id string) error { n, err := r.executeSQL(ctx, Delete(r.tableName).Where(Eq{"id": id, "user_id": userID})) if err != nil { @@ -93,8 +88,8 @@ func (r *grantRepository) DeleteForUser(ctx context.Context, userID, id string) return nil } -func (r *grantRepository) DeleteOtherEpochs(ctx context.Context, userID string, epoch int) error { - return r.delete(ctx, And{Eq{"user_id": userID}, NotEq{"user_epoch": epoch}}) +func (r *grantRepository) DeleteStaleEpochs(ctx context.Context, userID string, currentEpoch int) error { + return r.delete(ctx, And{Eq{"user_id": userID}, Lt{"user_epoch": currentEpoch}}) } // SetEpoch only moves grants still on fromEpoch, so grants killed by an earlier change never come back. @@ -107,10 +102,6 @@ func (r *grantRepository) SetEpoch(ctx context.Context, userID string, fromEpoch return err } -func (r *grantRepository) DeleteIfEpoch(ctx context.Context, id string, epoch int) error { - return r.delete(ctx, Eq{"id": id, "user_epoch": epoch}) -} - func (r *grantRepository) Touch(ctx context.Context, id, ip string, at, notSince time.Time) error { upd := Update(r.tableName).Set("last_used_at", at.UTC()).Set("last_used_ip", ip). Where(And{Eq{"id": id}, Or{Eq{"last_used_at": nil}, Lt{"last_used_at": notSince.UTC()}}}) diff --git a/persistence/grant_repository_test.go b/persistence/grant_repository_test.go index bb140ebd6..2436f8bb4 100644 --- a/persistence/grant_repository_test.go +++ b/persistence/grant_repository_test.go @@ -96,20 +96,18 @@ var _ = Describe("GrantRepository", func() { Expect(err).To(MatchError(model.ErrNotFound)) }) - It("moves epochs forward and deletes grants left on other epochs", func() { + It("moves epochs forward", func() { keep := newGrant(adminUser.ID, "h-keep") - drop := newGrant(adminUser.ID, "h-drop") + stay := newGrant(adminUser.ID, "h-stay") Expect(repo.Put(ctx, keep)).To(Succeed()) - Expect(repo.Put(ctx, drop)).To(Succeed()) + Expect(repo.Put(ctx, stay)).To(Succeed()) Expect(repo.SetEpoch(ctx, adminUser.ID, 0, 3, keep.ID)).To(Succeed()) - Expect(repo.DeleteOtherEpochs(ctx, adminUser.ID, 3)).To(Succeed()) - kept, err := repo.Get(ctx, keep.ID) Expect(err).ToNot(HaveOccurred()) Expect(kept.UserEpoch).To(Equal(3)) - _, err = repo.Get(ctx, drop.ID) - Expect(err).To(MatchError(model.ErrNotFound)) + stayed, _ := repo.Get(ctx, stay.ID) + Expect(stayed.UserEpoch).To(Equal(0)) Expect(repo.SetEpoch(ctx, adminUser.ID, 3, 4, "")).To(Succeed()) kept, _ = repo.Get(ctx, keep.ID) @@ -131,16 +129,29 @@ var _ = Describe("GrantRepository", func() { Expect(got.UserEpoch).To(Equal(3)) }) - It("deletes by epoch only while the row is still on it", func() { - g := newGrant(adminUser.ID, "h-cond") - g.UserEpoch = 5 - Expect(repo.Put(ctx, g)).To(Succeed()) - Expect(repo.DeleteIfEpoch(ctx, g.ID, 4)).To(Succeed()) - _, err := repo.Get(ctx, g.ID) - Expect(err).ToNot(HaveOccurred()) - Expect(repo.DeleteIfEpoch(ctx, g.ID, 5)).To(Succeed()) - _, err = repo.Get(ctx, g.ID) - Expect(err).To(MatchError(model.ErrNotFound)) + It("deletes only the user's grants on an epoch before the current one", func() { + older := newGrant(adminUser.ID, "h-older") + older.UserEpoch = 1 + previous := newGrant(adminUser.ID, "h-previous") + previous.UserEpoch = 4 + current := newGrant(adminUser.ID, "h-current") + current.UserEpoch = 5 + otherUser := newGrant(regularUser.ID, "h-other-user") + otherUser.UserEpoch = 1 + for _, g := range []*model.Grant{older, previous, current, otherUser} { + Expect(repo.Put(ctx, g)).To(Succeed()) + } + + Expect(repo.DeleteStaleEpochs(ctx, adminUser.ID, 5)).To(Succeed()) + + for _, g := range []*model.Grant{older, previous} { + _, err := repo.Get(ctx, g.ID) + Expect(err).To(MatchError(model.ErrNotFound)) + } + for _, g := range []*model.Grant{current, otherUser} { + _, err := repo.Get(ctx, g.ID) + Expect(err).ToNot(HaveOccurred()) + } }) It("touches a never-used grant, then throttles until notSince passes", func() { diff --git a/persistence/persistence_suite_test.go b/persistence/persistence_suite_test.go index ee2794454..dc6134793 100644 --- a/persistence/persistence_suite_test.go +++ b/persistence/persistence_suite_test.go @@ -1,7 +1,9 @@ package persistence import ( + "bytes" "context" + "os" "path/filepath" "testing" "time" @@ -348,6 +350,18 @@ var _ = BeforeSuite(func() { } }) +// captureTraceLogs sends trace logs, SQL included, to a buffer for the rest of the spec. +func captureTraceLogs() *bytes.Buffer { + buf := &bytes.Buffer{} + log.SetOutput(buf) + log.SetLevel(log.LevelTrace) + DeferCleanup(func() { + log.SetOutput(os.Stderr) + log.SetLevel(log.LevelFatal) + }) + return buf +} + func GetDBXBuilder() *dbx.DB { return dbx.NewFromDB(db.Db(), db.Dialect) } diff --git a/persistence/property_repository.go b/persistence/property_repository.go index 87d97491c..916b16430 100644 --- a/persistence/property_repository.go +++ b/persistence/property_repository.go @@ -21,6 +21,7 @@ func NewPropertyRepository(db dbx.Builder) model.PropertyRepository { } func (r propertyRepository) Put(ctx context.Context, id string, value string) error { + ctx = withSecretArgs(ctx) update := Update(r.tableName).Set("value", value).Where(Eq{"id": id}) count, err := r.executeSQL(ctx, update) if err != nil { @@ -35,6 +36,7 @@ func (r propertyRepository) Put(ctx context.Context, id string, value string) er } func (r propertyRepository) PutIfAbsent(ctx context.Context, id string, value string) error { + ctx = withSecretArgs(ctx) insert := Insert(r.tableName).Columns("id", "value").Values(id, value).Options("OR IGNORE") _, err := r.executeSQL(ctx, insert) return err diff --git a/persistence/property_repository_test.go b/persistence/property_repository_test.go index 0f4f9ebe5..f80fa0b36 100644 --- a/persistence/property_repository_test.go +++ b/persistence/property_repository_test.go @@ -39,4 +39,17 @@ var _ = Describe("Property Repository", func() { Expect(pr.PutIfAbsent(ctx, "pia", "second")).To(Succeed()) Expect(pr.Get(ctx, "pia")).To(Equal("first")) }) + + It("never logs the values it writes, but still logs the SQL", func() { + logs := captureTraceLogs() + Expect(pr.Put(ctx, "secret-prop", "inserted-secret")).To(Succeed()) + Expect(pr.Put(ctx, "secret-prop", "updated-secret")).To(Succeed()) + Expect(pr.PutIfAbsent(ctx, "secret-prop-2", "absent-secret")).To(Succeed()) + + Expect(logs.String()).To(ContainSubstring("INSERT INTO property")) + Expect(logs.String()).To(ContainSubstring("UPDATE property")) + Expect(logs.String()).ToNot(ContainSubstring("inserted-secret")) + Expect(logs.String()).ToNot(ContainSubstring("updated-secret")) + Expect(logs.String()).ToNot(ContainSubstring("absent-secret")) + }) }) diff --git a/persistence/sql_base_repository.go b/persistence/sql_base_repository.go index 03cc6a01b..d929e7ef4 100644 --- a/persistence/sql_base_repository.go +++ b/persistence/sql_base_repository.go @@ -658,9 +658,28 @@ func (r sqlRepository) deleteByID(ctx context.Context, id string) error { return nil } +type secretArgsKey struct{} + +// withSecretArgs hides a statement's args from the SQL log: stored secrets are encrypted with a key that may be the public default. +func withSecretArgs(ctx context.Context) context.Context { + return context.WithValue(ctx, secretArgsKey{}, true) +} + +func hasSecretArgs(ctx context.Context) bool { + if ctx == nil { + return false + } + secret, _ := ctx.Value(secretArgsKey{}).(bool) + return secret +} + func (r sqlRepository) logSQL(ctx context.Context, sql string, args dbx.Params, err error, rowsAffected int64, start time.Time) { elapsed := time.Since(start) - fields := []any{ctx, "SQL: `" + sql + "`", "args", args, "rowsAffected", rowsAffected, "elapsedTime", elapsed} + var logArgs any = args + if hasSecretArgs(ctx) { + logArgs = "[REDACTED]" + } + fields := []any{ctx, "SQL: `" + sql + "`", "args", logArgs, "rowsAffected", rowsAffected, "elapsedTime", elapsed} if err == nil || errors.Is(err, context.Canceled) { log.Trace(append(fields, err)...) return diff --git a/persistence/user_repository.go b/persistence/user_repository.go index 20b4e5125..a750bbef4 100644 --- a/persistence/user_repository.go +++ b/persistence/user_repository.go @@ -118,6 +118,7 @@ func (r *userRepository) Put(ctx context.Context, u *model.User) error { } u.UpdatedAt = time.Now() if u.NewPassword != "" { + ctx = withSecretArgs(ctx) _ = r.encryptPassword(ctx, u) } values, err := toSQLArgs(*u) @@ -432,7 +433,7 @@ func (r *userRepository) initPasswordEncryptionKey(ctx context.Context) error { u.NewPassword = u.Password if err := r.encryptPassword(ctx, &u); err == nil { upd := Update(r.tableName).Set("password", u.NewPassword).Where(Eq{"id": u.ID}) - _, err = r.executeSQL(ctx, upd) + _, err = r.executeSQL(withSecretArgs(ctx), upd) if err != nil { log.Error("Password NOT encrypted! This may cause problems!", "user", u.UserName, "id", u.ID, err) } else { diff --git a/persistence/user_repository_test.go b/persistence/user_repository_test.go index 0e776fc3a..4f907b447 100644 --- a/persistence/user_repository_test.go +++ b/persistence/user_repository_test.go @@ -17,6 +17,7 @@ import ( "github.com/navidrome/navidrome/utils/slice" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/pocketbase/dbx" ) var _ = Describe("UserRepository", func() { @@ -74,6 +75,27 @@ var _ = Describe("UserRepository", func() { Expect(err).ToNot(HaveOccurred()) Expect(actual.Password).To(Equal("newpass")) }) + It("never logs the stored password, on insert or update, but still logs the SQL", func() { + logs := captureTraceLogs() + storedPassword := func(id string) string { + var enc string + Expect(GetDBXBuilder().NewQuery("select password from user where id = {:id}"). + Bind(dbx.Params{"id": id}).Row(&enc)).To(Succeed()) + return enc + } + u := model.User{ID: "u-logged", UserName: "u-logged", NewPassword: "first-secret"} + Expect(repo.Put(ctx, &u)).To(Succeed()) + inserted := storedPassword(u.ID) + u.NewPassword = "second-secret" + Expect(repo.Put(ctx, &u)).To(Succeed()) + updated := storedPassword(u.ID) + + Expect(logs.String()).To(ContainSubstring("INSERT INTO user")) + Expect(logs.String()).To(ContainSubstring("UPDATE user")) + for _, secret := range []string{inserted, updated, "first-secret", "second-secret"} { + Expect(logs.String()).ToNot(ContainSubstring(secret)) + } + }) It("persists and reads back the scrobble filter", func() { usr := model.User{ID: "u-filter", UserName: "u-filter", Name: "Filter User", ScrobbleFilter: `{"all":[{"contains":{"title":"????"}}]}`} diff --git a/server/apiv1/api_gen.go b/server/apiv1/api_gen.go index ccca0efc0..e174b0b43 100644 --- a/server/apiv1/api_gen.go +++ b/server/apiv1/api_gen.go @@ -1046,15 +1046,25 @@ type LoginResponseObject interface { VisitLoginResponse(w http.ResponseWriter) error } -type Login200JSONResponse GrantCreated +type Login200ResponseHeaders struct { + CacheControl *string +} + +type Login200JSONResponse struct { + Body GrantCreated + Headers Login200ResponseHeaders +} func (response Login200JSONResponse) VisitLoginResponse(w http.ResponseWriter) error { var buf bytes.Buffer - if err := json.NewEncoder(&buf).Encode(response); err != nil { + if err := json.NewEncoder(&buf).Encode(response.Body); err != nil { return err } w.Header().Set("Content-Type", "application/json") + if response.Headers.CacheControl != nil { + w.Header().Set("Cache-Control", fmt.Sprint(*response.Headers.CacheControl)) + } w.WriteHeader(200) _, err := buf.WriteTo(w) return err @@ -1366,15 +1376,25 @@ type SetupFirstAdminResponseObject interface { VisitSetupFirstAdminResponse(w http.ResponseWriter) error } -type SetupFirstAdmin201JSONResponse GrantCreated +type SetupFirstAdmin201ResponseHeaders struct { + CacheControl *string +} + +type SetupFirstAdmin201JSONResponse struct { + Body GrantCreated + Headers SetupFirstAdmin201ResponseHeaders +} func (response SetupFirstAdmin201JSONResponse) VisitSetupFirstAdminResponse(w http.ResponseWriter) error { var buf bytes.Buffer - if err := json.NewEncoder(&buf).Encode(response); err != nil { + if err := json.NewEncoder(&buf).Encode(response.Body); err != nil { return err } w.Header().Set("Content-Type", "application/json") + if response.Headers.CacheControl != nil { + w.Header().Set("Cache-Control", fmt.Sprint(*response.Headers.CacheControl)) + } w.WriteHeader(201) _, err := buf.WriteTo(w) return err @@ -1471,15 +1491,25 @@ type CreateAccessTokenResponseObject interface { VisitCreateAccessTokenResponse(w http.ResponseWriter) error } -type CreateAccessToken200JSONResponse AccessToken +type CreateAccessToken200ResponseHeaders struct { + CacheControl *string +} + +type CreateAccessToken200JSONResponse struct { + Body AccessToken + Headers CreateAccessToken200ResponseHeaders +} func (response CreateAccessToken200JSONResponse) VisitCreateAccessTokenResponse(w http.ResponseWriter) error { var buf bytes.Buffer - if err := json.NewEncoder(&buf).Encode(response); err != nil { + if err := json.NewEncoder(&buf).Encode(response.Body); err != nil { return err } w.Header().Set("Content-Type", "application/json") + if response.Headers.CacheControl != nil { + w.Header().Set("Cache-Control", fmt.Sprint(*response.Headers.CacheControl)) + } w.WriteHeader(200) _, err := buf.WriteTo(w) return err diff --git a/server/apiv1/api_test.go b/server/apiv1/api_test.go index 9e38672d1..3851db3d1 100644 --- a/server/apiv1/api_test.go +++ b/server/apiv1/api_test.go @@ -3,6 +3,7 @@ package apiv1 import ( "net/http" "net/http/httptest" + "strings" "github.com/getkin/kin-openapi/openapi3" "github.com/go-chi/chi/v5" @@ -30,6 +31,29 @@ var _ = Describe("Router", func() { } }) + It("declares Cache-Control no-store on the success responses of every no-store operation", func() { + doc, err := openapi3.NewLoader().LoadFromData(api.SpecJSON()) + Expect(err).ToNot(HaveOccurred()) + checked := map[string]bool{} + for _, item := range doc.Paths.Map() { + for _, op := range item.Operations() { + if !gateRulesV1.noStore[op.OperationID] { + continue + } + for code, resp := range op.Responses.Map() { + if !strings.HasPrefix(code, "2") { + continue + } + h := resp.Value.Headers["Cache-Control"] + Expect(h).ToNot(BeNil(), op.OperationID+" "+code) + Expect(h.Value.Schema.Value.Enum).To(ConsistOf("no-store"), op.OperationID+" "+code) + checked[op.OperationID] = true + } + } + } + Expect(checked).To(HaveLen(len(gateRulesV1.noStore))) + }) + It("returns a 404 problem for unknown paths", func() { w := serve(router, httptest.NewRequest(http.MethodGet, "/api/v1/nope", nil)) Expect(w.Code).To(Equal(http.StatusNotFound)) diff --git a/server/apiv1/apiv1_suite_test.go b/server/apiv1/apiv1_suite_test.go index 69416c924..6743951de 100644 --- a/server/apiv1/apiv1_suite_test.go +++ b/server/apiv1/apiv1_suite_test.go @@ -9,6 +9,7 @@ import ( "net/http" "net/http/httptest" "path/filepath" + "strings" "testing" "github.com/getkin/kin-openapi/openapi3" @@ -66,10 +67,18 @@ type testClient struct { } func (c testClient) call(method, path, bearer string, body any) *httptest.ResponseRecorder { + if body == nil { + return c.callRaw(method, path, bearer, "") + } + b, _ := json.Marshal(body) + return c.callRaw(method, path, bearer, string(b)) +} + +// callRaw sends body verbatim, for JSON a map cannot express, like keys differing only in case. +func (c testClient) callRaw(method, path, bearer, body string) *httptest.ResponseRecorder { var req *http.Request - if body != nil { - b, _ := json.Marshal(body) - req = httptest.NewRequestWithContext(c.ctx, method, path, bytes.NewReader(b)) + if body != "" { + req = httptest.NewRequestWithContext(c.ctx, method, path, strings.NewReader(body)) req.Header.Set("Content-Type", "application/json") } else { req = httptest.NewRequestWithContext(c.ctx, method, path, nil) diff --git a/server/apiv1/auth_handlers.go b/server/apiv1/auth_handlers.go index 1ddfaa33b..100a0e19a 100644 --- a/server/apiv1/auth_handlers.go +++ b/server/apiv1/auth_handlers.go @@ -22,12 +22,12 @@ func (rt *Router) CreateAccessToken(ctx context.Context, req CreateAccessTokenRe if err != nil { return nil, err } - return CreateAccessToken200JSONResponse{ + return CreateAccessToken200JSONResponse{Body: AccessToken{ AccessToken: tok.Token, TokenType: AccessTokenTokenTypeBearer, ExpiresIn: int(tok.ExpiresIn.Seconds()), Scopes: toScopes(tok.Scopes), - }, nil + }}, nil } func (rt *Router) ListGrants(ctx context.Context, req ListGrantsRequestObject) (ListGrantsResponseObject, error) { diff --git a/server/apiv1/auth_test.go b/server/apiv1/auth_test.go index 884865d16..e04766e54 100644 --- a/server/apiv1/auth_test.go +++ b/server/apiv1/auth_test.go @@ -131,7 +131,7 @@ var _ = Describe("auth endpoints", func() { gc := api.setup() at := api.mint(gc.Secret, nil) Expect(api.call(http.MethodGet, "/api/v1/auth/grants", at.AccessToken, nil).Code).To(Equal(http.StatusOK)) // caches the grant - Expect(realDS.Grant().Delete(ctx, gc.Grant.Id)).To(Succeed()) + Expect(realDS.Grant().DeleteForUser(ctx, gc.User.Id, gc.Grant.Id)).To(Succeed()) w := api.call(http.MethodPost, "/api/v1/auth/logout", at.AccessToken, nil) Expect(w.Code).To(Equal(http.StatusOK), w.Body.String()) @@ -142,13 +142,57 @@ var _ = Describe("auth endpoints", func() { gc := api.setup() at := api.mint(gc.Secret, nil) Expect(api.call(http.MethodGet, "/api/v1/auth/grants", at.AccessToken, nil).Code).To(Equal(http.StatusOK)) // caches the grant - Expect(realDS.Grant().Delete(ctx, gc.Grant.Id)).To(Succeed()) + Expect(realDS.Grant().DeleteForUser(ctx, gc.User.Id, gc.Grant.Id)).To(Succeed()) w := api.call(http.MethodPost, "/api/v1/auth/password", at.AccessToken, map[string]any{"currentPassword": "pw", "newPassword": "pw2"}) Expect(w.Code).To(Equal(http.StatusUnauthorized), w.Body.String()) Expect(w.Header().Get("WWW-Authenticate")).To(Equal(`Bearer error="invalid_token"`)) }) + It("rejects a case-variant scopes key that would widen an explicit empty subset", func() { + gc := api.setup() + w := api.callRaw(http.MethodPost, "/api/v1/auth/token", gc.Secret, `{"scopes":[],"Scopes":null}`) + Expect(w.Code).To(Equal(http.StatusBadRequest), w.Body.String()) + p := decodeProblem(w) + Expect(p.Code).To(Equal(ProblemCodeValidation)) + Expect(*p.Errors).To(ConsistOf(ValidationError{Field: "Scopes", Message: "must match the field name exactly"})) + }) + + It("rejects a case-variant client key that would skip its length limit", func() { + api.setup() + body := `{"username":"admin","password":"pw","client":"ok","Client":"` + strings.Repeat("x", 60_000) + `"}` + w := api.callRaw(http.MethodPost, "/api/v1/auth/login", "", body) + Expect(w.Code).To(Equal(http.StatusBadRequest), w.Body.String()) + Expect(*decodeProblem(w).Errors).To(ConsistOf(ValidationError{Field: "Client", Message: "must match the field name exactly"})) + }) + + DescribeTable("rejects a body with data after its JSON value, without echoing it", + func(path string, needsSecret bool, body string) { + secret := "" + if gc := api.setup(); needsSecret { + secret = gc.Secret + } + w := api.callRaw(http.MethodPost, path, secret, body) + Expect(w.Code).To(Equal(http.StatusBadRequest), w.Body.String()) + p := decodeProblem(w) + Expect(p.Code).To(Equal(ProblemCodeValidation)) + Expect(*p.Errors).To(ConsistOf(ValidationError{Field: "", Message: "must be a single JSON value"})) + Expect(w.Body.String()).ToNot(ContainSubstring("hunter2")) + }, + Entry("token with a trailing byte", "/api/v1/auth/token", true, `{"scopes":[],"Scopes":null}x`), + Entry("login with a second value", "/api/v1/auth/login", false, `{"username":"a","password":"hunter2","client":"c"} {}`), + ) + + It("checks the token body even when Content-Type has a repeated parameter", func() { + gc := api.setup() + req := httptest.NewRequestWithContext(ctx, http.MethodPost, "/api/v1/auth/token", strings.NewReader(`{"scopes":[],"Scopes":null}`)) + req.Header.Set("Content-Type", "application/json; a=1; a=2") + req.Header.Set("Authorization", "Bearer "+gc.Secret) + w := serve(api.router, req) + Expect(w.Code).To(Equal(http.StatusBadRequest), w.Body.String()) + Expect(*decodeProblem(w).Errors).To(ConsistOf(ValidationError{Field: "Scopes", Message: "must match the field name exactly"})) + }) + It("marks grant and token responses no-store", func() { w := api.call(http.MethodPost, "/api/v1/auth/setup", "", creds("admin", "pw")) Expect(w.Code).To(Equal(http.StatusCreated)) diff --git a/server/apiv1/gate.go b/server/apiv1/gate.go index ea4e2b2c9..ee25b15a2 100644 --- a/server/apiv1/gate.go +++ b/server/apiv1/gate.go @@ -1,13 +1,18 @@ package apiv1 import ( + "bytes" "cmp" "context" + "encoding/json" "errors" "fmt" + "io" + "maps" "net/http" "regexp" "slices" + "strconv" "strings" "github.com/getkin/kin-openapi/openapi3" @@ -265,19 +270,88 @@ func (g *gate) validate(w http.ResponseWriter, r *http.Request, op *gateOp, rctx err := openapi3filter.ValidateRequest(r.Context(), &openapi3filter.RequestValidationInput{ Request: r, PathParams: params, Route: op.route, Options: validationOptions, }) - if err == nil { - return true - } if tooLarge(err) { writeProblem(w, r, ClientError(err, tooLargeDetail)) return false } - fields := sanitizeValidation(err) + var fields []ValidationError + if err != nil { + fields = sanitizeValidation(err) + } else if fields = jsonBodyFields(r, op.route.Operation); len(fields) == 0 { + return true + } log.Debug(r.Context(), "API v1: request failed validation", "operation", op.id(), "errors", fields) writeProblemStatus(w, r, http.StatusBadRequest, ProblemCodeValidation, "the request does not match the API schema", fields...) return false } +// jsonBodyFields checks what kin-openapi misses in a JSON body: data after the first value, which Go's decoder +// ignores, and keys that only case-fold to a declared property, which encoding/json decodes into that property. +func jsonBodyFields(r *http.Request, op *openapi3.Operation) []ValidationError { + if op.RequestBody == nil || op.RequestBody.Value == nil || r.Body == nil { + return nil + } + // Keyed on the spec, not the request's Content-Type: the handlers decode JSON whatever the header says. + media := op.RequestBody.Value.Content.Get("application/json") + if media == nil || media.Schema == nil { + return nil + } + data, err := io.ReadAll(r.Body) + r.Body = io.NopCloser(bytes.NewReader(data)) + if err != nil || len(bytes.TrimSpace(data)) == 0 { + return nil + } + dec := json.NewDecoder(bytes.NewReader(data)) + var body any + if err := dec.Decode(&body); err != nil { + return []ValidationError{{Field: "", Message: "must be a single JSON value"}} + } + if _, err := dec.Token(); !errors.Is(err, io.EOF) { + return []ValidationError{{Field: "", Message: "must be a single JSON value"}} + } + var out []ValidationError + collectCaseAliases(body, media.Schema.Value, "", &out) + return out +} + +func collectCaseAliases(v any, schema *openapi3.Schema, path string, out *[]ValidationError) { + if schema == nil { + return + } + switch v := v.(type) { + case map[string]any: + for _, key := range slices.Sorted(maps.Keys(v)) { + field := joinField(path, key) + if prop, ok := schema.Properties[key]; ok { + if prop != nil { + collectCaseAliases(v[key], prop.Value, field, out) + } + continue + } + for name := range schema.Properties { + if strings.EqualFold(key, name) { + *out = append(*out, ValidationError{Field: field, Message: "must match the field name exactly"}) + break + } + } + } + case []any: + if schema.Items == nil { + return + } + for i, item := range v { + collectCaseAliases(item, schema.Items.Value, joinField(path, strconv.Itoa(i)), out) + } + } +} + +func joinField(path, name string) string { + if path == "" { + return name + } + return path + "." + name +} + var missingProperty = regexp.MustCompile(`property "([^"]+)" is missing`) // sanitizeValidation keeps only field paths and fixed messages: kin-openapi errors can embed the submitted value. diff --git a/server/apiv1/gate_test.go b/server/apiv1/gate_test.go index dc8f8aeb6..37cc67448 100644 --- a/server/apiv1/gate_test.go +++ b/server/apiv1/gate_test.go @@ -47,7 +47,9 @@ paths: schema: type: object required: [name] - properties: {name: {type: string, maxLength: 5}} + properties: + name: {type: string, maxLength: 5} + tags: {type: array, items: {type: object, properties: {label: {type: string, maxLength: 5}}}} responses: {'200': {description: ok}} /caps: get: {operationId: caps, x-module: core, security: [{bearerAuth: []}], responses: {'200': {description: ok}}} @@ -287,6 +289,64 @@ var _ = Describe("spec gate", func() { Expect(*decodeProblem(w).Errors).To(ConsistOf(ValidationError{Field: "name", Message: "is required"})) }) + DescribeTable("rejects a case variant of a declared body field, which Go would decode into it", + func(body, field string) { + fa.principal.Scopes = []string{"password"} + w := do(http.MethodPost, "/things", "Bearer x", body) + Expect(w.Code).To(Equal(http.StatusBadRequest), w.Body.String()) + p := decodeProblem(w) + Expect(p.Code).To(Equal(ProblemCodeValidation)) + Expect(*p.Errors).To(ConsistOf(ValidationError{Field: field, Message: "must match the field name exactly"})) + Expect(reached).To(BeEmpty()) + }, + Entry("top level", `{"name":"ok","NAME":"much-too-long"}`, "NAME"), + Entry("inside array items", `{"name":"ok","tags":[{"label":"a"},{"label":"b","Label":"much-too-long"}]}`, "tags.1.Label"), + Entry("Unicode case folding", "{\"name\":\"ok\",\"tag\u017f\":null}", "tag\u017f"), + ) + + DescribeTable("rejects data after the first JSON value, which Go's decoder would ignore", + func(body string) { + fa.principal.Scopes = []string{"password"} + w := do(http.MethodPost, "/things", "Bearer x", body) + Expect(w.Code).To(Equal(http.StatusBadRequest), w.Body.String()) + p := decodeProblem(w) + Expect(p.Code).To(Equal(ProblemCodeValidation)) + Expect(*p.Errors).To(ConsistOf(ValidationError{Field: "", Message: "must be a single JSON value"})) + Expect(reached).To(BeEmpty()) + }, + Entry("garbage", `{"name":"ok","NAME":"much-too-long"}x`), + Entry("a second value", `{"name":"ok"} {"NAME":"much-too-long"}`), + Entry("a stray bracket", `{"name":"ok"}]`), + ) + + DescribeTable("checks the body whatever parameters the Content-Type carries", + func(contentType string) { + fa.principal.Scopes = []string{"password"} + req := httptest.NewRequestWithContext(ctx, http.MethodPost, "/things", strings.NewReader(`{"name":"ok","NAME":"much-too-long"}`)) + req.Header.Set("Content-Type", contentType) + req.Header.Set("Authorization", "Bearer x") + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusBadRequest), w.Body.String()) + Expect(*decodeProblem(w).Errors).To(ConsistOf(ValidationError{Field: "NAME", Message: "must match the field name exactly"})) + Expect(reached).To(BeEmpty()) + }, + Entry("repeated parameter", "application/json; a=1; a=2"), + Entry("repeated charset", "application/json; charset=utf-8; CHARSET=latin1"), + ) + + It("accepts trailing whitespace after the JSON value", func() { + fa.principal.Scopes = []string{"password"} + Expect(do(http.MethodPost, "/things", "Bearer x", "{\"name\":\"ok\"}\n \t").Code).To(Equal(http.StatusOK)) + }) + + It("allows unknown body fields that do not collide with a declared one", func() { + fa.principal.Scopes = []string{"password"} + w := do(http.MethodPost, "/things", "Bearer x", `{"name":"ok","extra":{"Name":"x"},"tags":[{"label":"a","other":1}]}`) + Expect(w.Code).To(Equal(http.StatusOK), w.Body.String()) + Expect(reached).To(Equal("createThing")) + }) + It("validates path parameters", func() { w := do(http.MethodGet, "/things/toolong", "Bearer x", "") Expect(w.Code).To(Equal(http.StatusBadRequest)) diff --git a/server/apiv1/password_handlers.go b/server/apiv1/password_handlers.go index 5eef55090..32cf09e87 100644 --- a/server/apiv1/password_handlers.go +++ b/server/apiv1/password_handlers.go @@ -14,7 +14,7 @@ func (rt *Router) Login(ctx context.Context, req LoginRequestObject) (LoginRespo if err != nil { return nil, err } - return Login200JSONResponse(toGrantCreated(issued)), nil + return Login200JSONResponse{Body: toGrantCreated(issued)}, nil } func (rt *Router) SetupFirstAdmin(ctx context.Context, req SetupFirstAdminRequestObject) (SetupFirstAdminResponseObject, error) { @@ -23,7 +23,7 @@ func (rt *Router) SetupFirstAdmin(ctx context.Context, req SetupFirstAdminReques if err != nil { return nil, err } - return SetupFirstAdmin201JSONResponse(toGrantCreated(issued)), nil + return SetupFirstAdmin201JSONResponse{Body: toGrantCreated(issued)}, nil } func (rt *Router) ChangePassword(ctx context.Context, req ChangePasswordRequestObject) (ChangePasswordResponseObject, error) {