use refresh token to obtain fresh access token#3094
use refresh token to obtain fresh access token#3094openshift-merge-bot[bot] merged 1 commit intoopenshift-assisted:masterfrom
Conversation
openshift-ci
bot
added
approved
WalkthroughThis change refactors authentication token handling for the chatbot UI by switching from static OCM tokens to refresh tokens. It updates environment variables, configuration files, and proxy settings to support dynamic token refresh via a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (App.tsx)
participant ChatBot Component
participant Token Endpoint (/token)
participant SSO Server
User->>UI (App.tsx): Interacts with UI
UI (App.tsx)->>ChatBot Component: Renders if refreshToken exists
ChatBot Component->>ChatBot Component: onApiCall triggered
ChatBot Component->>ChatBot Component: Check token expiry
alt Token expired or near expiry
ChatBot Component->>Token Endpoint (/token): POST refreshToken
Token Endpoint->>SSO Server: Forward refresh request
SSO Server-->>Token Endpoint: Return new access token
Token Endpoint-->>ChatBot Component: Return new access token
ChatBot Component->>ChatBot Component: Update token and expiry
end
ChatBot Component->>API: Make call with fresh access token
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
apps/assisted-ui/src/main.tsx (1)
7-7: Type-safety gap: make the token non-optional during developmentLeaving
OCM_REFRESH_TOKENoptional means you’ll getstring | undefinedthroughout the codebase and may forget to handle theundefinedcase.
If the app truly requires the token to function (outside of unit tests), declare it as required and fail fast when it is missing.- OCM_REFRESH_TOKEN?: string; + OCM_REFRESH_TOKEN: string;apps/assisted-ui/README.md (1)
99-100: markdown-lint: prefix commands without showing outputWrap command snippets with a language fence to silence MD014 and improve readability.
-$ AIUI_CHAT_API_URL=<chatbot_url> OCM_REFRESH_TOKEN=$(ocm token --refresh) AIUI_SSO_API_URL=https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token yarn start:assisted_ui +```bash +AIUI_CHAT_API_URL=<chatbot_url> \ +OCM_REFRESH_TOKEN=$(ocm token --refresh) \ +AIUI_SSO_API_URL=https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token \ +yarn start:assisted_ui +```apps/assisted-ui/deploy/nginx.conf (1)
31-42: Consider adding the Host header for consistencyThe
/tokenlocation block is missing theproxy_set_header Host $host;directive that's present in the other proxy blocks. While this might work without it, adding it would ensure consistent behavior across all proxied requests.location /token { proxy_pass $AIUI_SSO_API_URL; proxy_http_version 1.1; + proxy_set_header Host $host; proxy_cache_bypass $http_upgrade; proxy_connect_timeout 120; proxy_send_timeout 120; proxy_read_timeout 120; send_timeout 120; client_max_body_size 2M; proxy_ssl_server_name on; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/assisted-ui/README.md(1 hunks)apps/assisted-ui/deploy/nginx.conf(1 hunks)apps/assisted-ui/deploy/start.sh(1 hunks)apps/assisted-ui/env.template.js(1 hunks)apps/assisted-ui/public/env.js(1 hunks)apps/assisted-ui/src/components/App.tsx(2 hunks)apps/assisted-ui/src/components/Chatbot.tsx(1 hunks)apps/assisted-ui/src/main.tsx(1 hunks)apps/assisted-ui/vite.config.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the assisted-installer-ui chatbot feedback implementation, the onfeedbacksubmit callback requires...
Learnt from: celdrake
PR: openshift-assisted/assisted-installer-ui#3051
File: libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx:196-222
Timestamp: 2025-07-18T12:35:50.945Z
Learning: In the assisted-installer-ui chatbot feedback implementation, the onFeedbackSubmit callback requires access to the messages array to retrieve both the bot response content and the associated user question for the API call, making it necessary to include messages in the useCallback dependency array rather than passing message content as props to avoid duplicating potentially long message data.
Applied to files:
apps/assisted-ui/src/components/Chatbot.tsxapps/assisted-ui/src/components/App.tsx
🧬 Code Graph Analysis (1)
apps/assisted-ui/src/components/App.tsx (1)
apps/assisted-ui/src/components/Chatbot.tsx (1)
refreshToken(7-8)
🪛 markdownlint-cli2 (0.17.2)
apps/assisted-ui/README.md
99-99: Dollar signs used before commands without showing output
(MD014, commands-show-output)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: tests
🔇 Additional comments (7)
apps/assisted-ui/public/env.js (1)
1-1: No orphanedwindow.OCM_TOKENreferences foundA repository-wide search for
OCM_TOKENreturned no matches, confirming the rename toOCM_REFRESH_TOKENis complete. No further updates are needed.apps/assisted-ui/src/main.tsx (1)
7-7: No leftover OCM_TOKEN typings detectedI searched for ambient Window interface augmentations and any *.d.ts files referencing OCM_TOKEN and found no occurrences. The codebase is clear of obsolete OCM_TOKEN declarations—no further cleanup required.
apps/assisted-ui/src/components/App.tsx (2)
7-7: LGTM!The import change correctly reflects the new token handling approach where
refreshTokenis now a constant instead of a function.
19-19: LGTM!The conditional change correctly uses the refresh token's presence to determine if the ChatBot should be rendered, which is more appropriate than calling a function during render.
apps/assisted-ui/deploy/start.sh (3)
6-6: LGTM!The new environment variable is properly exported with a sensible default value for local development.
9-9: LGTM!The environment variable is correctly added to the nginx.conf substitution.
16-16: LGTM!The environment variable rename correctly reflects the new refresh token approach.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/assisted-ui/src/components/Chatbot.tsx (3)
7-11: Race condition still present with module-level stateThe module-level mutable variables
tokenandexpirationcan still cause race conditions during concurrent token refreshes, as flagged in previous reviews.
14-14: Expiration check logic is still incorrectThe condition
Date.now() - 5000 > expirationis backwards - it should check if the current time plus 5 seconds is greater than expiration to refresh tokens before they expire.
49-58: Missing error handling for token retrievalThe
onApiCallhandler should handle potential errors fromgetOcmToken()to prevent unhandled promise rejections, as noted in previous reviews.
🧹 Nitpick comments (1)
apps/assisted-ui/src/components/Chatbot.tsx (1)
20-20: Remove redundant null coalescingSince line 15-17 already validates that
refreshTokenexists, the|| ''fallback is unnecessary and should be removed for clarity.- params.append('refresh_token', refreshToken || ''); + params.append('refresh_token', refreshToken);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/assisted-ui/README.md(1 hunks)apps/assisted-ui/deploy/nginx.conf(2 hunks)apps/assisted-ui/deploy/start.sh(1 hunks)apps/assisted-ui/env.template.js(1 hunks)apps/assisted-ui/public/env.js(1 hunks)apps/assisted-ui/src/components/App.tsx(2 hunks)apps/assisted-ui/src/components/Chatbot.tsx(1 hunks)apps/assisted-ui/src/main.tsx(1 hunks)apps/assisted-ui/vite.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/assisted-ui/env.template.js
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/assisted-ui/deploy/nginx.conf
- apps/assisted-ui/src/main.tsx
- apps/assisted-ui/deploy/start.sh
- apps/assisted-ui/src/components/App.tsx
- apps/assisted-ui/vite.config.ts
- apps/assisted-ui/public/env.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the assisted-installer-ui chatbot feedback implementation, the onfeedbacksubmit callback requires...
Learnt from: celdrake
PR: openshift-assisted/assisted-installer-ui#3051
File: libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx:196-222
Timestamp: 2025-07-18T12:35:50.945Z
Learning: In the assisted-installer-ui chatbot feedback implementation, the onFeedbackSubmit callback requires access to the messages array to retrieve both the bot response content and the associated user question for the API call, making it necessary to include messages in the useCallback dependency array rather than passing message content as props to avoid duplicating potentially long message data.
Applied to files:
apps/assisted-ui/src/components/Chatbot.tsx
🪛 markdownlint-cli2 (0.17.2)
apps/assisted-ui/README.md
99-99: Dollar signs used before commands without showing output
(MD014, commands-show-output)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: tests
🔇 Additional comments (1)
apps/assisted-ui/README.md (1)
99-99: Documentation accurately reflects the token changesThe updated environment variables correctly document the shift from static OCM tokens to refresh tokens, including the new SSO API URL for token refresh functionality.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ammont82, rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 0f2bf37
into
openshift-assisted:master
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores