Feat: add email login auth#832
Conversation
…agic link sending
…; update import order in authApi
📝 WalkthroughWalkthroughAdds an email magic-link sign-in flow: Welcome page UI/state changes to send and confirm magic links, a new Changes
Sequence DiagramsequenceDiagram
actor User
participant Welcome as Welcome Component
participant API as sendEmailMagicLink
participant Backend as Backend Service
User->>Welcome: Enter email & submit
activate Welcome
Welcome->>Welcome: set sending/loading state
Welcome->>API: call sendEmailMagicLink(email, redirectUri)
deactivate Welcome
activate API
API->>API: resolve backend URL, start timeout/AbortSignal
API->>Backend: POST /auth/email/send-link {email, frontendRedirectUri}
deactivate API
activate Backend
Backend-->>API: 200 OK or error payload
deactivate Backend
activate API
API->>API: parse response or throw (handle AbortError -> timeout message)
API-->>Welcome: resolve or reject
deactivate API
activate Welcome
Welcome->>Welcome: show "Check your email" on success or inline error on failure
deactivate Welcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/src/services/api/authApi.ts (1)
11-25: Please add redacted debug logging around this network hop.This is the only backend call in the new flow, but there’s no dev-only trace for request start, resolved target, or failure path. Add namespaced logs here and redact the email rather than logging full PII.
As per coding guidelines, "Use namespaced
debug+ dev-only detail for logging in frontend code. Never log secrets or full PII — redact." and "Add substantial development-oriented logging at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error paths."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/services/api/authApi.ts` around lines 11 - 25, Add dev-only, namespaced debug logging around sendEmailMagicLink: import and use a namespaced debug logger (e.g., debug('auth:sendEmailMagicLink')) at entry, before the fetch, on success and on error; redact the email when logging (do not log full PII—mask local part leaving maybe first char and domain or replace with *****@domain) and log the resolved backend target/endpoint, HTTP method, response.status and any returned error message (but never log tokens/PII). Ensure logs are only emitted in non-production/dev using the debug mechanism or an explicit NODE_ENV check so production does not print these details.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/pages/Welcome.tsx`:
- Around line 77-107: The success and pending UI states (the isSent confirmation
block and the sending spinner) and the inline error need accessible live region
attributes so screen readers announce changes: add role="status" and
aria-live="polite" to the pending spinner container and to the isSent
confirmation container that renders the "Check your email" message (the JSX that
uses isSent and references email), and add role="alert" (or
aria-live="assertive") to the inline error block that displays submission
errors; update the same attributes in the other similar branch (lines referenced
in the review: the block around the spinner/error and the block at 131-153) so
isSent, setIsSent, setEmail, and the error rendering path provide proper
announcements.
- Around line 136-144: The email input in Welcome.tsx (the input bound to
value={email} and onChange={e => setEmail(e.target.value)}, disabled by
isSending) lacks an accessible name; add an associated <label htmlFor="..."> and
give the input a matching id (e.g., id="email") or, if you prefer not to add a
visible label, add a clear aria-label (e.g., aria-label="Email address") on the
same input element so screen readers can identify the field.
- Around line 21-42: The new handleEmailSubmit flow (function handleEmailSubmit)
introduces platform branching (isTauri vs window.location.origin), async state
changes (setIsSending, setIsSent, setEmailError) and UI paths (success view and
"Use a different email" reset) but lacks unit tests; add a co-located Vitest
React test file (e.g., Welcome.test.tsx) under app/src that mocks
sendEmailMagicLink and toggles isTauri, and assert: the correct redirect URI is
passed (DESKTOP_FRONTEND_URI when isTauri returns true, window.location.origin
otherwise), the pending state shows while the promise is unresolved and
setIsSending is toggled, backend errors surface to the UI when mocked
sendEmailMagicLink rejects (and setEmailError content is rendered), success
rendering occurs when it resolves (setIsSent true), and clicking the "Use a
different email" control resets state; use Vitest/react-testing-library patterns
and the existing app/test/vitest.config.ts for setup.
In `@app/src/services/api/authApi.ts`:
- Around line 15-23: The POST to `${backendUrl}/auth/email/send-link` can hang;
wrap the fetch in an AbortController with a reasonable timeout (e.g., 8–15s) and
call controller.abort() on timeout, then catch AbortError around the fetch in
the same function (the block using getBackendUrl and the fetch to
/auth/email/send-link) and rethrow a clear, user-facing Error like "Request
timed out, please retry" instead of leaving the promise unresolved; ensure the
controller.signal is passed into fetch and cleanup of the timeout timer after
fetch completes.
---
Nitpick comments:
In `@app/src/services/api/authApi.ts`:
- Around line 11-25: Add dev-only, namespaced debug logging around
sendEmailMagicLink: import and use a namespaced debug logger (e.g.,
debug('auth:sendEmailMagicLink')) at entry, before the fetch, on success and on
error; redact the email when logging (do not log full PII—mask local part
leaving maybe first char and domain or replace with *****@domain) and log the
resolved backend target/endpoint, HTTP method, response.status and any returned
error message (but never log tokens/PII). Ensure logs are only emitted in
non-production/dev using the debug mechanism or an explicit NODE_ENV check so
production does not print these details.
🪄 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: 75cae279-1450-4206-b56a-95c8c75d3307
📒 Files selected for processing (2)
app/src/pages/Welcome.tsxapp/src/services/api/authApi.ts
…n Welcome component and tests
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/services/api/__tests__/authApi.test.ts (1)
5-44: Good behavior-driven test coverage for the happy path and timeout.The tests properly verify the request shape and timeout behavior without real network calls or time-dependent flakes.
Consider adding a test case for non-OK response handling to verify the error message extraction logic:
📝 Suggested additional test
it('throws the backend error message on non-OK responses', async () => { vi.spyOn(globalThis, 'fetch').mockResolvedValue( new Response(JSON.stringify({ error: 'Invalid email format' }), { status: 400 }) ); await expect(sendEmailMagicLink('bad-email', 'openhuman://')).rejects.toThrow( 'Invalid email format' ); }); it('throws a status-based fallback when error body is missing', async () => { vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('{}', { status: 500 })); await expect(sendEmailMagicLink('user@example.com', 'openhuman://')).rejects.toThrow( 'Failed to send magic link (500)' ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/services/api/__tests__/authApi.test.ts` around lines 5 - 44, Add tests that exercise sendEmailMagicLink's non-OK response handling: mock globalThis.fetch to resolve to a Response with status 400 and body JSON { error: 'Invalid email format' } and assert sendEmailMagicLink('bad-email', 'openhuman://') rejects with 'Invalid email format'; also add a test where fetch resolves to a Response with status 500 and an empty or non-error body (e.g. '{}') and assert the call rejects with the fallback message 'Failed to send magic link (500)'. Ensure the tests use the same pattern as existing tests (spyOn globalThis.fetch, mockResolvedValue with Response, and expect(...).rejects.toThrow(...)) and name them clearly (e.g., "throws the backend error message on non-OK responses" and "throws a status-based fallback when error body is missing") so they align with the current test suite around sendEmailMagicLink.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/services/api/__tests__/authApi.test.ts`:
- Around line 5-44: Add tests that exercise sendEmailMagicLink's non-OK response
handling: mock globalThis.fetch to resolve to a Response with status 400 and
body JSON { error: 'Invalid email format' } and assert
sendEmailMagicLink('bad-email', 'openhuman://') rejects with 'Invalid email
format'; also add a test where fetch resolves to a Response with status 500 and
an empty or non-error body (e.g. '{}') and assert the call rejects with the
fallback message 'Failed to send magic link (500)'. Ensure the tests use the
same pattern as existing tests (spyOn globalThis.fetch, mockResolvedValue with
Response, and expect(...).rejects.toThrow(...)) and name them clearly (e.g.,
"throws the backend error message on non-OK responses" and "throws a
status-based fallback when error body is missing") so they align with the
current test suite around sendEmailMagicLink.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dd976874-4101-49ef-a9b7-1d5784025bdf
📒 Files selected for processing (4)
app/src/pages/Welcome.tsxapp/src/pages/__tests__/Welcome.test.tsxapp/src/services/api/__tests__/authApi.test.tsapp/src/services/api/authApi.ts
Summary
Welcomescreen alongside the existing OAuth buttons.frontendRedirectUri(openhuman://for Tauri, current origin for web).sendEmailMagicLink(...)frontend API helper that calls the backend/auth/email/send-linkendpoint.Problem
Solution
sendEmailMagicLink(email, frontendRedirectUri)to POST to the backend auth endpoint.openhuman://so the backend can redirect back into the existing deep-link handler.window.location.originso the SPA can continue handling auth in-browser.Submission Checklist
app/test/e2e, mock backend,tests/json_rpc_e2e.rsas appropriate)sendEmailMagicLink(...)and brief inline note for the desktop deep-link constant(Any feature related checklist can go in here)
Impact
Related
Summary by CodeRabbit
New Features
Tests