feat(auth): invite users without a password and let them set it on first login - #348
Merged
cristim merged 2 commits intoMay 13, 2026
Conversation
…rst login Admins can now create a user with the password field left blank. When that happens the backend: - Stores the user inactive, with an unguessable random bcrypt hash so the password_hash NOT NULL constraint stays satisfied and no client input can match the placeholder. - Generates a setup token, hashes it for storage, and stamps a 7-day expiry (new PasswordSetupExpiry constant — longer than the 1-hour PasswordResetExpiry because invites typically sit in an inbox before the recipient acts). - Mails the recipient a "Set your password" link that lands on the existing /reset-password page; ConfirmPasswordReset already activates the user on first password set (admin-bootstrap flow), so the welcome link doubles as activation with no schema changes. The user-create modal drops the password "required" attribute, runs the strength checks only when a password was actually typed, and shows the admin a distinct success toast pointing at the invitee's mailbox. Added SendUserInviteEmail across EmailSenderInterface, the SMTP sender, the no-op sender, all test mocks, the Sender's high-level method and the template renderer / template constant. Two new unit tests cover the invite happy path and the email-send-failure-still-succeeds case so admins don't see a 5xx when SES rate-limits or the recipient address is malformed.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR implements a complete user invitation workflow, enabling admins to create users without passwords. Invited users receive time-limited setup emails and set their own passwords on first login. The change spans email infrastructure, backend user creation logic, frontend UX, and test infrastructure. ChangesUser Invitation Feature
Sequence DiagramsequenceDiagram
participant Admin
participant Frontend
participant APIServer
participant UserService
participant EmailService
participant Database
Admin->>Frontend: Create user without password
Frontend->>APIServer: POST /users (password="")
APIServer->>UserService: CreateUser(password="")
alt Invitation Flow
UserService->>Database: Insert user (Active=false)
UserService->>Database: Store setup token + expiry
UserService->>EmailService: SendUserInviteEmail(setupURL)
EmailService-->>EmailService: (async, fire-and-forget)
end
UserService-->>APIServer: created User
APIServer-->>Frontend: success + "invitation sent"
Frontend-->>Admin: confirmation message
Note over Admin,EmailService: User receives invite email with setup link
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Member
Author
|
@coderabbitai review Generated by Claude Code |
Contributor
|
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/auth/service_user.go`:
- Around line 180-187: The invite path currently swallows SendUserInviteEmail
errors (s.emailSender.SendUserInviteEmail) and only logs them, causing the
caller to see success for an unreachable/inactive account; change the flow so
that when SendUserInviteEmail returns an error (during the invite branch that
builds setupURL using s.dashboardURL and inviteToken) you return that error (or
a wrapped warning/error result) to the caller instead of only logging it, or add
a boolean status field to the service response indicating inviteDeliveryFailed
and set it on failure so callers (admin/UI) can surface the problem; update the
function that invokes this block to propagate the error/status to its API layer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a10bb5f-334a-42ea-8b16-dd6ed147f4eb
📒 Files selected for processing (18)
frontend/src/api/users.tsfrontend/src/index.htmlfrontend/src/users/userModals.tsinternal/api/coverage_gaps_test.gointernal/auth/interfaces.gointernal/auth/service.gointernal/auth/service_user.gointernal/auth/service_user_test.gointernal/auth/test_helpers.gointernal/email/interfaces.gointernal/email/nop_sender.gointernal/email/smtp_sender.gointernal/email/template_renderers.gointernal/email/templates.gointernal/purchase/mocks_test.gointernal/scheduler/scheduler_test.gointernal/server/app_test.gointernal/server/handler_ri_exchange_test.go
Comment on lines
+180
to
+187
| if invite { | ||
| setupURL := fmt.Sprintf("%s/reset-password?token=%s", s.dashboardURL, inviteToken) | ||
| if err := s.emailSender.SendUserInviteEmail(ctx, user.Email, setupURL); err != nil { | ||
| // Mirror the password-reset flow: log but don't fail the | ||
| // caller — the admin already sees a created user, and the | ||
| // invite can be re-sent through the password-reset endpoint. | ||
| logging.Errorf("Failed to send user invite email: %v", err) | ||
| } |
Contributor
There was a problem hiding this comment.
Surface invite-delivery failure to the caller.
This path still returns success for an inactive account even when SendUserInviteEmail fails, so the admin/UI cannot tell that the user is currently unreachable. In this admin-only flow, logging the failure is not enough; return a warning/status the caller can act on, or fail the request. Otherwise the new invite success UX can report a false positive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/auth/service_user.go` around lines 180 - 187, The invite path
currently swallows SendUserInviteEmail errors
(s.emailSender.SendUserInviteEmail) and only logs them, causing the caller to
see success for an unreachable/inactive account; change the flow so that when
SendUserInviteEmail returns an error (during the invite branch that builds
setupURL using s.dashboardURL and inviteToken) you return that error (or a
wrapped warning/error result) to the caller instead of only logging it, or add a
boolean status field to the service response indicating inviteDeliveryFailed and
set it on failure so callers (admin/UI) can surface the problem; update the
function that invokes this block to propagate the error/status to its API layer.
CodeRabbit on #348: the invite path silently swallowed SendUserInviteEmail errors and returned a success-shaped response, so the admin UI reported "Invitation email sent to ..." even when the recipient couldn't have received anything — the account would exist but be unreachable. Mirror the email_sent / email_reason pattern already used by purchase executions: - internal/auth/service_user.go: CreateUser now returns *CreateUserResult with optional InviteEmailSent + InviteEmailError fields. The user-row write still succeeds on email failure (a 5xx would imply rollback and push admins toward re-submitting, which the duplicate-email guard would then block); the failure is reported via the result so the API layer can pass it to the UI. - internal/auth/service_api.go: new APICreateUserResponse embeds APIUser to keep the existing flat JSON contract intact and only adds optional invite_email_sent / invite_email_error fields. POST /api/users now returns this wrapper. - frontend/src/api/types.ts, users.ts: createUser() resolves to the new CreateUserResponse type. - frontend/src/users/userModals.ts: three-way toast — password-up-front success, invite-delivered success, invite-failed warning that points the admin at Forgot Password as the recovery path. - Tests updated: the existing "still succeeds when email send fails" case is renamed to "surfaces delivery failure without 5xx" and now asserts InviteEmailSent==false + non-empty InviteEmailError. The happy path asserts InviteEmailSent==true. Non-invite paths assert InviteEmailSent is nil.
7 tasks
This was referenced May 13, 2026
Closed
cristim
added a commit
that referenced
this pull request
May 13, 2026
) (#357) The frontend at frontend/src/api/auth.ts:91-101 base64-encodes new_password before POSTing to /api/auth/reset-password, matching the convention used by login / change-password / update-profile. Every other password-handling endpoint in handler_auth.go calls decodeBase64Password before forwarding to the service. resetPassword was the one that didn't, so the bcrypt hash stored represented the base64 string rather than the plaintext. Login then decoded the typed-in plaintext and bcrypt-compared against the wrong hash — and the user got "invalid email or password" with no way to recover. Effect: every user invited via the new invite flow (#348) was locked out the moment they "set" a password. Forgot-Password for existing users was equally broken. P0/critical for any non-admin access. Tests: TestHandler_resetPassword_DecodesBase64 — happy path: base64 payload → service sees the plaintext. TestHandler_resetPassword_InvalidBase64 — malformed payload → 400 before the service call fires (#356 AC). TestHandler_resetPassword_Success / _Error updated to send properly-encoded payloads now that the handler decodes them.
cristim
added a commit
that referenced
this pull request
May 13, 2026
…358) Four users.test.ts cases were stale on `feat/multicloud-web-frontend` and are failing on every PR-merge ref against it (including #336, #352, others). The tests assert the pre-#348 / pre-#347 behaviour: 1. `loadUsers › should load users and groups` 2. `loadUsers › should render groups after loading` `mockGroups` is missing the `allowed_accounts: []` field that the backend now ships on every group (multi-cloud account-scoping work). The actual groups from `api.listGroups` carry the field, so the deep-equal assertion against the fixture diverges. 3. `openCreateUserModal › should make password required` 4. `saveUser › should validate empty password for new user` Issue #348 ("invite users without a password and let them set it on first login") made password optional on create. userModals.ts now explicitly sets `passwordInput.required = false` (line 36) so the form-level HTML validation doesn't block the invite flow, and saveUser intentionally calls `api.createUser` with an empty password to trigger the backend invite-email path. Both old tests asserted the opposite behaviour and now fail. Fix: - Add `allowed_accounts: []` to the two `mockGroups` entries. - Invert "should make password required" → "should not mark password as required (blank invites the user)" asserting `required === false`. - Replace "should validate empty password" → "should allow empty password for new user (invite flow, issue #348)" asserting createUser was called with an empty password and no error toast was rendered. Unblocks the frontend-build-sentinel workflow on PRs against `feat/multicloud-web-frontend`. 1633 frontend tests pass.
3 tasks
cristim
added a commit
that referenced
this pull request
May 13, 2026
…oses #344) (#352) * feat(frontend/recs): row-click toggles selection on Opportunities (closes #344) Replace the row-click → detail-drawer behaviour (issue #44) with row-click → toggle selection. Clicking anywhere on a recommendation row's body now toggles the row's checkbox + dispatches the existing change handler (which enforces the one-variant-per-cell radio behaviour from #224). Clicks on interactive descendants (input / button / a / label / select / [data-action]) still flow through to their own handlers, untouched. This closes the last task in the issue-344 phase-2 plan. The original T4 (right-side Plan-builder drawer) was dropped in plan review: every payload the drawer could carry (per-rec payment-option comparison, target/baseline tracking) is explicitly backend-deferred, so the drawer would have shipped as a prettier copy of the existing bottom action-box with no new information. The detail drawer that already opened on row-click had the same character — confidence + provenance duplicated the table, and usage history is empty by design until the collector wiring lands (known_issues/28). Net changes: - recommendations.ts: -227 lines (openDetailDrawer + fetchRecommend- ationDetail + detailFetchCache + clearRecommendationDetailCache + renderUsageSparkline removed); +13 lines (row-click toggle handler). - styles/components.css: -84 lines (.detail-drawer*, .confidence-*). - recommendations.test.ts: -136 lines (5 drawer tests removed) + +84 lines (4 row-click selection tests covering: toggle on non-interactive cell, second click unselects, interactive descendant click does NOT toggle, native checkbox click doesn't double-toggle). - api/recommendations.ts: untouched. getRecommendationDetail stays exported so the backend endpoint contract is preserved; resurrecting the drawer if needed is a git revert of this commit. Verification: 1629/1632 tests pass / 1 skipped (2 unrelated failures in users.test.ts pre-date this branch — they came in with PR #348). Coverage 80.70% stmts / 82.46% lines (slight dip from 80.93/82.70 as expected when removing tested code paths; well within parity gates). Build 517 KiB (down from 522 KiB after T3; the deleted drawer assets shaved ~5 KiB). * fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Admins can now create a user with the password field left blank. When that
happens the backend mails the recipient a "Set your password" link and
keeps the account inactive until they follow it — the recipient picks their
own password, never one the admin has to type and transmit.
Independent of #347 (role-allowlist fix); both branch off
feat/multicloud-web-frontendand can land in either order.What changes
Backend (auth service) —
internal/auth/service_user.goreq.Password == "":password_hashso theusers.password_hash NOT NULLconstraint stays satisfied and noclient input can ever match the placeholder.
Active = false, a hashed setup token, and aPasswordResetExpiry7 days out (newPasswordSetupExpiryconstant— longer than the 1-hour
PasswordResetExpirybecause invitestypically sit in an inbox before the recipient acts).
emailSender.SendUserInviteEmail(ctx, email, setupURL)./reset-password?token=…URL — theConfirmPasswordResetflow already activates inactive users on firstpassword set (admin-bootstrap flow), so the welcome link doubles as
activation with no schema migration.
mirroring the password-reset endpoint's anti-enumeration behaviour.
Email —
internal/email/{interfaces,templates,template_renderers,smtp_sender,nop_sender}.goSendUserInviteEmail(ctx, email, setupURL)across theSenderInterface, SMTP sender and no-op sender.userInviteTemplate("Set your password" copy, 7-day expiry hint,pointer to "Forgot password?" if the link expires).
Frontend —
frontend/src/users/userModals.ts,frontend/src/api/users.ts,frontend/src/index.htmlrequired; the placeholder + a<small>hint explain the invite behaviour.
typed.
createUserAPI helper passes an empty password through verbatiminstead of base64-wrapping it.
Tests / interface mocks
service_user_test.gocases: invite happy path captures thestored
Userand assertsActive == false, non-emptyPasswordResetToken, anPasswordResetExpiryin the future, and anon-empty placeholder
PasswordHash. Second case asserts the APIcall still succeeds when
SendUserInviteEmailreturns an error.SenderInterfaceimpls / mocks updated:internal/auth/test_helpers.go,internal/api/coverage_gaps_test.go,internal/purchase/mocks_test.go,internal/scheduler/scheduler_test.go,internal/server/app_test.go,internal/server/handler_ri_exchange_test.go.Why not a schema migration
The
users.password_hash NOT NULLconstraint stays — the placeholder isa real bcrypt hash of a fresh 32-byte token, just one no client can
reproduce. This avoids an irreversible migration on a constraint that's
defended other code paths since
000001_initial_schema.up.sql. Theadmin-bootstrap flow (
SetupAdmin↔ConfirmPasswordReset) hasalready established the "inactive user, password set later" pattern in
production, so we're slotting into an existing groove rather than
inventing a new account state.
Test plan
go build ./...go test ./...(full repo, all green)go test -v -run TestService_CreateUser ./internal/auth/...—both new cases pass:
invite_flow_when_password_omitted,invite_flow_still_succeeds_when_email_send_fails.field blank → confirm the success toast names the invitee's email,
the invite email arrives with a
/reset-password?token=…link,following the link lets the user set a password and lands them
logged in.
flow unchanged, toast says "User created successfully".
Out of scope (separate work if needed)
invites but a friendlier "Set your password" heading when the user is
inactive would be nicer.
the existing Forgot Password flow on the invitee's behalf.
Generated by Claude Code
Summary by CodeRabbit
Release Notes