feat(settings): add v1 api token and token reveal popup#1045
Open
fayazg wants to merge 3 commits into
Open
Conversation
Surface the user-scoped v1 API Gateway token (audience api-gw.*) alongside the existing v2 OIDC token in the account settings developer section, so users migrating off the Individual Dashboard can keep calling v1 APIs. The v1 token is minted by the auth middleware via refresh-token exchange and returned by GET /api/profile/developer as an optional v1Token; the row is hidden when unavailable. Replace the inline show/hide toggle with a reveal popup (DialogService dynamic component) showing the full token on a single scrollable line with Copy and Close; the card keeps only the masked preview. Remove the redundant /profile/developer page (redirect to /settings) and point the header "Developer Settings" menu item at /settings. LFXV2-2542 Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds a typed developer token response with optional v1 data, shows tokens in a popup dialog, updates account settings token handling and UI for v1/v2 tokens, and redirects developer navigation to settings with anchor scrolling enabled. ChangesDeveloper Token v1/v2 Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for exposing a user-scoped v1 API Gateway token in the existing developer settings area and replaces inline token reveal with a dedicated popup dialog, while removing/redirecting the legacy /profile/developer page to consolidate the experience under /settings.
Changes:
- Extend developer token API/typing to optionally include a v1 API Gateway token (
v1Token) alongside the existing v2 bearer token. - Introduce a dynamic “token reveal” dialog and update account settings UI to show v2 and (when available) v1 token rows with masked previews.
- Redirect legacy developer settings route and update header navigation to point to
/settings#developer-settings.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/profile.interface.ts | Adds DeveloperTokenInfo interface including optional v1Token. |
| apps/lfx-one/src/server/controllers/profile.controller.ts | Surfaces req.apiGatewayToken as optional v1Token and logs has_v1_token. |
| apps/lfx-one/src/app/shared/services/user.service.ts | Types getDeveloperTokenInfo() to DeveloperTokenInfo. |
| apps/lfx-one/src/app/shared/components/token-reveal-dialog/token-reveal-dialog.component.ts | Adds new dynamic dialog component for token reveal/copy. |
| apps/lfx-one/src/app/shared/components/token-reveal-dialog/token-reveal-dialog.component.html | Dialog markup for wrapped, scrollable token display with Copy/Close. |
| apps/lfx-one/src/app/shared/components/header/header.component.ts | Updates “Developer Settings” menu item to /settings with fragment. |
| apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts | Adds v1 token state, dialog open handler, and updated copy handler. |
| apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.html | Renders separate v2 + conditional v1 token rows; uses reveal popup. |
| apps/lfx-one/src/app/modules/profile/profile.routes.ts | Redirects /profile/developer to /settings. |
| apps/lfx-one/src/app/modules/profile/developer/profile-developer.component.ts | Removes legacy profile developer component. |
| apps/lfx-one/src/app/modules/profile/developer/profile-developer.component.html | Removes legacy profile developer template. |
Comments suppressed due to low confidence (1)
apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts:410
- navigator.clipboard.writeText() failures (permission denied, insecure context, etc.) are not handled here. A rejected promise will be unhandled and the user won't get any feedback.
navigator.clipboard.writeText(token).then(() => {
this.tokenCopied.set(true);
this.messageService.add({ severity: 'success', summary: 'Copied', detail: 'Token copied to clipboard' });
setTimeout(() => this.tokenCopied.set(false), 2000);
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts (1)
403-410: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle clipboard write failures in
copyToken
navigator.clipboard.writeText()can fail when clipboard access is unavailable or denied, and this path currently drops that error without any user feedback. Switch this toasync/awaitwithtry/catch(or add a.catch()handler) and show a failure toast.🤖 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 `@apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts` around lines 403 - 410, The copyToken method currently ignores clipboard write failures, so update AccountSettingsComponent.copyToken to handle navigator.clipboard.writeText errors explicitly. Refactor the promise flow in copyToken to async/await or add a catch handler, and on failure use messageService.add to show an error toast instead of only setting tokenCopied on success. Keep the existing success path and tokenCopied reset behavior intact.Source: Coding guidelines
🤖 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 `@apps/lfx-one/src/app/modules/profile/profile.routes.ts`:
- Around line 45-46: The legacy profile route currently uses a bare redirectTo
to /settings, which drops users at the top of settings instead of the developer
section. Update the profile.routes.ts entry for the developer path so it
preserves the deep link target /settings#developer-settings, likely by routing
through a small redirect component or guard instead of static redirectTo. Use
the existing route definition for the developer path as the place to adjust, and
ensure old /profile/developer links land on the anchored developer settings
section.
In
`@apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts`:
- Around line 103-106: The copied state is currently shared through tokenCopied,
so both the v1 and v2 copy buttons show Copied! after either one is clicked.
Update account-settings.component.ts to track copy state separately for each
token/button, and wire the corresponding copy handlers and button label bindings
to the correct signal so only the initiating button changes state. Use the
existing developerToken/developerV1Token and their related copy logic to locate
and split the shared state.
In `@apps/lfx-one/src/app/shared/components/header/header.component.ts`:
- Around line 61-62: The settings shortcut in HeaderComponent is setting a
fragment, but it will not navigate to the developer section unless fragment
scrolling is enabled or the settings page explicitly handles the route fragment.
Update the navigation behavior in header.component.ts so the `/settings` link
with `fragment: 'developer-settings'` actually scrolls to that anchor, either by
enabling anchor scrolling in the router configuration or by handling
`route.fragment` in the settings page component.
---
Outside diff comments:
In
`@apps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.ts`:
- Around line 403-410: The copyToken method currently ignores clipboard write
failures, so update AccountSettingsComponent.copyToken to handle
navigator.clipboard.writeText errors explicitly. Refactor the promise flow in
copyToken to async/await or add a catch handler, and on failure use
messageService.add to show an error toast instead of only setting tokenCopied on
success. Keep the existing success path and tokenCopied reset behavior intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 83406d7d-06a8-4db0-8b86-4786e04d4625
📒 Files selected for processing (11)
apps/lfx-one/src/app/modules/profile/developer/profile-developer.component.htmlapps/lfx-one/src/app/modules/profile/developer/profile-developer.component.tsapps/lfx-one/src/app/modules/profile/profile.routes.tsapps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.htmlapps/lfx-one/src/app/modules/settings/account-settings/account-settings.component.tsapps/lfx-one/src/app/shared/components/header/header.component.tsapps/lfx-one/src/app/shared/components/token-reveal-dialog/token-reveal-dialog.component.htmlapps/lfx-one/src/app/shared/components/token-reveal-dialog/token-reveal-dialog.component.tsapps/lfx-one/src/app/shared/services/user.service.tsapps/lfx-one/src/server/controllers/profile.controller.tspackages/shared/src/interfaces/profile.interface.ts
💤 Files with no reviewable changes (2)
- apps/lfx-one/src/app/modules/profile/developer/profile-developer.component.ts
- apps/lfx-one/src/app/modules/profile/developer/profile-developer.component.html
Address review comments from copilot[bot], coderabbitai: - account-settings: scope "Copied!" state per token so copying one button no longer flips both (per coderabbitai, copilot[bot]) - account-settings: handle navigator.clipboard.writeText failure with an error toast (per coderabbitai, copilot[bot]) - account-settings: import DynamicDialogModule alongside DialogService (per copilot[bot]) - token-reveal-dialog: correct JSDoc to describe the wrapped, scrollable box (per copilot[bot]) - app.config: enable router anchorScrolling so #developer-settings scrolls (per coderabbitai) - profile.routes: remove the legacy /profile/developer route; /settings#developer-settings is the single canonical location (per coderabbitai) Resolves 6 review threads. Signed-off-by: Fayaz G <5818912+fayazg@users.noreply.github.com>
Contributor
Author
Review Feedback AddressedCommit: be835c0 Changes Made
Design Decision
Threads Resolved6 of 6 unresolved threads addressed in this iteration. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
apps/lfx-one/src/app/modules/profile/profile.routes.ts:54
/profile/developeris removed, but there is no backward-compat redirect in the route table. This contradicts the PR description (“route now redirects to /settings”) and will make existing deep links/bookmarks 404.
Add a redirect entry for the removed route (absolute redirect is fine) so /profile/developer lands on Settings instead of breaking.
// Backward-compat redirects for old URLs
{ path: 'overview', redirectTo: 'attribution' },
{ path: 'edit', redirectTo: 'attribution' },
{ path: 'affiliations', redirectTo: 'attribution' },
{ path: 'work-experience', redirectTo: 'attribution' },
{ path: 'identity-services', redirectTo: 'identities' },
{ path: 'badges', redirectTo: 'attribution' },
{ path: 'certificates', redirectTo: 'attribution' },
{ path: 'visibility', redirectTo: 'attribution' },
{ path: 'manage', redirectTo: 'attribution' },
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.
Summary
api-gw.platform.linuxfoundation.org) alongside the existing v2 OIDC session token in the account settings developer section (/settings), so users migrating off the retiring Individual Dashboard can keep calling v1 APIs. The v1 token is already minted by the auth middleware via refresh-token exchange (req.apiGatewayToken); the developer endpoint now returns it as an optionalv1Token, and the row is hidden when unavailable.DialogService.open()dynamic component) that shows the full token in a wrapped, scrollable box (no horizontal scrolling) with Copy and Close; the card keeps only the masked preview./profile/developerpage (it duplicated this UI): the route now redirects to/settings, and the header "Developer Settings" menu item points to/settings#developer-settings.Changes
packages/shared: newDeveloperTokenInfointerface (token,type, optionalv1Token).getDeveloperTokenInforeturnsv1Tokenwhen present; logshas_v1_token(never the value); existing no-store cache headers retained.getDeveloperTokenInfo()typed toDeveloperTokenInfo; account-settings developer section renders v2 + v1 rows; new sharedTokenRevealDialogComponentfor the popup; oldprofile-developercomponent removed.Notes / trade-offs
DialogService(display-only, no result to handle) — consistent with existing usages likedocuments-table..spec.tsforTokenRevealDialogComponent: this repo has no component unit-test runner (testing is Playwright E2E); sibling dialog components follow the same pattern.Part of the myprofile (Individual Dashboard) cutover. JIRA: LFXV2-2542