Skip to content

Commit 8ed7cd3

Browse files
committed
test(identity): pin fail-hard semantics of self-registered admin hook
The provisioning worker treats any non-nil error from a post-provisioning hook as fatal: the tenant is marked provisioning_failed instead of active (executePostProvisioningHooks + markTenantAsActive). This contract prevents an apparently-active tenant whose admin identity was never created - which would surface to the user as a misleading "invalid email or password" 401 on the very first login attempt (see task 5). Pin the hook side of that contract: - Document the fail-hard semantics on Provision, naming the worker function that depends on it and the user-visible failure that the contract prevents. - Extend fakeRepo with findByEmailErr / saveWithRolesErr injection fields and add an errorInjectingMetadataStore that simulates GetMetadata and UpdateMetadata failures. - Add unit tests covering every failure path and key invariants: GetMetadata failure, missing email, missing password hash, SaveIdentityWithRoles failure (the core fail-hard scenario), UpdateMetadata failure when clearing credentials, plus happy-path and idempotency assertions. The SaveIdentityWithRoles test also verifies registration metadata is NOT cleared on failure so retries can succeed. No production code change beyond the doc comment - the hook already returns errors at every failure point. These tests prevent silent regressions in error propagation.
1 parent d1857ac commit 8ed7cd3

3 files changed

Lines changed: 246 additions & 0 deletions

File tree

services/identity/bootstrap/demo_users_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ type fakeRepo struct {
2323
// schemaNotProvisioned makes FindByEmail return ErrTenantSchemaNotProvisioned
2424
// for any email lookup, simulating a missing tenant schema.
2525
schemaNotProvisioned bool
26+
27+
// Error injection fields for failure-path tests. When non-nil, the matching
28+
// method returns the configured error instead of its normal result.
29+
findByEmailErr error
30+
saveWithRolesErr error
2631
}
2732

