Resolve runtime product name in passkey flow templates#2996
Closed
RushanNanayakkara wants to merge 4 commits into
Closed
Resolve runtime product name in passkey flow templates#2996RushanNanayakkara wants to merge 4 commits into
RushanNanayakkara wants to merge 4 commits into
Conversation
Replaces the hardcoded "ThunderID" relyingPartyName string in flow
builder seed JSON with a {{productName}} placeholder, and resolves
it at hook-load time from config.brand.product_name. This is the
only end-user-visible surface that the existing brand configuration
mechanism could not previously reach -- the WebAuthn browser dialog
now reads the deployment's configured product name instead of the
literal "ThunderID".
Reuses the existing updateTemplatePlaceholderReferences util so no
new placeholder syntax or substitution machinery is introduced. The
hooks' return shapes are unchanged, and the admin-facing Passkey
properties panel continues to surface the resolved value as the
default (admins can still override it).
Contributor
📝 WalkthroughWalkthroughThis PR replaces hardcoded "ThunderID" with a {{productName}} placeholder in passkey JSON resources and resolves that placeholder at runtime from config.brand.product_name in both flow-builder and login-flow resource hooks; tests mock useConfig and assert substitution. ChangesDynamic Brand Resolution Across Flow Resources
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The eslint-disable comment matches the established pattern in ApplicationCreate/__tests__/useApplicationCreate.test.tsx. The assertion is needed because vitest's importOriginal() returns `unknown` and we spread it into the mock implementation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/apps/console/src/features/login-flow/api/useGetLoginFlowBuilderResources.ts (1)
61-64: 💤 Low valueConsider extending
Resourcestype to includeexecutors.The type assertion
(coreResources as Resources & {executors?: unknown[]})suggests theResourcestype definition doesn't include anexecutorsproperty. If executors are a legitimate part of the resources structure, updating the type would provide better type safety and eliminate the need for this assertion.🤖 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/api/useGetLoginFlowBuilderResources.ts` around lines 61 - 64, The code is using a type assertion for coreResources to add an executors property, which indicates Resources lacks that field; update the Resources type definition to include executors?: ExecutorType[] (or unknown[] if ExecutorType isn't defined) so coreResources naturally typesafe contains executors, then remove the inline cast in useGetLoginFlowBuilderResources.ts and rely on coreResources.executors together with resolvedLocal.executors; ensure you update the Resources interface/ type declaration where it's defined and adapt any imports/usages of Resources accordingly.
🤖 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.
Nitpick comments:
In
`@frontend/apps/console/src/features/login-flow/api/useGetLoginFlowBuilderResources.ts`:
- Around line 61-64: The code is using a type assertion for coreResources to add
an executors property, which indicates Resources lacks that field; update the
Resources type definition to include executors?: ExecutorType[] (or unknown[] if
ExecutorType isn't defined) so coreResources naturally typesafe contains
executors, then remove the inline cast in useGetLoginFlowBuilderResources.ts and
rely on coreResources.executors together with resolvedLocal.executors; ensure
you update the Resources interface/ type declaration where it's defined and
adapt any imports/usages of Resources accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 08751a2f-a009-4b07-b54b-b52471fb39ee
📒 Files selected for processing (1)
frontend/apps/console/src/features/login-flow/api/useGetLoginFlowBuilderResources.ts
The Resources interface already declares executors: Step[], so the inline intersection cast was redundant. Removing it as suggested by CodeRabbit.
4 tasks
Contributor
Author
|
Superseded by #2997 (squashed to a single commit on a renamed branch). Same diff. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
1 participant
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
Fixes #2995.
Resolves the only end-user-visible surface that the existing
configuration.brand.productNamemechanism could not previously reach: the WebAuthn passkey relying-party name shipped in the flow-builder seed JSON.What changed
"ThunderID"with the"{{productName}}"placeholder in 12 lines across 4 seed JSON files (flows/data/templates.json,login-flow/data/{templates,executors,widgets}.json).useGetFlowBuilderCoreResourcesanduseGetLoginFlowBuilderResourcesnow resolve the placeholder at load time using the existingupdateTemplatePlaceholderReferencesutil andconfig.brand.product_namefromuseConfig().useConfig, and added a substitution assertion inuseGetFlowBuilderCoreResources.test.ts.No new util, no new placeholder syntax — reuses the
{{key}}mechanism already present for ID generation and i18n. Hook return shapes are unchanged.Test plan
pnpm test useGetFlowBuilderCoreResources useGetLoginFlowBuilderResources— 38/38 passingpnpm testandpnpm lintinfrontend/apps/console— covered by CIconfiguration.brand.productNameoverridden in Helm values, instantiate a passkey-based template and verify the relying-party-name field defaults to the configured product name in the Passkey properties panel."ThunderID").