From 57ac5f390c4f472861bdf47f92f1041f0af6274e Mon Sep 17 00:00:00 2001 From: JJ Eaton <58495321+jayleaton@users.noreply.github.com> Date: Fri, 23 Oct 2026 19:07:22 +0700 Subject: [PATCH] fix(admin): admin migrate sends the bootstrap admin's invite, again once its link expires Rebuilt on main, where #113 already has admin migrate send the bootstrap invite once. What this keeps from the original PR is the part main lacks: InviteBootstrapAdmin resends once the last invite is older than the link lifetime (inviteTTL = auth.DefaultInviteTTL, also set as the admin auth instance's InviteTTL so the two cannot drift), so a first email that never arrived or expired heals on a later deploy, and it never invites a disabled pending admin. Co-Authored-By: Claude Opus 5.5 --- .env.example | 5 +- Makefile | 2 +- apps/admin/cmd/admin/main.go | 15 +++- apps/admin/internal/config/config.go | 3 +- apps/admin/internal/services/admins.go | 32 ++++++-- apps/admin/internal/services/admins_test.go | 78 +++++++++++++------ apps/admin/internal/services/services_test.go | 4 +- docs/admin.md | 2 +- 8 files changed, 102 insertions(+), 39 deletions(-) diff --git a/.env.example b/.env.example index 35fa0f8..f21b3aa 100644 --- a/.env.example +++ b/.env.example @@ -82,8 +82,9 @@ ADMIN_PUBLIC_URL=http://localhost:3001 ADMIN_AUTH_SECRET=dev-only-admin-secret-change-me-0123456789 # Rotating: ADMIN_AUTH_SECRET_PREVIOUS / ADMIN_AUTH_SECRET_REVOKED and # ADMIN_AUTH_SECRET_GRACE work like the app's AUTH_SECRET_* (docs/secrets.md). -# Seeded (without a password) by `make admin-migrate`; then run -# `make admin-invite EMAIL=...` and open the set-password link from the log. +# Seeded (without a password) by `make admin-migrate`, which also sends its +# set-password link, again only once the last one has expired (in dev: open +# it from the log). ADMIN_BOOTSTRAP_EMAIL=admin@example.com # Admins must enrol in TOTP before their first session. Default: on in production. # ADMIN_REQUIRE_2FA=true diff --git a/Makefile b/Makefile index 3c28f0e..38d5084 100644 --- a/Makefile +++ b/Makefile @@ -211,7 +211,7 @@ admin-dev: db-up admin-migrate ## Run the admin dashboard (0.0.0.0, ADMIN_PORT o $(GO) run ./apps/admin/cmd/admin serve .PHONY: admin-migrate -admin-migrate: ## Migrate the admin database (auth + audit log) and seed ADMIN_BOOTSTRAP_EMAIL +admin-migrate: ## Migrate the admin database (auth + audit log), seed ADMIN_BOOTSTRAP_EMAIL and send its invite (again once it expires) $(GO) run ./apps/admin/cmd/admin migrate .PHONY: admin-invite diff --git a/apps/admin/cmd/admin/main.go b/apps/admin/cmd/admin/main.go index 7271650..f4fa0a2 100644 --- a/apps/admin/cmd/admin/main.go +++ b/apps/admin/cmd/admin/main.go @@ -2,7 +2,7 @@ // Railway service). // // admin serve run the HTTP server (default) -// admin migrate migrate the admin database (auth, then audit log) and seed ADMIN_BOOTSTRAP_EMAIL +// admin migrate migrate the admin database (auth, then audit log), seed ADMIN_BOOTSTRAP_EMAIL and email its invite (again once it expires) // admin invite email a set-password link to a new or pending admin // admin reset-2fa remove an admin's 2FA (the last admin lost the authenticator and backup codes) // @@ -51,6 +51,11 @@ const ( // email (`invite `, `reset-2fa `): the command and the address. const emailCommandArgs = 2 +// inviteTTL is how long an admin's set-password link stays valid; `admin +// migrate` resends the bootstrap admin's invite once the last one is older. +// The admin auth instance gets the same value, so the two cannot drift. +const inviteTTL = auth.DefaultInviteTTL + const usage = "usage: admin [serve | migrate | invite | reset-2fa ]" func main() { @@ -105,8 +110,9 @@ func main() { // migrate runs the admin database's migrations: the admin auth instance's // tables, then the audit log. The app database is the web service's to -// migrate. Then it seeds ADMIN_BOOTSTRAP_EMAIL and emails its invite once -// (idempotent), so the first admin needs no shell in the image. +// migrate. Then it seeds ADMIN_BOOTSTRAP_EMAIL and emails its invite while +// that admin is pending, at most once per inviteTTL, so the first admin needs +// no shell in the image and a failed or expired first link heals itself. func migrate(ctx context.Context, cfg config.Config, log *slog.Logger) error { url, err := cfg.Secrets.AdminDatabaseURL() if err != nil { @@ -141,7 +147,7 @@ func migrate(ctx context.Context, cfg config.Config, log *slog.Logger) error { admin, created, err := svc.Bootstrap(ctx, actor, in) sent := false if err == nil { - sent, err = svc.InviteBootstrapAdmin(ctx, actor, admin) + sent, err = svc.InviteBootstrapAdmin(ctx, actor, admin, inviteTTL) } // Let a queued invite email go out before the pre-deploy command exits. if serr := admins.Shutdown(ctx); err == nil { @@ -418,6 +424,7 @@ func newAdminAuth(ctx context.Context, cfg config.Config, adminPool auth.DB, eve CookiePrefix: "admin", // No Better Auth clients: bind the admin cookies to this host. HostOnlyCookies: true, + InviteTTL: inviteTTL, DisableSignUp: true, AppName: cfg.AppName, TrustedOrigins: cfg.TrustedOrigins, diff --git a/apps/admin/internal/config/config.go b/apps/admin/internal/config/config.go index 477ec73..e4c6009 100644 --- a/apps/admin/internal/config/config.go +++ b/apps/admin/internal/config/config.go @@ -42,7 +42,8 @@ type Config struct { // ADMIN_AUTH_SECRET is accepted (0: packages/auth default). AuthSecretGrace time.Duration // BootstrapEmail is seeded as the first admin (without a password) by - // `admin migrate`; `admin invite ` then sends its set-password link. + // `admin migrate`, which also sends its set-password link while it is + // pending, at most once per link lifetime (72 h). BootstrapEmail string // RequireTwoFactor makes every admin enrol in TOTP before getting a // session. Defaults to on in production. diff --git a/apps/admin/internal/services/admins.go b/apps/admin/internal/services/admins.go index 0e5f4cf..675fa03 100644 --- a/apps/admin/internal/services/admins.go +++ b/apps/admin/internal/services/admins.go @@ -160,6 +160,22 @@ func (service *AdminService) invitedBefore(ctx context.Context, admin Admin) (bo return false, nil } +// lastInvite returns when admin was last invited, looked up like +// invitedBefore: under its id, else its email (a first invite targets the +// email and precedes every id-targeted one). The log lists newest first. +func (service *AdminService) lastInvite(ctx context.Context, admin Admin) (at time.Time, invited bool, err error) { + for _, target := range []string{admin.ID, admin.Email} { + page, err := service.log.List(ctx, AuditQuery{Action: ActionAdminInvite, TargetID: target, Limit: 1}) + if err != nil { + return time.Time{}, false, err + } + if len(page.Entries) > 0 { + return page.Entries[0].CreatedAt, true, nil + } + } + return time.Time{}, false, nil +} + // DisableAdmin blocks the admin's sign-in and signs them out everywhere. // Nobody disables themselves or the last active admin. func (service *AdminService) DisableAdmin(ctx context.Context, actor Actor, adminID string) error { @@ -351,17 +367,21 @@ func (service *AdminService) Bootstrap(ctx context.Context, actor Actor, in vali // InviteBootstrapAdmin emails the bootstrap admin's invite from `admin // migrate` (the pre-deploy command, which runs inside the image), so a // deployment whose image has no shell to run `admin invite` in still gets its -// first admin. It sends only while a is pending with no invite on record, so -// later deploys send nothing (a fresh link: "Forgot password?" on the sign-in -// page, or another admin). Only the operator may call it. -func (service *AdminService) InviteBootstrapAdmin(ctx context.Context, actor Actor, admin Admin) (sent bool, err error) { +// first admin. It sends only while the admin is pending (no password, not +// disabled) and no invite is on record from the last linkTTL (how long a +// set-password link stays valid): deploys inside that window send nothing, +// and once the last link has expired, or its email never arrived (sends run +// in the background, so nothing here sees a rejection), the next deploy sends +// a fresh one. Only the operator may call it. +func (service *AdminService) InviteBootstrapAdmin(ctx context.Context, actor Actor, admin Admin, linkTTL time.Duration) (sent bool, err error) { if !actor.System { return false, ErrForbidden } - if admin.HasPassword { + if admin.HasPassword || admin.Disabled { return false, nil } - if invited, err := service.invitedBefore(ctx, admin); err != nil || invited { + last, invited, err := service.lastInvite(ctx, admin) + if err != nil || (invited && time.Since(last) < linkTTL) { return false, err } err = mutate(ctx, service.log, actor, adminEntry(ActionAdminInvite, admin, map[string]string{}), func() error { diff --git a/apps/admin/internal/services/admins_test.go b/apps/admin/internal/services/admins_test.go index 8e605dc..195b71f 100644 --- a/apps/admin/internal/services/admins_test.go +++ b/apps/admin/internal/services/admins_test.go @@ -3,6 +3,7 @@ package services_test import ( "fmt" "testing" + "time" "go.uber.org/mock/gomock" @@ -425,34 +426,67 @@ func TestResetTwoFactorByEmail(test *testing.T) { func TestInviteBootstrapAdmin(test *testing.T) { test.Parallel() svc, accounts, log := newAdminService(test) + bootstrap := services.SystemActor("bootstrap") + const linkTTL = 72 * time.Hour + invitedAgo := func(age time.Duration) services.AuditPage { + return services.AuditPage{Total: 1, Entries: []services.AuditEntry{{Action: "admin.invite", CreatedAt: time.Now().Add(-age)}}} + } + send := func() { + gomock.InOrder( + log.EXPECT().Begin(ctx, adminAudit(bootstrap, "admin.invite", map[string]string{})).Return(entryID, nil), + accounts.EXPECT().SendSetPasswordLink(ctx, "a2").Return(nil), + log.EXPECT().Resolve(ctx, entryID, succeeded).Return(nil), + ) + } // A pending admin with no invite on record (under the id or the email) - // gets the link once, by the operator. - bootstrap := services.SystemActor("bootstrap") - gomock.InOrder( - log.EXPECT().List(ctx, priorInvite).Return(services.AuditPage{}, nil), - log.EXPECT().List(ctx, priorEmailInvite).Return(services.AuditPage{}, nil), - log.EXPECT().Begin(ctx, adminAudit(bootstrap, "admin.invite", map[string]string{})).Return(entryID, nil), - accounts.EXPECT().SendSetPasswordLink(ctx, "a2").Return(nil), - log.EXPECT().Resolve(ctx, entryID, succeeded).Return(nil), - ) - if sent, err := svc.InviteBootstrapAdmin(ctx, bootstrap, pending); err != nil || !sent { - test.Fatalf("InviteBootstrapAdmin = %v, %v", sent, err) + // gets the link, by the operator. + log.EXPECT().List(ctx, priorInvite).Return(services.AuditPage{}, nil) + log.EXPECT().List(ctx, priorEmailInvite).Return(services.AuditPage{}, nil) + send() + if sent, err := svc.InviteBootstrapAdmin(ctx, bootstrap, pending, linkTTL); err != nil || !sent { + test.Fatalf("never invited = %v, %v", sent, err) } - // Later deploys send nothing: an invite is on record, or it was accepted. - log.EXPECT().List(ctx, priorInvite).Return(services.AuditPage{Total: 1}, nil) - if sent, err := svc.InviteBootstrapAdmin(ctx, bootstrap, pending); err != nil || sent { - test.Fatalf("invited before = %v, %v", sent, err) + // Deploys while the last link is valid send nothing (by id, or by email + // for a first invite). + for _, age := range []time.Duration{time.Minute, linkTTL - time.Hour} { + log.EXPECT().List(ctx, priorInvite).Return(invitedAgo(age), nil) + if sent, err := svc.InviteBootstrapAdmin(ctx, bootstrap, pending, linkTTL); err != nil || sent { + test.Fatalf("invited %s ago = %v, %v", age, sent, err) + } } - if sent, err := svc.InviteBootstrapAdmin(ctx, bootstrap, bob); err != nil || sent { - test.Fatalf("accepted = %v, %v", sent, err) + log.EXPECT().List(ctx, priorInvite).Return(services.AuditPage{}, nil) + log.EXPECT().List(ctx, priorEmailInvite).Return(invitedAgo(time.Hour), nil) + if sent, err := svc.InviteBootstrapAdmin(ctx, bootstrap, pending, linkTTL); err != nil || sent { + test.Fatalf("first invite by email 1h ago = %v, %v", sent, err) } - log.EXPECT().List(ctx, priorInvite).Return(services.AuditPage{}, errDB) - sent, err := svc.InviteBootstrapAdmin(ctx, bootstrap, pending) - wantInternal(test, err, errDB) - if sent { - test.Fatal("sent on error") + // Once the last link has expired, the next deploy sends a fresh one. + log.EXPECT().List(ctx, priorInvite).Return(invitedAgo(linkTTL+time.Hour), nil) + send() + if sent, err := svc.InviteBootstrapAdmin(ctx, bootstrap, pending, linkTTL); err != nil || !sent { + test.Fatalf("last invite expired = %v, %v", sent, err) + } + + // Accepted or disabled admins: no lookup, no send. + disabledPending := pending + disabledPending.Disabled = true + for _, admin := range []services.Admin{bob, disBob, disabledPending} { + if sent, err := svc.InviteBootstrapAdmin(ctx, bootstrap, admin, linkTTL); err != nil || sent { + test.Fatalf("%+v = %v, %v", admin, sent, err) + } + } + + for _, query := range []services.AuditQuery{priorInvite, priorEmailInvite} { + if query == priorEmailInvite { + log.EXPECT().List(ctx, priorInvite).Return(services.AuditPage{}, nil) + } + log.EXPECT().List(ctx, query).Return(services.AuditPage{}, errDB) + sent, err := svc.InviteBootstrapAdmin(ctx, bootstrap, pending, linkTTL) + wantInternal(test, err, errDB) + if sent { + test.Fatal("sent on error") + } } } diff --git a/apps/admin/internal/services/services_test.go b/apps/admin/internal/services/services_test.go index 811f116..5f3104b 100644 --- a/apps/admin/internal/services/services_test.go +++ b/apps/admin/internal/services/services_test.go @@ -115,7 +115,7 @@ func policyCalls(fakes ports, policy services.Policy) map[string]func(services.A return err }, "AdminService.InviteBootstrapAdmin": func(actor services.Actor) error { - _, err := admins.InviteBootstrapAdmin(ctx, actor, pending) + _, err := admins.InviteBootstrapAdmin(ctx, actor, pending, time.Hour) return err }, "AuditService.List": func(actor services.Actor) error { @@ -444,7 +444,7 @@ func mutations() []mutation { return []any{fakes.accounts.EXPECT().SendSetPasswordLink(ctx, "a2").Return(err)} }, call: func(fakes ports) error { - sent, err := admins(fakes).InviteBootstrapAdmin(ctx, cli, pending) + sent, err := admins(fakes).InviteBootstrapAdmin(ctx, cli, pending, time.Hour) if sent != (err == nil) { return fmt.Errorf("sent = %v with err %w", sent, err) } diff --git a/docs/admin.md b/docs/admin.md index 385f658..094dc8b 100644 --- a/docs/admin.md +++ b/docs/admin.md @@ -3,7 +3,7 @@ `apps/admin` is a separate binary and Railway service for the people who run the app: browse, search and inspect users (sessions, verification, role, ban state), ban (reason + expiry) and unban, revoke sessions, set roles, manage admins, and read the audit log of every admin action. Admin changes fail closed: if the audit log cannot record the attempt, nothing is changed, and an attempt whose outcome was never recorded shows as "outcome unknown" until a reconciler checks the current state. Sign-ins, sign-outs, refused 2FA codes and spent backup codes are audited best-effort after the fact (they never block authentication), so the log can miss one if it was down at that moment. An analytics page is a placeholder behind a port. - **Isolated.** Admins are users of the admin app's own auth instance in its own database (`ADMIN_DATABASE_URL`, cookie `admin.session_token`, secret `ADMIN_AUTH_SECRET`), never rows in the app's `user` table. It reads and administers the app's users through `DATABASE_URL`. depguard keeps `apps/web` and `apps/admin` from importing each other and keeps the higher-privilege `packages/database/adminrepo` out of the web app. -- **Invite only.** There is no sign-up. `admin migrate` seeds `ADMIN_BOOTSTRAP_EMAIL` without a password and emails its set-password link once. It is the pre-deploy command and runs inside the image, so the first admin needs no shell. `admin invite ` (or `make admin-invite EMAIL=...`, or the Admins page) emails a one-time set-password link. With `ADMIN_REQUIRE_2FA=true` (the default in production) setting the password continues into TOTP enrolment with ten backup codes before the first session. The codes are shown once, stored as keyed hashes, and each one works for a single sign-in. +- **Invite only.** There is no sign-up. `admin migrate` seeds `ADMIN_BOOTSTRAP_EMAIL` without a password and emails its set-password link, and again on the first deploy after that link expires (72 h) while the admin is still pending, so a first email that never arrived heals itself. It is the pre-deploy command and runs inside the image, so the first admin needs no shell. `admin invite ` (or `make admin-invite EMAIL=...`, or the Admins page) emails a one-time set-password link. With `ADMIN_REQUIRE_2FA=true` (the default in production) setting the password continues into TOTP enrolment with ten backup codes before the first session. The codes are shown once, stored as keyed hashes, and each one works for a single sign-in. - **Banning** revokes every session of the user at once; the web app refuses their sign-in until the ban is lifted or expires. ```sh