2833
func newFakeRepo() *fakeRepo {
@@ -47,6 +52,9 @@ func (f *fakeRepo) FindByID(_ context.Context, id uuid.UUID) (*domain.Identity,
4752
}
4853

4954
func (f *fakeRepo) FindByEmail(_ context.Context, email string) (*domain.Identity, error) {
55+
if f.findByEmailErr != nil {
56+
return nil, f.findByEmailErr
57+
}
5058
if f.schemaNotProvisioned {
5159
return nil, db.ErrTenantSchemaNotProvisioned
5260
}
@@ -80,6 +88,9 @@ func (f *fakeRepo) SaveIdentityWithInvitation(_ context.Context, identity *domai
8088

8189
func (f *fakeRepo) SaveIdentityWithRoles(_ context.Context, identity *domain.Identity, roles []*domain.RoleAssignment) error {
8290
f.saveWithRolesCalled = true
91+
if f.saveWithRolesErr != nil {
92+
return f.saveWithRolesErr
93+
}
8394
f.identities[identity.Email()] = identity
8495
f.roles[identity.ID()] = append(f.roles[identity.ID()], roles...)
8596
return nil

services/identity/bootstrap/self_registered_admin.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,14 @@ func (h *SelfRegisteredAdminHook) AsPostProvisioningHook() func(ctx context.Cont
8484
// Provision creates the self-registered admin identity from tenant metadata.
8585
// If no registration metadata is present (e.g., tenant was created via API, not
8686
// self-registration), this is a no-op.
87+
//
88+
// Fail-hard semantics: every infrastructure or domain failure surfaces as a
89+
// non-nil error. The provisioning worker treats a non-nil hook error as fatal
90+
// (services/tenant/worker.executePostProvisioningHooks): tenant status stays
91+
// in provisioning and the worker transitions it to provisioning_failed instead
92+
// of active. This prevents an apparently-active tenant whose admin identity
93+
// was never created (which would surface to the user as a misleading "invalid
94+
// email or password" 401 on the very first login attempt).
8795
func (h *SelfRegisteredAdminHook) Provision(ctx context.Context, tenantID tenant.TenantID) error {
8896
// Read tenant metadata to check for registration credentials.
8997
metadata, err := h.tenantStore.GetMetadata(ctx, tenantID)

services/identity/bootstrap/self_registered_admin_test.go

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ package bootstrap
22

33
import (
44
"context"
5+
"errors"
56
"log/slog"
67
"testing"
78

89
gateway "github.com/meridianhub/meridian/services/api-gateway"
910
"github.com/meridianhub/meridian/services/identity/domain"
1011
"github.com/meridianhub/meridian/shared/platform/tenant"
1112
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
1214
)
1315

1416
// mockIdentityRepo satisfies domain.Repository for testing.
@@ -27,6 +29,46 @@ func (m *mockTenantMetadataStore) UpdateMetadata(_ context.Context, _ tenant.Ten
2729
return nil
2830
}
2931

32+
// errorInjectingMetadataStore is a TenantMetadataStore that returns
33+
// configurable metadata and can simulate read/write failures, used to verify
34+
// the fail-hard semantics of SelfRegisteredAdminHook.Provision.
35+
type errorInjectingMetadataStore struct {
36+
metadata map[string]interface{}
37+
getErr error
38+
updateErr error
39+
updateCalled bool
40+
}
41+
42+
func (m *errorInjectingMetadataStore) GetMetadata(_ context.Context, _ tenant.TenantID) (map[string]interface{}, error) {
43+
if m.getErr != nil {
44+
return nil, m.getErr
45+
}
46+
// Return a copy so callers cannot mutate the test fixture.
47+
if m.metadata == nil {
48+
return nil, nil
49+
}
50+
out := make(map[string]interface{}, len(m.metadata))
51+
for k, v := range m.metadata {
52+
out[k] = v
53+
}
54+
return out, nil
55+
}
56+
57+
func (m *errorInjectingMetadataStore) UpdateMetadata(_ context.Context, _ tenant.TenantID, _ map[string]interface{}) error {
58+
m.updateCalled = true
59+
return m.updateErr
60+
}
61+
62+
// validRegistrationMetadata returns a metadata map that would satisfy the
63+
// hook's validation, suitable for tests that exercise downstream failure
64+
// paths.
65+
func validRegistrationMetadata() map[string]interface{} {
66+
return map[string]interface{}{
67+
MetaKeyRegistrationEmail: "owner@example.com",
68+
MetaKeyRegistrationPasswordHash: "$2a$12$dummyhashdummyhashdummyhashdummyhashdummyhashdummyhash",
69+
}
70+
}
71+
3072
func TestNewSelfRegisteredAdminHook_NilIdentityRepo(t *testing.T) {
3173
_, err := NewSelfRegisteredAdminHook(nil, &mockTenantMetadataStore{}, slog.Default())
3274
assert.ErrorIs(t, err, ErrNilRepository)
@@ -56,3 +98,188 @@ func TestSelfRegisteredAdminHook_AsPostProvisioningHook(t *testing.T) {
5698
// Verify RoleTenantOwner is the correct role for self-registered admins.
5799
assert.Equal(t, domain.Role("TENANT_OWNER"), domain.RoleTenantOwner)
58100
}
101+
102+
// --- Failure-path tests for fail-hard semantics ---
103+
//
104+
// The provisioning worker treats any non-nil error from a post-provisioning
105+
// hook as fatal: the tenant is marked provisioning_failed instead of active
106+
// (services/tenant/worker/provisioning_worker.go:executePostProvisioningHooks
107+
// + markTenantAsActive). These tests pin the hook's contract: every
108+
// infrastructure or domain failure surfaces as a non-nil error so the
109+
// worker's failure handling actually fires.
110+
111+
func TestProvision_NoMetadata_NoOp(t *testing.T) {
112+
tid := tenant.MustNewTenantID("acme")
113+
114+
repo := newFakeRepo()
115+
store := &errorInjectingMetadataStore{} // no metadata at all
116+
117+
hook, err := NewSelfRegisteredAdminHook(repo, store, slog.Default())
118+
require.NoError(t, err)
119+
120+
err = hook.Provision(context.Background(), tid)
121+
require.NoError(t, err, "tenants without registration metadata should be a no-op, not an error")
122+
assert.False(t, repo.saveWithRolesCalled, "no identity should be saved when metadata is absent")
123+
assert.False(t, store.updateCalled, "metadata should not be cleared when nothing was set")
124+
}
125+
126+
func TestProvision_GetMetadataFails_ReturnsError(t *testing.T) {
127+
tid := tenant.MustNewTenantID("acme")
128+
129+
repo := newFakeRepo()
130+
store := &errorInjectingMetadataStore{
131+
getErr: errors.New("connection refused"),
132+
}
133+
134+
hook, err := NewSelfRegisteredAdminHook(repo, store, slog.Default())
135+
require.NoError(t, err)
136+
137+
err = hook.Provision(context.Background(), tid)
138+
require.Error(t, err, "GetMetadata failure must surface so the worker marks the tenant provisioning_failed")
139+
assert.Contains(t, err.Error(), "reading tenant metadata")
140+
assert.False(t, repo.saveWithRolesCalled)
141+
}
142+
143+
func TestProvision_MissingEmail_ReturnsInvalidMetadataError(t *testing.T) {
144+
tid := tenant.MustNewTenantID("acme")
145+
146+
repo := newFakeRepo()
147+
store := &errorInjectingMetadataStore{
148+
metadata: map[string]interface{}{
149+
MetaKeyRegistrationPasswordHash: "$2a$12$dummyhash",
150+
// email key intentionally missing - partial metadata is invalid.
151+
},
152+
}
153+
154+
hook, err := NewSelfRegisteredAdminHook(repo, store, slog.Default())
155+
require.NoError(t, err)
156+
157+
err = hook.Provision(context.Background(), tid)
158+
require.Error(t, err)
159+
assert.ErrorIs(t, err, ErrInvalidRegistrationMetadata,
160+
"partial metadata (hash without email) must surface as ErrInvalidRegistrationMetadata")
161+
assert.False(t, repo.saveWithRolesCalled)
162+
}
163+
164+
func TestProvision_MissingPasswordHash_ReturnsInvalidMetadataError(t *testing.T) {
165+
tid := tenant.MustNewTenantID("acme")
166+
167+
repo := newFakeRepo()
168+
store := &errorInjectingMetadataStore{
169+
metadata: map[string]interface{}{
170+
MetaKeyRegistrationEmail: "owner@example.com",
171+
// hash key intentionally missing - partial metadata is invalid.
172+
},
173+
}
174+
175+
hook, err := NewSelfRegisteredAdminHook(repo, store, slog.Default())
176+
require.NoError(t, err)
177+
178+
err = hook.Provision(context.Background(), tid)
179+
require.Error(t, err)
180+
assert.ErrorIs(t, err, ErrInvalidRegistrationMetadata,
181+
"partial metadata (email without hash) must surface as ErrInvalidRegistrationMetadata")
182+
assert.False(t, repo.saveWithRolesCalled)
183+
}
184+
185+
func TestProvision_SaveIdentityWithRolesFails_ReturnsError(t *testing.T) {
186+
// This is the core fail-hard test: when the identity write fails (DB
187+
// down, constraint violation, etc.) the hook MUST return an error so the
188+
// provisioning worker does not flip the tenant to active. Otherwise the
189+
// user would land on an "active" tenant with no admin identity and see a
190+
// misleading "invalid email or password" 401 on first login.
191+
tid := tenant.MustNewTenantID("acme")
192+
193+
dbErr := errors.New("database connection failed")
194+
repo := newFakeRepo()
195+
repo.saveWithRolesErr = dbErr
196+
197+
store := &errorInjectingMetadataStore{
198+
metadata: validRegistrationMetadata(),
199+
}
200+
201+
hook, err := NewSelfRegisteredAdminHook(repo, store, slog.Default())
202+
require.NoError(t, err)
203+
204+
err = hook.Provision(context.Background(), tid)
205+
require.Error(t, err, "SaveIdentityWithRoles failure must surface so the tenant is marked provisioning_failed")
206+
assert.ErrorIs(t, err, dbErr, "underlying error must be wrapped, not swallowed")
207+
assert.Contains(t, err.Error(), "saving identity with roles")
208+
assert.True(t, repo.saveWithRolesCalled, "the failure should occur at SaveIdentityWithRoles, not earlier")
209+
assert.False(t, store.updateCalled,
210+
"registration metadata must NOT be cleared when identity creation fails - retries need it intact")
211+
}
212+
213+
func TestProvision_UpdateMetadataFails_ReturnsError(t *testing.T) {
214+
// Clearing the bcrypt hash from metadata is fatal-by-design: leaving it
215+
// behind violates minimal credential retention. If the clear fails the
216+
// hook must surface an error so the worker treats the tenant as failed
217+
// and operators investigate.
218+
tid := tenant.MustNewTenantID("acme")
219+
220+
repo := newFakeRepo()
221+
store := &errorInjectingMetadataStore{
222+
metadata: validRegistrationMetadata(),
223+
updateErr: errors.New("update conflict"),
224+
}
225+
226+
hook, err := NewSelfRegisteredAdminHook(repo, store, slog.Default())
227+
require.NoError(t, err)
228+
229+
err = hook.Provision(context.Background(), tid)
230+
require.Error(t, err, "UpdateMetadata failure must surface to abort tenant activation")
231+
assert.Contains(t, err.Error(), "clearing registration metadata")
232+
assert.True(t, repo.saveWithRolesCalled, "identity should have been saved before clear was attempted")
233+
}
234+
235+
func TestProvision_HappyPath_ClearsMetadataAndSavesIdentity(t *testing.T) {
236+
// Sanity check that the success path still works end-to-end: identity is
237+
// saved with TENANT_OWNER role and registration credentials are cleared.
238+
tid := tenant.MustNewTenantID("acme")
239+
240+
repo := newFakeRepo()
241+
store := &errorInjectingMetadataStore{
242+
metadata: validRegistrationMetadata(),
243+
}
244+
245+
hook, err := NewSelfRegisteredAdminHook(repo, store, slog.Default())
246+
require.NoError(t, err)
247+
248+
err = hook.Provision(context.Background(), tid)
249+
require.NoError(t, err)
250+
251+
assert.True(t, repo.saveWithRolesCalled, "identity should be saved")
252+
assert.True(t, store.updateCalled, "registration metadata should be cleared")
253+
254+
identity, ok := repo.identities["owner@example.com"]
255+
require.True(t, ok, "identity should exist after provisioning")
256+
roles := repo.roles[identity.ID()]
257+
require.Len(t, roles, 1)
258+
assert.Equal(t, domain.RoleTenantOwner, roles[0].Role(),
259+
"self-registered admin must have TENANT_OWNER role")
260+
}
261+
262+
func TestProvision_IdempotentWhenIdentityAlreadyExists(t *testing.T) {
263+
// If the hook is re-run (e.g. after a transient failure), and the
264+
// identity already exists in the tenant schema, it should skip creation
265+
// silently rather than error out.
266+
tid := tenant.MustNewTenantID("acme")
267+
268+
existing, err := domain.NewIdentity(tid, "owner@example.com")
269+
require.NoError(t, err)
270+
271+
repo := newFakeRepo()
272+
repo.identities["owner@example.com"] = existing
273+
274+
store := &errorInjectingMetadataStore{
275+
metadata: validRegistrationMetadata(),
276+
}
277+
278+
hook, err := NewSelfRegisteredAdminHook(repo, store, slog.Default())
279+
require.NoError(t, err)
280+
281+
err = hook.Provision(context.Background(), tid)
282+
require.NoError(t, err, "re-running the hook against an existing admin must succeed")
283+
assert.False(t, repo.saveWithRolesCalled, "must not attempt to recreate an existing identity")
284+
assert.True(t, store.updateCalled, "metadata should still be cleared on the idempotent path")
285+
}

0 commit comments

Comments
 (0)