diff --git a/core/apiauth/service.go b/core/apiauth/service.go index d6cdf4e20..1f1fd4906 100644 --- a/core/apiauth/service.go +++ b/core/apiauth/service.go @@ -56,7 +56,8 @@ type Service struct { checkers func(ds model.DataStore) []CredentialChecker // per datastore, so password change can check inside its transaction cache *livenessCache now func() time.Time - signer func() (*signer, error) + signerMu sync.Mutex + sg *signer } func New(ds model.DataStore) *Service { @@ -68,13 +69,24 @@ func New(ds model.DataStore) *Service { cache: newLivenessCache(cacheTTL), now: time.Now, } - // Loaded on first use so building the router never touches the database. - s.signer = sync.OnceValues(func() (*signer, error) { - return loadSigner(context.Background(), ds, func() time.Time { return s.now() }) - }) return s } +// signer loads the key on first use, so building the router never touches the database; only a success is kept. +func (s *Service) signer() (*signer, error) { + s.signerMu.Lock() + defer s.signerMu.Unlock() + if s.sg != nil { + return s.sg, nil + } + sg, err := loadSigner(context.Background(), s.ds, func() time.Time { return s.now() }) + if err != nil { + return nil, err + } + s.sg = sg + return sg, nil +} + func PasswordChangeable(u model.User) bool { return u.IsAdmin || conf.Server.EnableUserEditing } @@ -109,6 +121,7 @@ func (s *Service) Setup(ctx context.Context, username, password string, meta Cli // issue stores a grant bound to the epoch read with the user, so a racing password change leaves it dead. func (s *Service) issue(ctx context.Context, ds model.DataStore, u model.User, provider string, meta ClientMeta, scopes []string) (*Issued, error) { + u.Password = "" secret, hash := newSecret() g := model.Grant{ UserID: u.ID, diff --git a/core/apiauth/service_test.go b/core/apiauth/service_test.go index 706e09ca3..06d716faa 100644 --- a/core/apiauth/service_test.go +++ b/core/apiauth/service_test.go @@ -2,6 +2,7 @@ package apiauth import ( "context" + "errors" "strings" "time" @@ -13,6 +14,30 @@ import ( . "github.com/onsi/gomega" ) +var errFlakyProps = errors.New("database is locked") + +type flakyPropsDS struct { + model.DataStore + failures int +} + +func (d *flakyPropsDS) Property() model.PropertyRepository { + return &flakyProps{PropertyRepository: d.DataStore.Property(), ds: d} +} + +type flakyProps struct { + model.PropertyRepository + ds *flakyPropsDS +} + +func (p *flakyProps) PutIfAbsent(ctx context.Context, id, value string) error { + if p.ds.failures > 0 { + p.ds.failures-- + return errFlakyProps + } + return p.PropertyRepository.PutIfAbsent(ctx, id, value) +} + var meta = ClientMeta{Name: "Living room", Client: "TestApp", ClientVersion: "1.0"} var _ = Describe("Service: grants and tokens", func() { @@ -35,6 +60,7 @@ var _ = Describe("Service: grants and tokens", func() { Expect(err).ToNot(HaveOccurred()) Expect(issued.Secret).To(HavePrefix("ndg_")) Expect(issued.User.ID).To(Equal(u.ID)) + Expect(issued.User.Password).To(BeEmpty()) Expect(issued.Grant.Scopes).To(Equal(model.Scopes{ScopeAll})) Expect(issued.Grant.Provider).To(Equal("password")) Expect(issued.Grant.Name).To(Equal("Living room")) @@ -102,6 +128,23 @@ var _ = Describe("Service: grants and tokens", func() { Expect(principal.User.ID).To(Equal(u.ID)) }) + It("retries loading the signing key after a failed load", func() { + flaky := &flakyPropsDS{DataStore: realDS, failures: 1} + svc = New(flaky) + svc.SetClock(func() time.Time { return now }) + u := createUser(ctx, "pw", false) + issued, err := svc.Login(ctx, u.UserName, "pw", meta, nil) + Expect(err).ToNot(HaveOccurred()) + p, err := svc.ResolveGrant(ctx, issued.Secret, "") + Expect(err).ToNot(HaveOccurred()) + + _, err = svc.Mint(ctx, p, nil) + Expect(err).To(MatchError(errFlakyProps)) + tok, err := svc.Mint(ctx, p, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(tok.Token).ToNot(BeEmpty()) + }) + It("attenuates to the requested subset", func() { u := createUser(ctx, "pw", false) issued, _ := svc.Login(ctx, u.UserName, "pw", meta, nil)