fix unable to login if authentication is delegated to another CAS ser… #6865
Conversation
|
|
WalkthroughIntroduces a conditional guard in the authentication webview to verify SAML/CAS redirect URLs originate from the Rocket.Chat server before extracting tokens and building the payload, preventing the webview from closing on external redirects. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant WebView
participant AppHandler as App (Auth Handler)
participant Server as Rocket.Chat Server
User->>WebView: Perform SSO login (SAML/CAS)
WebView->>Server: Navigate through IdP flow
Server-->>WebView: Redirect to callback URL (may contain ticket or saml_idp_credentialToken)
WebView->>AppHandler: onNavigation(url)
alt url startsWith(server) (isRocketChatServer)
AppHandler->>AppHandler: validate URL and extract token (ticket or saml_idp_credentialToken)
alt saml_idp_credentialToken present
AppHandler->>AppHandler: build payload { credentialToken, saml: true }
else CAS token path
AppHandler->>AppHandler: build payload { cas: { credentialToken: ssoToken } }
end
AppHandler->>Server: submit authentication payload
AppHandler->>WebView: close()
else external redirect (not Rocket.Chat server)
AppHandler->>WebView: ignore / keep webview open
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/views/AuthenticationWebView.tsx (1)
112-121: Declare thepayloadvariable before use.The variable
payloadis assigned at lines 115 and 117 but never declared, causing a TypeScript compilation error with strict mode enabled. Addlet payload;before the conditional at line 112.Additionally, while the
isRocketChatServerguard correctly prevents premature webview closure during CAS delegation, ensure the SAML vs CAS payload distinction is tested with both single-factor and delegated authentication flows.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/views/AuthenticationWebView.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/views/AuthenticationWebView.tsx (1)
app/sagas/login.js (4)
server(86-86)server(230-230)server(299-299)server(375-375)
|
Hey |
|
Hello, and thank you for this PR. if (authType === 'saml' || authType === 'cas') {
const parsedUrl = parse(url, true);
// Only close the webview when redirected back to the Rocket.Chat server
// This prevents premature closure when CAS delegates to another CAS server for MFA
const isRocketChatServer = url.startsWith(server);
// ticket -> cas / validate & saml_idp_credentialToken -> saml
if (isRocketChatServer && (parsedUrl.pathname?.includes('validate') || parsedUrl.query?.ticket || parsedUrl.query?.saml_idp_credentialToken)) {
let payload: ICredentials;
if (authType === 'saml') {
const token = parsedUrl.query?.saml_idp_credentialToken || ssoToken;
const credentialToken = { credentialToken: token };
payload = { ...credentialToken, saml: true };
} else {
payload = { cas: { credentialToken: ssoToken } };
}
debouncedLogin(payload);
}
}We hope that this will be quickly incorporated into a future update of the mobile application 🙏 |
|
@vasusadariya can you add @floriannari changes? |
|
hey @diegolmello thanks! i will add these changes that @floriannari suggest me |
|
hey @diegolmello i have added all the changes that @floriannari has suggested so please go through it and if all good then you can merge it! |
4 participants
Proposed changes
Fixed a critical bug in the CAS authentication flow that prevented multi-factor authentication (MFA) from completing. The issue occurred when using cascaded CAS servers where the first CAS server delegates to a second CAS server for MFA.
Previously, the
AuthenticationWebViewwould close prematurely when detecting any CAS ticket in the URL, including intermediate redirects between CAS servers. This fix adds a check to ensure the webview only closes when the authentication flow redirects back to the Rocket.Chat server itself, allowing the complete MFA flow to finish successfully.Changes:
Issue(s)
Fixes the CAS MFA authentication issue where the app closes the authentication webview before completing the second factor validation.
How to test or reproduce
Setup:
Steps to reproduce the bug (before fix):
Expected behavior (after fix):
Affected screens:
AuthenticationWebView.tsx- CAS/SAML authentication flowScreenshots
N/A - Authentication flow behavior (no visual UI changes)
Types of changes
Checklist
Further comments
This is a targeted fix that addresses the root cause without affecting other authentication flows. The change is minimal and adds a single conditional check to verify the redirect URL matches the Rocket.Chat server before triggering the login process.
Technical details:
url.includes(server)before processing CAS/SAML ticketsThis fix is backward compatible and doesn't affect:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.