Frontend/auth updates#1727
Conversation
📝 WalkthroughWalkthroughOAuth authentication is reorganized around a shared utility module with CSRF state and PKCE support. Cognito clients and frontend routes now use application-specific callback paths, with dedicated callback pages, shared token-exchange handling, updated tests, and refreshed Postman collections. ChangesOAuth callback flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant PublicDashboard
participant Cognito
participant AuthCallback
PublicDashboard->>Browser: Generate CSRF state and PKCE challenge
Browser->>Cognito: Request hosted login with state and code challenge
Cognito-->>AuthCallback: Redirect with authorization code and state
AuthCallback->>Cognito: Exchange code with PKCE verifier
Cognito-->>AuthCallback: Return tokens
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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. 🔧 Stylelint (17.14.0)webroot/src/styles.common/_mixins.lessConfigurationError: Could not find "stylelint-config-rational-order". Do you need to install the package or use the "configBasedir" option? webroot/src/styles.common/mixins/auth-error.lessConfigurationError: Could not find "stylelint-config-rational-order". Do you need to install the package or use the "configBasedir" option? webroot/src/pages/AuthCallback/StaffSocialWork/StaffSocialWork.lessConfigurationError: Could not find "stylelint-config-rational-order". Do you need to install the package or use the "configBasedir" option?
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
webroot/src/pages/AuthCallback/StaffCosmo/StaffCosmo.vue (1)
8-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting shared error markup.
This template is duplicated verbatim across
StaffCosmo.vue,StaffJcc.vue, and (per the stack outline)StaffSocialWork.vue/LicenseeJcc.vue. Each new page only needs to render this same error card, so extracting a sharedAuthCallbackErrorcomponent (or a slot in a common wrapper) would reduce duplication and centralize future error-copy changes.🤖 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 `@webroot/src/pages/AuthCallback/StaffCosmo/StaffCosmo.vue` around lines 8 - 20, Extract the duplicated error card markup from StaffCosmo.vue, StaffJcc.vue, StaffSocialWork.vue, and LicenseeJcc.vue into a shared AuthCallbackError component or common wrapper. Preserve the existing isError conditional, translations, styling classes, and DashboardPublic router link, then replace each page’s inline markup with the shared component.backend/social-work-app/docs/internal/postman/postman-collection.json (1)
164-167: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExercise the PKCE and CSRF flow in these requests.
The authorize requests omit
state,code_challenge, andcode_challenge_method; the token request omitscode_verifier. Update the collection’s pre-request setup and exchange request so Postman validates the security contract introduced here.Also applies to: 240-247, 312-319
🤖 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/social-work-app/docs/internal/postman/postman-collection.json` around lines 164 - 167, Update the Postman authorization-flow requests and shared pre-request setup to generate and send a CSRF state and PKCE code challenge with code_challenge_method, then persist the matching code_verifier for the flow. Add the stored state, challenge parameters, and verifier to the corresponding token exchange requests, including the sections identified by the repeated authorize/token request blocks.webroot/src/pages/AuthCallback/_mixins/handler.mixin.ts (3)
138-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove commented-out
toNativeexport.The commented-out
// export default toNative(MixinAuthCallbackHandler);is dead code. The direct class export on line 140 is correct for use withmixins()in subclasses likeStaffCosmo.ts.🧹 Proposed cleanup
- -// export default toNative(MixinAuthCallbackHandler); - export default MixinAuthCallbackHandler;🤖 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 `@webroot/src/pages/AuthCallback/_mixins/handler.mixin.ts` at line 138, Remove the commented-out toNative export near MixinAuthCallbackHandler, keeping the active direct class export unchanged for mixins() consumers.
84-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd type annotations for
cognitoAuthDomainandcognitoClientIdparameters.Both
getTokensandfetchCognitoTokensleavecognitoAuthDomainandcognitoClientIduntyped, defaulting toany. Addingstringannotations would improve type safety and consistency with the typedauthTypeparameter.🔧 Proposed fix
- async getTokens(appMode: AppModes, authType: AuthTypes, cognitoAuthDomain, cognitoClientId): Promise<void> { + async getTokens(appMode: AppModes, authType: AuthTypes, cognitoAuthDomain: string, cognitoClientId: string): Promise<void> {- async fetchCognitoTokens(authType: AuthTypes, cognitoAuthDomain, cognitoClientId): Promise<void> { + async fetchCognitoTokens(authType: AuthTypes, cognitoAuthDomain: string, cognitoClientId: string): Promise<void> {Also applies to: 99-99
🤖 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 `@webroot/src/pages/AuthCallback/_mixins/handler.mixin.ts` at line 84, Add explicit string type annotations to the cognitoAuthDomain and cognitoClientId parameters in both getTokens and fetchCognitoTokens, preserving the existing appMode and authType types.
99-117: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout and error logging to the Cognito token exchange.
The
axios.post()call has no explicit timeout — if Cognito is slow or unresponsive, the user is stuck on a blank callback page until the browser's default timeout fires. Additionally, the.catch()on line 88 silently swallows the error with no logging, making production debugging difficult.⏱️ Proposed fix: add timeout and error logging
params.append('code_verifier', consumePkceCodeVerifier() || ''); - const { data } = await axios.post(`${cognitoAuthDomain}/oauth2/token`, params); + const { data } = await axios.post(`${cognitoAuthDomain}/oauth2/token`, params, { + timeout: 15000, + });And in
getTokens():- await this.fetchCognitoTokens(authType, cognitoAuthDomain, cognitoClientId).catch(() => { + await this.fetchCognitoTokens(authType, cognitoAuthDomain, cognitoClientId).catch((err) => { + console.error('Auth callback token exchange failed:', err); this.isError = true; });🤖 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 `@webroot/src/pages/AuthCallback/_mixins/handler.mixin.ts` around lines 99 - 117, Update fetchCognitoTokens to pass an explicit timeout option to the axios.post token request, using the project’s standard request timeout if available. In getTokens, replace the silent catch with error logging that includes the caught error and relevant Cognito token-exchange context before preserving the existing failure handling.webroot/src/pages/AuthCallback/StaffSocialWork/StaffSocialWork.spec.ts (1)
12-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSmoke test only covers mounting; consider testing auth callback behavior.
The test verifies the component mounts but doesn't exercise the CSRF state verification, token exchange, or error display paths that are the core purpose of this callback page. The mixin's
created()will setisError = truewhenverifyCsrfState()fails (no stored state in test), which is untested.💡 Suggested additional test cases
it('should display error when CSRF state verification fails', async () => { const wrapper = await mountShallow(StaffSocialWork); await wrapper.vm.$nextTick(); expect(wrapper.findComponent(StaffSocialWork).exists()).to.equal(true); // Assert isError is true and error UI is rendered }); it('should verify CSRF state and exchange tokens on valid callback', async () => { // Set up sessionStorage with matching state, route query with code+state // Assert token exchange was called and redirect occurred });🤖 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 `@webroot/src/pages/AuthCallback/StaffSocialWork/StaffSocialWork.spec.ts` around lines 12 - 19, Expand the StaffSocialWork tests beyond mounting by covering the auth callback behavior in the component’s created lifecycle and mixin. Add a test for failed verifyCsrfState() that waits for $nextTick and asserts isError plus the rendered error UI, and a valid callback test that configures matching sessionStorage state and route code/state, then verifies token exchange and redirect behavior using the relevant mocked methods.
🤖 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 `@webroot/src/pages/AuthCallback/_mixins/mixins.spec.ts`:
- Around line 43-50: Update the “should successfully get tokens” test to stub
the axios.post token exchange with a successful response and spy on the router
navigation method used by getTokens, then assert it is called with the expected
route instead of checking history.state.replaced. Ensure no real request to
http://localhost/oauth2/token can occur.
In `@webroot/src/pages/MfaResetConfirmLicensee/MfaResetConfirmLicensee.ts`:
- Around line 51-61: Handle failures from createPkceChallenge() within
MfaResetConfirmLicensee.created() using try/catch, preserve the existing CSRF
initialization, and surface a clear user-facing error or disable login when PKCE
generation fails so navigation cannot occur with an empty code_challenge.
- Line 92: Update the getHostedLoginUri call in MfaResetConfirmLicensee to pass
AppModes.JCC instead of this.appMode, ensuring the licensee callback path always
targets the supported JCC route.
In `@webroot/src/utils/auth.ts`:
- Around line 133-218: Resolve the LICENSEE callback mismatch in
getHostedLoginUri: either add the missing LicenseeCosmo and LicenseeSocialWork
callback pages and router entries for /auth/callback/licensee/cosmo and
/auth/callback/licensee/socialwork, or restrict callback-path generation so
LICENSEE only produces supported routes such as /auth/callback/licensee/jcc.
---
Nitpick comments:
In `@backend/social-work-app/docs/internal/postman/postman-collection.json`:
- Around line 164-167: Update the Postman authorization-flow requests and shared
pre-request setup to generate and send a CSRF state and PKCE code challenge with
code_challenge_method, then persist the matching code_verifier for the flow. Add
the stored state, challenge parameters, and verifier to the corresponding token
exchange requests, including the sections identified by the repeated
authorize/token request blocks.
In `@webroot/src/pages/AuthCallback/_mixins/handler.mixin.ts`:
- Line 138: Remove the commented-out toNative export near
MixinAuthCallbackHandler, keeping the active direct class export unchanged for
mixins() consumers.
- Line 84: Add explicit string type annotations to the cognitoAuthDomain and
cognitoClientId parameters in both getTokens and fetchCognitoTokens, preserving
the existing appMode and authType types.
- Around line 99-117: Update fetchCognitoTokens to pass an explicit timeout
option to the axios.post token request, using the project’s standard request
timeout if available. In getTokens, replace the silent catch with error logging
that includes the caught error and relevant Cognito token-exchange context
before preserving the existing failure handling.
In `@webroot/src/pages/AuthCallback/StaffCosmo/StaffCosmo.vue`:
- Around line 8-20: Extract the duplicated error card markup from
StaffCosmo.vue, StaffJcc.vue, StaffSocialWork.vue, and LicenseeJcc.vue into a
shared AuthCallbackError component or common wrapper. Preserve the existing
isError conditional, translations, styling classes, and DashboardPublic router
link, then replace each page’s inline markup with the shared component.
In `@webroot/src/pages/AuthCallback/StaffSocialWork/StaffSocialWork.spec.ts`:
- Around line 12-19: Expand the StaffSocialWork tests beyond mounting by
covering the auth callback behavior in the component’s created lifecycle and
mixin. Add a test for failed verifyCsrfState() that waits for $nextTick and
asserts isError plus the rendered error UI, and a valid callback test that
configures matching sessionStorage state and route code/state, then verifies
token exchange and redirect behavior using the relevant mocked methods.
🪄 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: 85950346-9adc-4ed8-8df5-ef52b99643b1
📒 Files selected for processing (80)
backend/common-cdk/common_constructs/user_pool.pybackend/common-cdk/tests/test_user_pool.pybackend/compact-connect/docs/internal/postman/postman-collection.jsonbackend/compact-connect/docs/postman/postman-collection.jsonbackend/compact-connect/stacks/persistent_stack/staff_users.pybackend/compact-connect/stacks/provider_users/provider_users.pybackend/compact-connect/tests/app/base.pybackend/cosmetology-app/docs/internal/postman/postman-collection.jsonbackend/cosmetology-app/docs/postman/postman-collection.jsonbackend/cosmetology-app/docs/search-internal/postman/postman-collection.jsonbackend/cosmetology-app/stacks/persistent_stack/staff_users.pybackend/cosmetology-app/tests/app/base.pybackend/social-work-app/docs/internal/postman/postman-collection.jsonbackend/social-work-app/docs/postman/postman-collection.jsonbackend/social-work-app/docs/search-internal/postman/postman-collection.jsonbackend/social-work-app/stacks/persistent_stack/staff_users.pybackend/social-work-app/tests/app/base.pywebroot/src/app.config.tswebroot/src/components/App/App.spec.tswebroot/src/components/App/App.tswebroot/src/components/AutoLogout/AutoLogout.tswebroot/src/components/ChangePassword/ChangePassword.tswebroot/src/components/MilitaryAffiliationInfoBlock/MilitaryAffiliationInfoBlock.tswebroot/src/components/Page/PageMainNav/PageMainNav.tswebroot/src/components/StateSettingsList/StateSettingsList.tswebroot/src/components/UserAccount/UserAccount.tswebroot/src/models/LicenseeUser/LicenseeUser.model.spec.tswebroot/src/models/LicenseeUser/LicenseeUser.model.tswebroot/src/models/StaffUser/StaffUser.model.spec.tswebroot/src/models/StaffUser/StaffUser.model.tswebroot/src/models/User/User.model.tswebroot/src/network/licenseApi/data.api.tswebroot/src/network/licenseApi/interceptors.tswebroot/src/network/searchApi/interceptors.tswebroot/src/network/stateApi/interceptors.tswebroot/src/network/userApi/interceptors.tswebroot/src/pages/AuthCallback/AuthCallback.spec.tswebroot/src/pages/AuthCallback/AuthCallback.tswebroot/src/pages/AuthCallback/LicenseeJcc/LicenseeJcc.lesswebroot/src/pages/AuthCallback/LicenseeJcc/LicenseeJcc.spec.tswebroot/src/pages/AuthCallback/LicenseeJcc/LicenseeJcc.tswebroot/src/pages/AuthCallback/LicenseeJcc/LicenseeJcc.vuewebroot/src/pages/AuthCallback/StaffCosmo/StaffCosmo.lesswebroot/src/pages/AuthCallback/StaffCosmo/StaffCosmo.spec.tswebroot/src/pages/AuthCallback/StaffCosmo/StaffCosmo.tswebroot/src/pages/AuthCallback/StaffCosmo/StaffCosmo.vuewebroot/src/pages/AuthCallback/StaffJcc/StaffJcc.lesswebroot/src/pages/AuthCallback/StaffJcc/StaffJcc.spec.tswebroot/src/pages/AuthCallback/StaffJcc/StaffJcc.tswebroot/src/pages/AuthCallback/StaffJcc/StaffJcc.vuewebroot/src/pages/AuthCallback/StaffSocialWork/StaffSocialWork.lesswebroot/src/pages/AuthCallback/StaffSocialWork/StaffSocialWork.spec.tswebroot/src/pages/AuthCallback/StaffSocialWork/StaffSocialWork.tswebroot/src/pages/AuthCallback/StaffSocialWork/StaffSocialWork.vuewebroot/src/pages/AuthCallback/_mixins/handler.mixin.tswebroot/src/pages/AuthCallback/_mixins/mixins.spec.tswebroot/src/pages/CompactSettings/CompactSettings.tswebroot/src/pages/Home/Home.tswebroot/src/pages/Logout/Logout.tswebroot/src/pages/MfaResetConfirmLicensee/MfaResetConfirmLicensee.tswebroot/src/pages/MfaResetStartLicensee/MfaResetStartLicensee.tswebroot/src/pages/PrivilegeDetail/PrivilegeDetail.tswebroot/src/pages/PublicDashboard/PublicDashboard.spec.tswebroot/src/pages/PublicDashboard/PublicDashboard.tswebroot/src/pages/StateSettings/StateSettings.tswebroot/src/router/index.tswebroot/src/router/routes.tswebroot/src/store/global/global.mutations.tswebroot/src/store/global/global.spec.tswebroot/src/store/global/global.state.tswebroot/src/store/user/user.actions.tswebroot/src/store/user/user.mutations.tswebroot/src/store/user/user.spec.tswebroot/src/store/user/user.state.tswebroot/src/styles.common/_mixins.lesswebroot/src/styles.common/mixins/auth-error.lesswebroot/src/utils/auth.tswebroot/tests/helpers/setup.tswebroot/tsconfig.jsonwebroot/vue.config.js
💤 Files with no reviewable changes (2)
- webroot/src/pages/AuthCallback/AuthCallback.spec.ts
- webroot/src/pages/AuthCallback/AuthCallback.ts
| it('should successfully get tokens', async () => { | ||
| const wrapper = await mountShallow(AuthCallbackHandlerMixin); | ||
| const component = wrapper.vm; | ||
|
|
||
| await component.getTokens(AppModes.JCC, AuthTypes.STAFF, 'http://localhost', 'abc'); | ||
|
|
||
| expect(component.$router.options.history.state.replaced).to.equal(true); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if axios is mocked in the test setup files
rg -n --type=ts 'axios|sinon.*stub|mock.*axios' webroot/tests/helpers/ webroot/src/pages/AuthCallback/_mixins/mixins.spec.ts -g '!**/node_modules/**'Repository: csg-org/CompactConnect
Length of output: 251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## File list\n'
git ls-files 'webroot/src/pages/AuthCallback/_mixins/*' 'webroot/tests/helpers/*' | sed 's#^`#-` #'
printf '\n## Spec outline\n'
ast-grep outline webroot/src/pages/AuthCallback/_mixins/mixins.spec.ts --view expanded || true
printf '\n## Mixin outline\n'
ast-grep outline webroot/src/pages/AuthCallback/_mixins/mixins.ts --view expanded || true
printf '\n## Search for relevant symbols\n'
rg -n "getTokens|redirectUser|created\\(|axios\\.post|oauth2/token|pkce|code_verifier|isError|replaced" webroot/src/pages/AuthCallback/_mixins webroot/tests/helpers -g '!**/node_modules/**'Repository: csg-org/CompactConnect
Length of output: 2213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## handler.mixin.ts (selected lines)\n'
sed -n '1,220p' webroot/src/pages/AuthCallback/_mixins/handler.mixin.ts | cat -n
printf '\n## mixins.spec.ts (selected lines)\n'
sed -n '1,220p' webroot/src/pages/AuthCallback/_mixins/mixins.spec.ts | cat -n
printf '\n## test helper files\n'
sed -n '1,220p' webroot/tests/helpers/setup.ts | cat -n
printf '\n--- index.ts ---\n'
sed -n '1,220p' webroot/tests/helpers/index.ts | cat -n
printf '\n## broader axios/search hits\n'
rg -n "axios\\.post|stub\\(.*axios|mock.*axios|sinon\\.stub\\(.*post|createSandbox|mountShallow|router\\.options\\.history\\.state\\.replaced" webroot/src webroot/tests -g '!**/node_modules/**'Repository: csg-org/CompactConnect
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## router/history-related setup\n'
rg -n "createWebHistory|history.state.replaced|router\\.replace|router\\.push|isReady\\(|replaceState|replaced" webroot/tests/helpers webroot/src/pages/AuthCallback webroot/src/router webroot/src -g '!**/node_modules/**' | head -n 200
printf '\n## any axios mocking/interception in tests\n'
rg -n "axios|nock|msw|fetchMock|MockAdapter|axios-mock|stub.*post|mock.*post" webroot/tests webroot/src -g '!**/node_modules/**' | head -n 200Repository: csg-org/CompactConnect
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## explicit axios mocks in tests\n'
rg -n "vi\\.mock\\(['\"]axios['\"]|jest\\.mock\\(['\"]axios['\"]|mock.*axios|axios-mock-adapter|MockAdapter|nock|msw|sinon\\.stub\\(axios|stub\\(.*post" webroot/tests webroot/src/pages/AuthCallback -g '!**/node_modules/**' | head -n 200
printf '\n## auth callback related specs\n'
rg -n "redirectUser|created\\(|verifyCsrfState|consumePkceCodeVerifier|getTokens\\(" webroot/src/pages/AuthCallback -g '!**/node_modules/**' | head -n 200Repository: csg-org/CompactConnect
Length of output: 194
Mock the token exchange in this test
getTokens() calls axios.post() and the success path navigates with router.push(), so asserting history.state.replaced doesn’t prove the right behavior. Stub the token response and assert the route change directly; otherwise this spec can hit http://localhost/oauth2/token for real.
🤖 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 `@webroot/src/pages/AuthCallback/_mixins/mixins.spec.ts` around lines 43 - 50,
Update the “should successfully get tokens” test to stub the axios.post token
exchange with a successful response and spy on the router navigation method used
by getTokens, then assert it is called with the expected route instead of
checking history.state.replaced. Ensure no real request to
http://localhost/oauth2/token can occur.
| csrfState = ''; | ||
| pkceChallenge = ''; | ||
|
|
||
| // | ||
| // Lifecycle | ||
| // | ||
| async created(): Promise<void> { | ||
| this.csrfState = createAuthCsrfState(); | ||
| this.pkceChallenge = await createPkceChallenge(); | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Unhandled rejection in async created() if createPkceChallenge() fails.
If the Web Crypto API throws (e.g., crypto.subtle.digest unavailable), the rejection is unhandled and this.pkceChallenge remains ''. The user could then click "go to login" and get a URI with an empty code_challenge, causing a confusing token-exchange failure rather than a clear error message.
🛡️ Suggested fix: add try/catch in created()
async created(): Promise<void> {
this.csrfState = createAuthCsrfState();
- this.pkceChallenge = await createPkceChallenge();
+ try {
+ this.pkceChallenge = await createPkceChallenge();
+ } catch {
+ this.serverMessage = this.$t('serverErrors.networkError');
+ this.isLoading = false;
+ }
}📝 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.
| csrfState = ''; | |
| pkceChallenge = ''; | |
| // | |
| // Lifecycle | |
| // | |
| async created(): Promise<void> { | |
| this.csrfState = createAuthCsrfState(); | |
| this.pkceChallenge = await createPkceChallenge(); | |
| } | |
| csrfState = ''; | |
| pkceChallenge = ''; | |
| // | |
| // Lifecycle | |
| // | |
| async created(): Promise<void> { | |
| this.csrfState = createAuthCsrfState(); | |
| try { | |
| this.pkceChallenge = await createPkceChallenge(); | |
| } catch { | |
| this.serverMessage = this.$t('serverErrors.networkError'); | |
| this.isLoading = false; | |
| } | |
| } | |
🤖 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 `@webroot/src/pages/MfaResetConfirmLicensee/MfaResetConfirmLicensee.ts` around
lines 51 - 61, Handle failures from createPkceChallenge() within
MfaResetConfirmLicensee.created() using try/catch, preserve the existing CSRF
initialization, and surface a clear user-facing error or disable login when PKCE
generation fails so navigation cannot occur with an empty code_challenge.
|
|
||
| get hostedLoginUriLicensee(): string { | ||
| return getHostedLoginUri(this.appMode, AuthTypes.LICENSEE, '/login'); | ||
| return getHostedLoginUri(this.appMode, AuthTypes.LICENSEE, '/login', this.csrfState, this.pkceChallenge); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check routes.ts for licensee callback route definitions
rg -n 'auth/callback/licensee' webroot/src/router/routes.tsRepository: csg-org/CompactConnect
Length of output: 208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the URI builder and the related dashboard call site
rg -n "function getHostedLoginUri|getHostedLoginUri\(" webroot/src
rg -n "PublicDashboard" webroot/src
# Inspect the relevant files and nearby context
sed -n '1,220p' webroot/src/utils/*.ts 2>/dev/null || true
sed -n '1,220p' webroot/src/pages/PublicDashboard/PublicDashboard.ts 2>/dev/null || true
# Search for any non-JCC licensee callback routes/pages
rg -n "auth/callback/licensee|LicenseeJcc|licensee/(cosmo|socialwork|jcc)" webroot/srcRepository: csg-org/CompactConnect
Length of output: 16937
this.appMode should not drive the licensee callback path. getHostedLoginUri() builds /auth/callback/licensee/{mode}, but only /auth/callback/licensee/jcc exists here, and PublicDashboard.ts already pins licensee login to AppModes.JCC. Use AppModes.JCC here too.
🤖 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 `@webroot/src/pages/MfaResetConfirmLicensee/MfaResetConfirmLicensee.ts` at line
92, Update the getHostedLoginUri call in MfaResetConfirmLicensee to pass
AppModes.JCC instead of this.appMode, ensuring the licensee callback path always
targets the supported JCC route.
| export const getCognitoConfig = (appMode: AppModes, authType: AuthTypes): CognitoConfig => { | ||
| const config: CognitoConfig = { | ||
| scopes: '', | ||
| clientId: '', | ||
| authDomain: '', | ||
| }; | ||
|
|
||
| switch (authType) { | ||
| case AuthTypes.STAFF: | ||
| config.scopes = staffLoginScopes; | ||
|
|
||
| if (appMode === AppModes.JCC) { | ||
| config.clientId = envConfig.cognitoClientIdStaff; | ||
| config.authDomain = envConfig.cognitoAuthDomainStaff; | ||
| } else if (appMode === AppModes.COSMETOLOGY) { | ||
| config.clientId = envConfig.cognitoClientIdStaffCosmo; | ||
| config.authDomain = envConfig.cognitoAuthDomainStaffCosmo; | ||
| } else if (appMode === AppModes.SOCIAL_WORK) { | ||
| config.clientId = envConfig.cognitoClientIdStaffSw; | ||
| config.authDomain = envConfig.cognitoAuthDomainStaffSw; | ||
| } | ||
|
|
||
| break; | ||
| case AuthTypes.LICENSEE: | ||
| config.scopes = licenseeLoginScopes; | ||
| config.clientId = envConfig.cognitoClientIdLicensee; | ||
| config.authDomain = envConfig.cognitoAuthDomainLicensee; | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
| return config; | ||
| }; | ||
|
|
||
| export const getHostedLoginUri = (appMode: AppModes, authType: AuthTypes, hostedIdpPath = '/login', state = '', codeChallenge = ''): string => { | ||
| const { domain } = envConfig; | ||
| const { | ||
| scopes, | ||
| clientId, | ||
| authDomain | ||
| } = getCognitoConfig(appMode, authType); | ||
| const getCallbackPath = () => { | ||
| let userScopePath = ``; | ||
| let compactScopePath = ``; | ||
|
|
||
| switch (authType) { | ||
| case AuthTypes.STAFF: | ||
| userScopePath += `/staff`; | ||
| break; | ||
| case AuthTypes.LICENSEE: | ||
| userScopePath += `/licensee`; | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
| switch (appMode) { | ||
| case AppModes.JCC: | ||
| compactScopePath += `/jcc`; | ||
| break; | ||
| case AppModes.COSMETOLOGY: | ||
| compactScopePath += `/cosmo`; | ||
| break; | ||
| case AppModes.SOCIAL_WORK: | ||
| compactScopePath += `/socialwork`; | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
| return `/auth/callback${userScopePath}${compactScopePath}`; | ||
| }; | ||
| const loginUriQuery = [ | ||
| `?client_id=${clientId}`, | ||
| `&response_type=code`, | ||
| `&scope=${encodeURIComponent(scopes || '')}`, | ||
| `&state=${encodeURIComponent(state)}`, | ||
| `&code_challenge=${encodeURIComponent(codeChallenge)}`, | ||
| `&code_challenge_method=S256`, | ||
| `&redirect_uri=${encodeURIComponent(`${domain}${getCallbackPath()}`)}`, | ||
| ].join(''); | ||
| const loginUri = `${authDomain}${hostedIdpPath}${loginUriQuery}`; | ||
|
|
||
| return loginUri; | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for all AuthCallback page components and verify route coverage.
# 1. List all callback page component directories
echo "=== AuthCallback page components ==="
fd -t d . webroot/src/pages/AuthCallback --max-depth 1 --exec echo {}
# 2. Check routes file for callback path definitions
echo ""
echo "=== Callback routes in routes.ts ==="
rg -n 'auth/callback' webroot/src/router/routes.ts -C2
# 3. Check for any LicenseeCosmo or LicenseeSocialWork components
echo ""
echo "=== Searching for LicenseeCosmo / LicenseeSocialWork ==="
fd -t f -i 'licensee' webroot/src/pages/AuthCallbackRepository: csg-org/CompactConnect
Length of output: 1669
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== AuthCallback routes ==="
sed -n '55,110p' webroot/src/router/routes.ts
echo
echo "=== AuthCallback page tree ==="
find webroot/src/pages/AuthCallback -maxdepth 2 -type f | sort
echo
echo "=== Search for licensee callback paths ==="
rg -n '/auth/callback/licensee/(cosmo|socialwork|jcc)|LicenseeCosmo|LicenseeSocialWork' webroot/src -C 2Repository: csg-org/CompactConnect
Length of output: 4025
Missing licensee callback routes for non-JCC modes
getHostedLoginUri can generate /auth/callback/licensee/cosmo and /auth/callback/licensee/socialwork, but the router only defines /auth/callback/licensee/jcc and there are no LicenseeCosmo or LicenseeSocialWork callback pages. Add those routes/pages or stop generating those LICENSEE paths.
🤖 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 `@webroot/src/utils/auth.ts` around lines 133 - 218, Resolve the LICENSEE
callback mismatch in getHostedLoginUri: either add the missing LicenseeCosmo and
LicenseeSocialWork callback pages and router entries for
/auth/callback/licensee/cosmo and /auth/callback/licensee/socialwork, or
restrict callback-path generation so LICENSEE only produces supported routes
such as /auth/callback/licensee/jcc.
2 participants
Requirements List
Description List
stateparameter to be a random-value CSRF protectionTesting List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsCloses #1641
Summary by CodeRabbit
New Features
Bug Fixes
Tests