fix(api): retry the signing-key load after a failure and drop the password from issued grants

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,

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)