Conversation
📝 WalkthroughWalkthroughThe PR reworked SP metadata fetching in the SsoConfiguration component by replacing the prior implementation with a version that explicitly maps API response fields ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/organizations/SsoConfiguration.vue (1)
116-147: Consider showing a toast on metadata fetch failure for UX consistency.The
fetchProviders()function showstoast.error(t('sso-error-loading'))on failure, butfetchSpMetadata()silently fails (only logs to console). Users may be confused if the SP metadata section doesn't appear without any feedback.Proposed enhancement
catch (error) { console.error('Error fetching SSO SP metadata:', error) spMetadata.value = null + toast.error(t('sso-error-loading-metadata')) }This would require adding a new translation key
sso-error-loading-metadatatomessages/en.json.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/organizations/SsoConfiguration.vue` around lines 116 - 147, fetchSpMetadata currently only logs errors to console; update it to show a user-facing error toast on both non-OK responses and exceptions by calling toast.error(t('sso-error-loading-metadata')). Inside fetchSpMetadata, after the existing console.error branches (both the if (!response.ok) branch and the catch block that sets spMetadata.value = null), add toast.error(t('sso-error-loading-metadata')) so users see feedback when spMetadata.value is not populated; ensure the new translation key sso-error-loading-metadata is added to messages/en.json.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/organizations/SsoConfiguration.vue`:
- Around line 116-147: fetchSpMetadata currently only logs errors to console;
update it to show a user-facing error toast on both non-OK responses and
exceptions by calling toast.error(t('sso-error-loading-metadata')). Inside
fetchSpMetadata, after the existing console.error branches (both the if
(!response.ok) branch and the catch block that sets spMetadata.value = null),
add toast.error(t('sso-error-loading-metadata')) so users see feedback when
spMetadata.value is not populated; ensure the new translation key
sso-error-loading-metadata is added to messages/en.json.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9dfce61f-7e72-447b-8917-f5085bf2dd35
📒 Files selected for processing (3)
cloudflare_workers/api/index.tssrc/components/organizations/SsoConfiguration.vuesupabase/functions/_backend/private/sso/sp-metadata.ts
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/organizations/SsoConfiguration.vue (1)
131-141: Align response typing with the backend contract.
sp_metadata_urlis treated as optional here, but the backend contract returns it as required. Keeping it optional with a fallback can hide contract regressions.Proposed contract-tightening diff
- const data = await response.json() as { + const data = await response.json() as { acs_url: string entity_id: string nameid_format: string - sp_metadata_url?: string + sp_metadata_url: string } spMetadata.value = { acs_url: data.acs_url, entity_id: data.entity_id, - sp_metadata_url: data.sp_metadata_url ?? data.entity_id, + sp_metadata_url: data.sp_metadata_url, nameid_format: data.nameid_format, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/organizations/SsoConfiguration.vue` around lines 131 - 141, The response typing currently marks sp_metadata_url as optional and uses a fallback (sp_metadata_url ?? entity_id), which hides backend contract regressions; update the response type from response.json() so sp_metadata_url is required (sp_metadata_url: string) and assign spMetadata.value.sp_metadata_url directly from data.sp_metadata_url (remove the fallback), ensuring the component relies on the backend-provided value; adjust any related form/usage that assumed a fallback accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/organizations/SsoConfiguration.vue`:
- Around line 131-141: The response typing currently marks sp_metadata_url as
optional and uses a fallback (sp_metadata_url ?? entity_id), which hides backend
contract regressions; update the response type from response.json() so
sp_metadata_url is required (sp_metadata_url: string) and assign
spMetadata.value.sp_metadata_url directly from data.sp_metadata_url (remove the
fallback), ensuring the component relies on the backend-provided value; adjust
any related form/usage that assumed a fallback accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0591897c-9aa0-4950-b321-2f06486e942d
📒 Files selected for processing (1)
src/components/organizations/SsoConfiguration.vue
1 participant
Summary (AI generated)
GET /private/sso/sp-metadatainstead of building URLs from frontend host config.sp_metadata_urlin the SSO SP metadata backend response./private/sso/sp-metadataroute in the Cloudflare API worker to make the endpoint available onapi.capgo.app.Motivation (AI generated)
The SSO setup screen could display incorrect SAML endpoints when frontend runtime config temporarily resolved to the API domain. This caused admins to configure IdPs with invalid URLs. Fetching metadata from backend source-of-truth prevents host mismatches.
Business Impact (AI generated)
This reduces SSO setup failures for Enterprise customers, lowers support/debug time during onboarding, and improves reliability of identity integration flows.
Test Plan (AI generated)
bunx eslint "src/components/organizations/SsoConfiguration.vue" "supabase/functions/_backend/private/sso/sp-metadata.ts" "cloudflare_workers/api/index.ts"GET https://api.capgo.app/private/sso/sp-metadatareturns 200 (after deploy) and UI still loads metadata block.Generated with AI
Summary by CodeRabbit