Implement Magic Link Frontend#2987
Open
RandithaK wants to merge 39 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces Magic Link passwordless authentication by refactoring the backend executor, designing multi-step frontend flows for authentication and registration, integrating SDK callback handling, and migrating cross-SDK storage from sessionStorage to localStorage for better persistence across tabs. ChangesMagic Link Authentication Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
frontend/apps/console/src/features/flows/components/create-flow/SelectFlowTemplate.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. frontend/apps/console/src/features/flows/models/templates.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/ExecutionExtendedProperties.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
RandithaK
force-pushed
the
feature/Magic-Link-Frontend
branch
from
48c1002 to
5c19cca
Compare
| Path string `yaml:"path" json:"path"` | ||
| LoginPath string `yaml:"login_path" json:"login_path"` | ||
| ErrorPath string `yaml:"error_path" json:"error_path"` | ||
| MagicLinkPath string `yaml:"magic_link_path" json:"magic_link_path"` |
Member
There was a problem hiding this comment.
Let's not term this a magic link specific config, rather have it as a generic callback path for any internal returns. But don't make this misleading with external IDP callback path in the SDK too
Contributor
Author
There was a problem hiding this comment.
We are having a discussion in
#2736
Contributions are welcome 👍
| Config Config `yaml:"config"` | ||
| ServerHome string `yaml:"server_home"` | ||
| GateClientLoginURL *url.URL | ||
| GateClientMagicLinkURL *url.URL |
Member
There was a problem hiding this comment.
Same as previous comment
Contributor
Author
There was a problem hiding this comment.
We are having a discussion in
#2736
Contributions are welcome 👍
RandithaK
force-pushed
the
feature/Magic-Link-Frontend
branch
2 times, most recently
from
4b90eed to
e49b2c9
Compare
# Conflicts: # samples/apps/react-sdk-sample/src/main.tsx
…ionStorage across SDKs
RandithaK
force-pushed
the
feature/Magic-Link-Frontend
branch
from
e49b2c9 to
1b199b2
Compare
RandithaK
force-pushed
the
feature/Magic-Link-Frontend
branch
from
178de30 to
9e4a528
Compare
RandithaK
requested review from
DonOmalVindula,
brionmario,
jeradrutnam and
rajithacharith
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds end-to-end Magic Link authentication support across backend, SDKs, router integrations, and console flow templates/config to enable passwordless sign-in/registration via emailed links.
Changes:
- Introduces Magic Link callback handling in
@thunderid/reactand aMagicLinkRoutewrapper in@thunderid/react-router. - Updates executor naming and flow templates/config (backend + console) to use
MagicLinkExecutorwithgenerate/verifymodes. - Adjusts storage behavior (notably
authIdand PKCE-related data) and adds runtime config support for a dedicated Magic Link callback path.
Reviewed changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/flow/authentication/magic_link_test.go | Updates integration test flow executor name to MagicLinkExecutor. |
| sdks/vue/src/components/auth/sign-in/v2/SignIn.ts | Switches authId persistence from sessionStorage to localStorage. |
| sdks/vue/src/ThunderIDVueClient.ts | Switches thunderid_auth_id lookup/persist to localStorage. |
| sdks/react/src/index.ts | Exports new MagicLinkCallback component. |
| sdks/react/src/contexts/ThunderID/ThunderIDContext.ts | Removes storage from context props/defaults. |
| sdks/react/src/components/presentation/auth/SignIn/v2/SignIn.tsx | Switches thunderid_auth_id persistence from sessionStorage to localStorage. |
| sdks/react/src/components/auth/Callback/tests/MagicLinkCallback.test.tsx | Adds tests for MagicLinkCallback. |
| sdks/react/src/components/auth/Callback/MagicLinkCallback.tsx | Adds Magic Link callback handler component. |
| sdks/react/src/ThunderIDReactClient.ts | Switches thunderid_auth_id lookup/persist to localStorage. |
| sdks/react-router/src/index.ts | Exports new MagicLinkRoute. |
| sdks/react-router/src/components/MagicLinkRoute.tsx | Adds a React Router wrapper around MagicLinkCallback. |
| sdks/react-router/src/tests/index.test.ts | Verifies MagicLinkRoute export. |
| sdks/javascript/src/StorageManager.ts | Stores PKCE verifier-related temporary data in localStorage and manages cleanup. |
| sdks/browser/src/utils/SPAUtils.ts | Switches PKCE get/set/remove from sessionStorage to localStorage. |
| samples/apps/react-sdk-sample/package.json | Uses workspace dependency for @thunderid/react. |
| frontend/packages/i18n/src/locales/en-US.ts | Adds Magic Link translation keys for sign-in/sign-up flows. |
| frontend/apps/gate/vite.config.ts | Adds react-router resolve alias for linked packages. |
| frontend/apps/gate/src/constants/routes.ts | Adds ROUTES.AUTH.MAGIC_LINK. |
| frontend/apps/gate/src/constants/tests/routes.test.ts | Adds coverage for ROUTES.AUTH.MAGIC_LINK. |
| frontend/apps/gate/src/config/appRoutes.tsx | Registers /magiclink route using MagicLinkRoute. |
| frontend/apps/console/vite.config.ts | Adds react-router resolve alias for linked packages. |
| frontend/apps/console/src/features/login-flow/data/widgets.json | Adds Magic Link widget definition. |
| frontend/apps/console/src/features/login-flow/data/templates.json | Adds Magic Link template definition for flow builder. |
| frontend/apps/console/src/features/login-flow/data/steps.json | Adds an Email view step definition used by Magic Link flows. |
| frontend/apps/console/src/features/login-flow/data/executors.json | Adds Magic Link executor steps for generate and verify modes. |
| frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/execution-properties/constants.ts | Adds MAGIC_LINK_MODES options. |
| frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/execution-properties/tests/MagicLinkProperties.test.tsx | Adds tests for Magic Link mode selection UI. |
| frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/execution-properties/MagicLinkProperties.tsx | Adds executor properties UI for selecting Magic Link mode. |
| frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/ExecutionExtendedProperties.tsx | Wires Magic Link executor to use MagicLinkProperties. |
| frontend/apps/console/src/features/flows/models/templates.ts | Adds MAGIC_LINK and BASIC_MAGIC_LINK template types. |
| frontend/apps/console/src/features/flows/data/templates.json | Adds Magic Link templates for authentication/registration (incl. combined basic+magic). |
| frontend/apps/console/src/features/flows/components/create-flow/SelectFlowTemplate.tsx | Adds icons for the new Magic Link templates. |
| backend/internal/system/config/runtimeconfig_test.go | Adds test for invalid magic link path fallback; minor hostname constant reuse. |
| backend/internal/system/config/runtimeconfig.go | Adds GateClientMagicLinkURL and magic link path parsing/fallback. |
| backend/internal/system/config/config.go | Adds MagicLinkPath to gate client config and defaults it; introduces localhost const reuse. |
| backend/internal/flow/executor/magic_link_executor_test.go | Renames executor/test suite and adds applicationId propagation + execution policy test. |
| backend/internal/flow/executor/magic_link_executor.go | Renames executor, adds GetExecutionPolicy, and includes applicationId in generated link params. |
| backend/internal/flow/executor/init.go | Registers executor under ExecutorNameMagicLink. |
| backend/internal/flow/executor/constants.go | Renames executor constant to ExecutorNameMagicLink. |
| backend/internal/authn/magiclink/service_test.go | Updates runtime config and expectation to use /gate/magiclink path. |
| backend/internal/authn/magiclink/service.go | Uses GateClientMagicLinkURL when building magic link URLs. |
| backend/cmd/server/bootstrap/i18n/en-US.json | Adds Magic Link translation keys for server bootstrap i18n. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (7)
sdks/react/src/components/auth/Callback/tests/MagicLinkCallback.test.tsx:1
- The test sets the callback URL parameter as
executionId=..., butMagicLinkCallbackreads the execution id from theidquery parameter (urlParams.get('id')). As written, this test should fail becauseexecutionIdwill benullin the component and the flow will go through the error redirect path. Update the test URL to useid=exec-1(or adjust component parsing to accept bothidandexecutionIdconsistently with how links are generated).
sdks/react/src/components/auth/Callback/tests/MagicLinkCallback.test.tsx:1 - The test sets the callback URL parameter as
executionId=..., butMagicLinkCallbackreads the execution id from theidquery parameter (urlParams.get('id')). As written, this test should fail becauseexecutionIdwill benullin the component and the flow will go through the error redirect path. Update the test URL to useid=exec-1(or adjust component parsing to accept bothidandexecutionIdconsistently with how links are generated).
sdks/react/src/components/auth/Callback/MagicLinkCallback.tsx:1 - The component reads the execution id from the
idquery parameter, but the error message says “Missing executionId...”, which is misleading and also conflicts with the test expectations. Consider either (mandatory) updating the message to referenceid(e.g., “Missing id or token...”), or (optional) supporting bothidandexecutionIdfor backward compatibility and keeping the message aligned with the accepted parameter names.
sdks/react/src/components/auth/Callback/MagicLinkCallback.tsx:1 - The component reads the execution id from the
idquery parameter, but the error message says “Missing executionId...”, which is misleading and also conflicts with the test expectations. Consider either (mandatory) updating the message to referenceid(e.g., “Missing id or token...”), or (optional) supporting bothidandexecutionIdfor backward compatibility and keeping the message aligned with the accepted parameter names.
sdks/react/src/components/auth/Callback/MagicLinkCallback.tsx:1 crypto.randomUUID()isn’t available in all runtime environments (older browsers, some test runners, or restricted contexts), which can cause a runtime crash in the callback route. Use a safe fallback whencrypto.randomUUIDis unavailable (e.g., generate a random string withcrypto.getRandomValuesor a small helper).
sdks/react/src/contexts/ThunderID/ThunderIDContext.ts:1- Removing
Pick<ThunderIDReactConfig, 'storage'>fromThunderIDContextPropsis a breaking type change for SDK consumers that relied onstoragebeing available via context (and the default context value previously includedstorage: 'sessionStorage'). If the storage option is being deprecated/removed in favor of a fixed behavior, it should be reflected in public API docs/changelog and ideally accompanied by a clear migration path; otherwise, consider keeping the field (even deprecated) to avoid breaking downstream builds.
sdks/javascript/src/StorageManager.ts:1 - Iterating
localStorageviaObject.keys(localStorage)is not the most robust way to enumerate storage entries across environments; the standard approach is iteratinglocalStorage.lengthand callinglocalStorage.key(i). Consider switching to the standard iteration pattern here (and similarly inremoveTemporaryData) to avoid missing keys in edge environments and to be consistent with the Storage API.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@backend/internal/authn/magiclink/service.go`:
- Line 199: Update the docs to reflect the new config field and routing change:
document the new GateClientMagicLinkPath config (environment variable name, YAML
key, default value and semantics) and update any guides that reference magic
link callbacks to describe the new URL structure produced by
config.GetServerRuntime().GateClientMagicLinkURL (previously
GateClientLoginURL), plus deployment notes on how to set the new path; ensure
README/docs pages that mention the magic link callback, client endpoint, or
GateClientLoginURL are updated to reference
GateClientMagicLinkURL/GateClientMagicLinkPath and include examples of env/YAML
usage and expected callback URLs.
In `@backend/internal/system/config/config.go`:
- Around line 76-83: Add documentation updates for the new magic link
configuration and executor contract: document the
server.gate_client.magic_link_path config option (field MagicLinkPath in the
config struct) including its purpose, default value pattern
"<gate_client.path>/magiclink" and expected URL format in
docs/content/guides/getting-started/configuration.mdx; update
backend/authentication flow docs to reflect the MagicLinkExecutor rename (was
MagicLinkAuthExecutor), the two execution modes "generate" and "verify" and the
verify execution policy semantics and how flow authors should use them; and
update API/authentication docs to show the magic link callback URL/query
contract now includes applicationId so frontend/backends know how to construct
and validate callback requests (add examples and expected query parameters) in
docs/content/apis.mdx or a guides authentication doc.
In `@frontend/apps/console/src/features/login-flow/data/templates.json`:
- Around line 1593-1641: The template advances directly from the SEND_EMAIL_ID
task (EmailExecutor send) to VERIFY_LINK_ID (MagicLinkExecutor verify) which
skips the intermediate "email_sent" view that must wait for the user/callback
token; add an intermediate node (e.g., EMAIL_SENT_VIEW_ID) representing the
email_sent prompt/view and change the SEND_EMAIL_ID action.onSuccess to point to
EMAIL_SENT_VIEW_ID, then have that view transition (on user action or callback)
to VERIFY_LINK_ID so MagicLinkExecutor.verify only runs after the token/input is
provided; ensure any references like EmailExecutor, MagicLinkExecutor,
SEND_EMAIL_ID, VERIFY_LINK_ID, and AUTH_ASSERT_ID are updated to use the new
EMAIL_SENT_VIEW_ID in the flow.
- Around line 1665-1675: The END node with id "END" currently lacks the
completion executor, so update its node object to include the same executor
wiring used elsewhere: add an executor entry referencing
"LoginCompletionExecutor" (the post-auth completion/redirect handler) so that
when the flow reaches the END node—especially after "AuthAssertExecutor"
succeeds—it triggers the normal completion/redirect path; locate the node with
"id": "END" in templates.json and add the executor configuration consistent with
other templates that wire END through LoginCompletionExecutor.
In `@frontend/apps/console/src/features/login-flow/data/widgets.json`:
- Around line 1231-1259: The three component IDs in the reusable email-sent
widget are hard-coded and will collide when the widget is instantiated multiple
times; replace the fixed ids "image_logo_sent", "display_email_sent_heading",
and "display_email_sent_desc" with generated ID placeholders (e.g. use the
existing {{ID}} pattern used elsewhere in the widget like "{{ID}}_image_logo",
"{{ID}}_heading", "{{ID}}_description") so each instance gets unique ids and
update any internal references to those ids accordingly.
In `@sdks/react-router/src/index.ts`:
- Around line 25-26: Add documentation for the new Magic Link SDK components:
create docs for MagicLinkRoute and MagicLinkCallback under docs/content/sdks/
(or a new guide in docs/content/guides/) that describe each component's purpose,
public props (with types and defaults), usage examples showing how to import and
render MagicLinkRoute and MagicLinkCallback in a React Router app, and
integration steps (routing, expected URL/callback flow). Also include an
end-to-end Magic Link authentication section covering necessary backend
configuration (token/URL generation, callback endpoint), how the frontend
triggers and verifies the link, error handling and redirects, and
cross-references between the React and React Router SDK examples.
In `@sdks/react/src/components/auth/Callback/MagicLinkCallback.tsx`:
- Around line 111-123: The code reads the wrong query param key: change the
urlParams.get call that assigns executionId (currently using 'id') to use
'executionId' so the Magic Link callback recognizes the real parameter; update
the line that sets executionId (where executionId: string | null =
urlParams.get('id')) to urlParams.get('executionId') and keep the existing
clearTokenFromUrl(), error handling, and redirectWithError usage unchanged so
valid callbacks no longer trigger the missing-parameter branch.
In `@sdks/react/src/ThunderIDReactClient.ts`:
- Around line 280-281: The code reads/writes a global localStorage key
'thunderid_auth_id' (see authIdFromStorage and authId usage) which can collide
across tabs/instances; replace this with a namespaced storage helper that
composes the key from client/application id and a per-instance instanceId (e.g.,
`${appId}:${instanceId}:thunderid_auth_id`) and use that helper for all
get/set/remove operations; update the reads (authIdFromStorage), writes, and
cleanup logic (also at the other usage around lines 344-349) to call the shared
helper functions (e.g., getNamespacedAuthId, setNamespacedAuthId,
removeNamespacedAuthId) so each client instance uses its own scoped key and
avoids stale/cross-flow collisions.
In `@sdks/vue/src/ThunderIDVueClient.ts`:
- Around line 310-311: Replace the hardcoded global localStorage key
'thunderid_auth_id' with a scoped key and centralized helper: create a helper
(e.g., getScopedAuthKey(instanceIdOrClientId)) and accessor functions
(getAuthIdFromStorage(), setAuthIdToStorage(id), clearAuthIdFromStorage()) and
use those in ThunderIDVueClient where authIdFromStorage, authIdFromUrl and
authId are computed (and in the other occurrences around the file). The scoped
key should include a unique identifier for the client/instance/application
(e.g., `${baseKey}:${clientId}`) so stored auth IDs don’t bleed across tabs, and
replace direct localStorage.getItem/setItem/removeItem calls with the new helper
functions everywhere in this file.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a4c5136-930b-401d-b8e0-0697a3a70e7e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (42)
backend/cmd/server/bootstrap/i18n/en-US.jsonbackend/internal/authn/magiclink/service.gobackend/internal/authn/magiclink/service_test.gobackend/internal/flow/executor/constants.gobackend/internal/flow/executor/init.gobackend/internal/flow/executor/magic_link_executor.gobackend/internal/flow/executor/magic_link_executor_test.gobackend/internal/system/config/config.gobackend/internal/system/config/runtimeconfig.gobackend/internal/system/config/runtimeconfig_test.gofrontend/apps/console/src/features/flows/components/create-flow/SelectFlowTemplate.tsxfrontend/apps/console/src/features/flows/data/templates.jsonfrontend/apps/console/src/features/flows/models/templates.tsfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/ExecutionExtendedProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/execution-properties/MagicLinkProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/execution-properties/__tests__/MagicLinkProperties.test.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/execution-properties/constants.tsfrontend/apps/console/src/features/login-flow/data/executors.jsonfrontend/apps/console/src/features/login-flow/data/steps.jsonfrontend/apps/console/src/features/login-flow/data/templates.jsonfrontend/apps/console/src/features/login-flow/data/widgets.jsonfrontend/apps/console/vite.config.tsfrontend/apps/gate/src/config/appRoutes.tsxfrontend/apps/gate/src/constants/__tests__/routes.test.tsfrontend/apps/gate/src/constants/routes.tsfrontend/apps/gate/vite.config.tsfrontend/packages/i18n/src/locales/en-US.tssamples/apps/react-sdk-sample/package.jsonsdks/browser/src/utils/SPAUtils.tssdks/javascript/src/StorageManager.tssdks/react-router/src/__tests__/index.test.tssdks/react-router/src/components/MagicLinkRoute.tsxsdks/react-router/src/index.tssdks/react/src/ThunderIDReactClient.tssdks/react/src/components/auth/Callback/MagicLinkCallback.tsxsdks/react/src/components/auth/Callback/__tests__/MagicLinkCallback.test.tsxsdks/react/src/components/presentation/auth/SignIn/v2/SignIn.tsxsdks/react/src/contexts/ThunderID/ThunderIDContext.tssdks/react/src/index.tssdks/vue/src/ThunderIDVueClient.tssdks/vue/src/components/auth/sign-in/v2/SignIn.tstests/integration/flow/authentication/magic_link_test.go
|
|
||
| if u == nil { | ||
| u = config.GetServerRuntime().GateClientLoginURL | ||
| u = config.GetServerRuntime().GateClientMagicLinkURL |
Contributor
There was a problem hiding this comment.
🔴 Documentation required for configuration change.
This change switches the magic link URL fallback from GateClientLoginURL to GateClientMagicLinkURL, introducing a new configuration field GateClientMagicLinkPath. This is both a configuration option change and a user-facing behavior change (magic links now route to a different client endpoint).
Per the coding guidelines, configuration options and user-facing behavior changes require corresponding documentation updates under docs/. Please add:
- Configuration documentation for
GateClientMagicLinkPath(environment variable, YAML key, default value, behavior) - Guide updates explaining the magic link callback endpoint and URL structure
- Any relevant deployment notes about the new path configuration
🤖 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 `@backend/internal/authn/magiclink/service.go` at line 199, Update the docs to
reflect the new config field and routing change: document the new
GateClientMagicLinkPath config (environment variable name, YAML key, default
value and semantics) and update any guides that reference magic link callbacks
to describe the new URL structure produced by
config.GetServerRuntime().GateClientMagicLinkURL (previously
GateClientLoginURL), plus deployment notes on how to set the new path; ensure
README/docs pages that mention the magic link callback, client endpoint, or
GateClientLoginURL are updated to reference
GateClientMagicLinkURL/GateClientMagicLinkPath and include examples of env/YAML
usage and expected callback URLs.
Contributor
Author
There was a problem hiding this comment.
@coderabbitai
Documentation would be taken in a separate PR
Contributor
There was a problem hiding this comment.
@RandithaK, understood — I'll defer the documentation requirement to that separate PR.
Would you like me to open a GitHub issue to track the documentation updates needed for this change? That would cover:
- Configuration docs for
GateClientMagicLinkPath(env var, YAML key, default value) - Guide updates for the magic link callback endpoint and URL structure
- Deployment notes for the new path configuration
🧠 Learnings used
Learnt from: KaveeshaPiumini
Repo: thunder-id/thunderid PR: 2835
File: backend/internal/agent/declarative_resource.go:21-39
Timestamp: 2026-05-20T05:39:33.413Z
Learning: Do not treat importing a package named "testing" in production Go files as an anti-pattern in this codebase. For .go files that are not *_test.go, this convention is used to expose *ForTest constructors (e.g., NewAgentExporterForTest, NewApplicationExporterForTest) that are guarded by a call like testing.Testing() and panic when not running under tests. When reviewing, only flag importing "testing" in non-test files if it is not part of this established *ForTest/guard+panic convention.
Comment on lines
+76
to
83
| Hostname string `yaml:"hostname" json:"hostname"` | ||
| Port int `yaml:"port" json:"port"` | ||
| Scheme string `yaml:"scheme" json:"scheme"` | ||
| Path string `yaml:"path" json:"path"` | ||
| LoginPath string `yaml:"login_path" json:"login_path"` | ||
| ErrorPath string `yaml:"error_path" json:"error_path"` | ||
| MagicLinkPath string `yaml:"magic_link_path" json:"magic_link_path"` | ||
| } |
Contributor
There was a problem hiding this comment.
🔴 Documentation Required
This PR introduces user-facing changes that are not covered by documentation updates under docs/.
Please update the relevant documentation before merging.
Missing documentation:
server.gate_client.magic_link_pathconfiguration option: document purpose/default (<gate_client.path>/magiclink) and expected format indocs/content/guides/getting-started/configuration.mdx.- Magic Link executor contract changes (
MagicLinkAuthExecutor→MagicLinkExecutor,generate/verifymodes, verify execution policy semantics): document flow-authoring impact in the relevant backend/authentication flow docs underdocs/content/guides/. - Magic link callback URL/query contract update (now includes
applicationId): document backend/frontend integration expectations indocs/content/apis.mdxor an authentication guide underdocs/content/guides/.
As per coding guidelines: "If ANY of the above are detected and the PR does NOT include corresponding updates under docs/, post a single consolidated PR-level comment."
Also applies to: 757-759
🤖 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 `@backend/internal/system/config/config.go` around lines 76 - 83, Add
documentation updates for the new magic link configuration and executor
contract: document the server.gate_client.magic_link_path config option (field
MagicLinkPath in the config struct) including its purpose, default value pattern
"<gate_client.path>/magiclink" and expected URL format in
docs/content/guides/getting-started/configuration.mdx; update
backend/authentication flow docs to reflect the MagicLinkExecutor rename (was
MagicLinkAuthExecutor), the two execution modes "generate" and "verify" and the
verify execution policy semantics and how flow authors should use them; and
update API/authentication docs to show the magic link callback URL/query
contract now includes applicationId so frontend/backends know how to construct
and validate callback requests (add examples and expected query parameters) in
docs/content/apis.mdx or a guides authentication doc.
Contributor
Author
There was a problem hiding this comment.
@coderabbitai
Documentation would be taken in a separate PR
Contributor
There was a problem hiding this comment.
@RandithaK, understood! Would you like me to open a follow-up GitHub issue to track the missing documentation items so they don't get lost? I can capture all three items (the magic_link_path config option, the MagicLinkExecutor rename/modes, and the updated magic link callback URL contract) as actionable tasks in the issue.
🧠 Learnings used
Learnt from: KaveeshaPiumini
Repo: thunder-id/thunderid PR: 2835
File: backend/internal/agent/declarative_resource.go:21-39
Timestamp: 2026-05-20T05:39:33.413Z
Learning: Do not treat importing a package named "testing" in production Go files as an anti-pattern in this codebase. For .go files that are not *_test.go, this convention is used to expose *ForTest constructors (e.g., NewAgentExporterForTest, NewApplicationExporterForTest) that are guarded by a call like testing.Testing() and panic when not running under tests. When reviewing, only flag importing "testing" in non-test files if it is not part of this established *ForTest/guard+panic convention.
Comment on lines
+1593
to
+1641
| "onSuccess": "{{SEND_EMAIL_ID}}" | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| "id": "{{SEND_EMAIL_ID}}", | ||
| "type": "TASK_EXECUTION", | ||
| "size": { | ||
| "width": 200, | ||
| "height": 100 | ||
| }, | ||
| "position": { | ||
| "x": 650, | ||
| "y": 266 | ||
| }, | ||
| "data": { | ||
| "action": { | ||
| "type": "EXECUTOR", | ||
| "executor": { | ||
| "name": "EmailExecutor", | ||
| "mode": "send" | ||
| }, | ||
| "onSuccess": "{{VERIFY_LINK_ID}}" | ||
| }, | ||
| "properties": { | ||
| "emailTemplate": "MAGIC_LINK" | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| "id": "{{VERIFY_LINK_ID}}", | ||
| "type": "TASK_EXECUTION", | ||
| "size": { | ||
| "width": 200, | ||
| "height": 100 | ||
| }, | ||
| "position": { | ||
| "x": 950, | ||
| "y": 266 | ||
| }, | ||
| "data": { | ||
| "action": { | ||
| "type": "EXECUTOR", | ||
| "executor": { | ||
| "name": "MagicLinkExecutor", | ||
| "mode": "verify" | ||
| }, | ||
| "onSuccess": "{{AUTH_ASSERT_ID}}" | ||
| } |
Contributor
There was a problem hiding this comment.
Don't advance straight from email send to verification.
After the email is sent, this template immediately runs MagicLinkExecutor in verify mode, but there is no intermediate email_sent prompt/view to wait for the callback token. That makes the standalone template inconsistent with the widget flow and likely leaves verification without the required input.
🤖 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 `@frontend/apps/console/src/features/login-flow/data/templates.json` around
lines 1593 - 1641, The template advances directly from the SEND_EMAIL_ID task
(EmailExecutor send) to VERIFY_LINK_ID (MagicLinkExecutor verify) which skips
the intermediate "email_sent" view that must wait for the user/callback token;
add an intermediate node (e.g., EMAIL_SENT_VIEW_ID) representing the email_sent
prompt/view and change the SEND_EMAIL_ID action.onSuccess to point to
EMAIL_SENT_VIEW_ID, then have that view transition (on user action or callback)
to VERIFY_LINK_ID so MagicLinkExecutor.verify only runs after the token/input is
provided; ensure any references like EmailExecutor, MagicLinkExecutor,
SEND_EMAIL_ID, VERIFY_LINK_ID, and AUTH_ASSERT_ID are updated to use the new
EMAIL_SENT_VIEW_ID in the flow.
Comment on lines
+1665
to
+1675
| { | ||
| "id": "END", | ||
| "type": "END", | ||
| "size": { | ||
| "width": 85, | ||
| "height": 34 | ||
| }, | ||
| "position": { | ||
| "x": 1600, | ||
| "y": 300 | ||
| } |
Contributor
There was a problem hiding this comment.
Add the completion executor to the END node.
Every other starter template in this file wires END through LoginCompletionExecutor; this one ends with a bare node. That can skip the normal post-auth completion/redirect path after AuthAssertExecutor succeeds.
Suggested fix
{
"id": "END",
"type": "END",
"size": {
"width": 85,
"height": 34
},
"position": {
"x": 1600,
"y": 300
- }
+ },
+ "config": {},
+ "data": {
+ "action": {
+ "type": "EXECUTOR",
+ "executor": {
+ "name": "LoginCompletionExecutor"
+ }
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { | |
| "id": "END", | |
| "type": "END", | |
| "size": { | |
| "width": 85, | |
| "height": 34 | |
| }, | |
| "position": { | |
| "x": 1600, | |
| "y": 300 | |
| } | |
| { | |
| "id": "END", | |
| "type": "END", | |
| "size": { | |
| "width": 85, | |
| "height": 34 | |
| }, | |
| "position": { | |
| "x": 1600, | |
| "y": 300 | |
| }, | |
| "config": {}, | |
| "data": { | |
| "action": { | |
| "type": "EXECUTOR", | |
| "executor": { | |
| "name": "LoginCompletionExecutor" | |
| } | |
| } | |
| } | |
| } |
🤖 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 `@frontend/apps/console/src/features/login-flow/data/templates.json` around
lines 1665 - 1675, The END node with id "END" currently lacks the completion
executor, so update its node object to include the same executor wiring used
elsewhere: add an executor entry referencing "LoginCompletionExecutor" (the
post-auth completion/redirect handler) so that when the flow reaches the END
node—especially after "AuthAssertExecutor" succeeds—it triggers the normal
completion/redirect path; locate the node with "id": "END" in templates.json and
add the executor configuration consistent with other templates that wire END
through LoginCompletionExecutor.
Comment on lines
+1231
to
+1259
| "components": [ | ||
| { | ||
| "alt": "{{ t(signin:images.app_logo.alt) }}", | ||
| "category": "DISPLAY", | ||
| "height": "60", | ||
| "id": "image_logo_sent", | ||
| "resourceType": "ELEMENT", | ||
| "src": "{{ meta(application.logoUrl) }}", | ||
| "type": "IMAGE", | ||
| "width": "" | ||
| }, | ||
| { | ||
| "align": "center", | ||
| "category": "DISPLAY", | ||
| "id": "display_email_sent_heading", | ||
| "label": "{{ t(signin:forms.magicLink.emailSent.title) }}", | ||
| "resourceType": "ELEMENT", | ||
| "type": "TEXT", | ||
| "variant": "HEADING_1" | ||
| }, | ||
| { | ||
| "align": "center", | ||
| "category": "DISPLAY", | ||
| "id": "display_email_sent_desc", | ||
| "label": "{{ t(signin:forms.magicLink.emailSent.description) }}", | ||
| "resourceType": "ELEMENT", | ||
| "type": "TEXT", | ||
| "variant": "BODY_1" | ||
| } |
Contributor
There was a problem hiding this comment.
Use generated IDs for the email-sent view elements.
image_logo_sent, display_email_sent_heading, and display_email_sent_desc are fixed IDs inside a reusable widget. If this widget is added more than once, those IDs will collide in the flow graph. Use {{ID}} placeholders here like the rest of the widget.
Suggested fix
- "id": "image_logo_sent",
+ "id": "{{ID}}",
@@
- "id": "display_email_sent_heading",
+ "id": "{{ID}}",
@@
- "id": "display_email_sent_desc",
+ "id": "{{ID}}",🤖 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 `@frontend/apps/console/src/features/login-flow/data/widgets.json` around lines
1231 - 1259, The three component IDs in the reusable email-sent widget are
hard-coded and will collide when the widget is instantiated multiple times;
replace the fixed ids "image_logo_sent", "display_email_sent_heading", and
"display_email_sent_desc" with generated ID placeholders (e.g. use the existing
{{ID}} pattern used elsewhere in the widget like "{{ID}}_image_logo",
"{{ID}}_heading", "{{ID}}_description") so each instance gets unique ids and
update any internal references to those ids accordingly.
Comment on lines
+25
to
+26
| export {default as MagicLinkRoute} from './components/MagicLinkRoute'; | ||
| export * from './components/MagicLinkRoute'; |
Contributor
There was a problem hiding this comment.
🔴 Documentation Required
This PR introduces new SDK components that are not covered by documentation updates under docs/.
Please update the relevant SDK documentation before merging.
Missing documentation:
MagicLinkRoutecomponent (React Router SDK): Document the component's purpose, props, usage examples, and integration steps indocs/content/sdks/or create a Magic Link authentication guide underdocs/content/guides/.MagicLinkCallbackcomponent (React SDK): Document the component's purpose, props, usage examples, and how it handles the Magic Link verification flow indocs/content/sdks/.- Magic Link authentication flow: Provide end-to-end developer guidance covering both backend configuration and frontend integration across the React and React Router SDKs.
🤖 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 `@sdks/react-router/src/index.ts` around lines 25 - 26, Add documentation for
the new Magic Link SDK components: create docs for MagicLinkRoute and
MagicLinkCallback under docs/content/sdks/ (or a new guide in
docs/content/guides/) that describe each component's purpose, public props (with
types and defaults), usage examples showing how to import and render
MagicLinkRoute and MagicLinkCallback in a React Router app, and integration
steps (routing, expected URL/callback flow). Also include an end-to-end Magic
Link authentication section covering necessary backend configuration (token/URL
generation, callback endpoint), how the frontend triggers and verifies the link,
error handling and redirects, and cross-references between the React and React
Router SDK examples.
Contributor
Author
There was a problem hiding this comment.
@coderabbitai
Documentation would be taken in a separate PR
Contributor
There was a problem hiding this comment.
@RandithaK, understood! Deferring the documentation to a separate PR is a reasonable approach.
Would you like me to open a GitHub issue to track the documentation work for the Magic Link SDK components (MagicLinkRoute and MagicLinkCallback), so it doesn't get lost?
Comment on lines
+111
to
+123
| const executionId: string | null = urlParams.get('id'); | ||
| const token: string | null = urlParams.get('token'); | ||
| const applicationId: string | null = urlParams.get('applicationId'); | ||
|
|
||
| clearTokenFromUrl(); | ||
|
|
||
| if (!executionId || !token) { | ||
| const error = new Error('Missing executionId or token in Magic Link URL'); | ||
| // eslint-disable-next-line no-console | ||
| console.error('Magic Link callback error:', error); | ||
| redirectWithError(error); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Use the correct query parameter for execution ID.
Line 111 reads id, but the callback URL and follow-up navigation in this PR use executionId. This causes valid magic-link callbacks to fail with a missing-parameter error.
💡 Proposed fix
- const executionId: string | null = urlParams.get('id');
+ const executionId: string | null = urlParams.get('executionId') || urlParams.get('id');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const executionId: string | null = urlParams.get('id'); | |
| const token: string | null = urlParams.get('token'); | |
| const applicationId: string | null = urlParams.get('applicationId'); | |
| clearTokenFromUrl(); | |
| if (!executionId || !token) { | |
| const error = new Error('Missing executionId or token in Magic Link URL'); | |
| // eslint-disable-next-line no-console | |
| console.error('Magic Link callback error:', error); | |
| redirectWithError(error); | |
| return; | |
| } | |
| const executionId: string | null = urlParams.get('executionId') || urlParams.get('id'); | |
| const token: string | null = urlParams.get('token'); | |
| const applicationId: string | null = urlParams.get('applicationId'); | |
| clearTokenFromUrl(); | |
| if (!executionId || !token) { | |
| const error = new Error('Missing executionId or token in Magic Link URL'); | |
| // eslint-disable-next-line no-console | |
| console.error('Magic Link callback error:', error); | |
| redirectWithError(error); | |
| return; | |
| } |
🤖 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 `@sdks/react/src/components/auth/Callback/MagicLinkCallback.tsx` around lines
111 - 123, The code reads the wrong query param key: change the urlParams.get
call that assigns executionId (currently using 'id') to use 'executionId' so the
Magic Link callback recognizes the real parameter; update the line that sets
executionId (where executionId: string | null = urlParams.get('id')) to
urlParams.get('executionId') and keep the existing clearTokenFromUrl(), error
handling, and redirectWithError usage unchanged so valid callbacks no longer
trigger the missing-parameter branch.
Contributor
Author
There was a problem hiding this comment.
@coderabbitai
const executionId: string | null = urlParams.get('id');
Line 111.
I think it's a false positive
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what query params the backend uses when building the magic link URL
rg -n "queryParams\|executionId\|applicationId\|\"id\"\|'id'" backend/internal/authn/magiclink/service.go -A3 -B3
echo "---"
# Check the magic link executor for queryParams construction
rg -n "queryParams\|executionId\|applicationId\|\"id\"\|GenerateMagicLink" backend/internal/flow/executor/magic_link_executor.go -A5 -B2Length of output: 658
🏁 Script executed:
#!/bin/bash
# Find the magic link service file
fd "service.go" backend/internal/authn/magiclink/
echo "---"
# Find the magic link executor file
fd "magic_link_executor.go" backend/
echo "=== magic link service ==="
cat backend/internal/authn/magiclink/service.go | grep -n "id\|token\|query\|param\|URL\|url" | head -50
echo "=== magic link executor ==="
cat backend/internal/flow/executor/magic_link_executor.go | grep -n "queryParam\|\"id\"\|executionId\|applicationId\|GenerateMagicLink" | head -50Length of output: 4322
@RandithaK, you're absolutely right — this is a false positive on my part. The backend magic link executor explicitly sets "id" (not "executionId") in the query parameters:
// backend/internal/flow/executor/magic_link_executor.go
queryParams := map[string]string{
"id": ctx.ExecutionID,
"applicationId": ctx.Application.ID,
}So urlParams.get('id') on line 111 correctly reads the execution ID from the magic link callback URL. Apologies for the noise!
(∩｀-´)⊃━☆ﾟ.*･｡ﾟ
✏️ Learnings added
Learnt from: RandithaK
Repo: thunder-id/thunderid PR: 2987
File: sdks/react/src/components/auth/Callback/MagicLinkCallback.tsx:111-123
Timestamp: 2026-05-27T09:18:12.313Z
Learning: In `sdks/react/src/components/auth/Callback/MagicLinkCallback.tsx`, the magic link callback URL uses `id` (not `executionId`) as the query parameter for the execution ID. This is because the backend (`backend/internal/flow/executor/magic_link_executor.go`) explicitly builds the queryParams map with `"id": ctx.ExecutionID`. So `urlParams.get('id')` is the correct way to retrieve the execution ID in the frontend callback.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Comment on lines
+280
to
281
| const authIdFromStorage: string = localStorage.getItem('thunderid_auth_id') ?? ''; | ||
| const authId: string = authIdFromUrl || authIdFromStorage; |
Contributor
There was a problem hiding this comment.
Scope thunderid_auth_id to instance/client to prevent stale and cross-flow collisions.
Using a global localStorage key for auth flow state can mix concurrent flows across tabs/instances and reuse stale authId after browser restarts. Please namespace this key (e.g., include instanceId and client/application identity) and read/write/remove through a shared helper.
Suggested direction
-const authIdFromStorage: string = localStorage.getItem('thunderid_auth_id') ?? '';
+const authKey = `thunderid_auth_id_${this.getInstanceId()}`;
+const authIdFromStorage: string = localStorage.getItem(authKey) ?? '';
-if (authIdFromUrl && !authIdFromStorage) {
- localStorage.setItem('thunderid_auth_id', authIdFromUrl);
+if (authIdFromUrl && !authIdFromStorage) {
+ localStorage.setItem(authKey, authIdFromUrl);
}Also applies to: 344-349
🤖 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 `@sdks/react/src/ThunderIDReactClient.ts` around lines 280 - 281, The code
reads/writes a global localStorage key 'thunderid_auth_id' (see
authIdFromStorage and authId usage) which can collide across tabs/instances;
replace this with a namespaced storage helper that composes the key from
client/application id and a per-instance instanceId (e.g.,
`${appId}:${instanceId}:thunderid_auth_id`) and use that helper for all
get/set/remove operations; update the reads (authIdFromStorage), writes, and
cleanup logic (also at the other usage around lines 344-349) to call the shared
helper functions (e.g., getNamespacedAuthId, setNamespacedAuthId,
removeNamespacedAuthId) so each client instance uses its own scoped key and
avoids stale/cross-flow collisions.
Comment on lines
+310
to
311
| const authIdFromStorage: string | null = localStorage.getItem('thunderid_auth_id'); | ||
| const authId: string = authIdFromUrl || authIdFromStorage || ''; |
Contributor
There was a problem hiding this comment.
Use a scoped auth-id storage key instead of global thunderid_auth_id.
The new localStorage fallback persists across tabs/sessions, so a global key can bind a flow to the wrong tab/instance and revive stale authId. Scope the key (instance/client/application) and centralize access in one helper.
Also applies to: 375-380
🤖 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 `@sdks/vue/src/ThunderIDVueClient.ts` around lines 310 - 311, Replace the
hardcoded global localStorage key 'thunderid_auth_id' with a scoped key and
centralized helper: create a helper (e.g.,
getScopedAuthKey(instanceIdOrClientId)) and accessor functions
(getAuthIdFromStorage(), setAuthIdToStorage(id), clearAuthIdFromStorage()) and
use those in ThunderIDVueClient where authIdFromStorage, authIdFromUrl and
authId are computed (and in the other occurrences around the file). The scoped
key should include a unique identifier for the client/instance/application
(e.g., `${baseKey}:${clientId}`) so stored auth IDs don’t bleed across tabs, and
replace direct localStorage.getItem/setItem/removeItem calls with the new helper
functions everywhere in this file.
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
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.
Purpose
This PR implements the end-to-end Magic Link Authentication flow across the frontend, SDKs, and backend. It introduces Magic Link routing and callback handling in the React ecosystem, updates the flow/login executors in the backend and Console UI, and refactors storage handling in the JavaScript SDK.
Approach
MagicLinkCallbackto the React SDK andMagicLinkRouteto the React-Router SDK to handle the magic link verification process, route configuration, and redirect flows.StorageManager.tsandSPAUtils.tsin the JavaScript/Browser SDKs to replace custom storage abstractions with standardsessionStoragemechanism.magic_link_executor.goto include theapplicationIDin query parameters and enhanced the flow with execution policies.email_sentprompts to notify users during the verification process.templates.json,steps.json,executors.json, andwidgets.json) to natively support Magic Link flow templates and execution properties.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Release Notes
New Features
Improvements