sso login change password send link for email #532
Conversation
WalkthroughThis pull request introduces a new SVG icon component, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PasswordForm
participant API
Note over PasswordForm: On form submission, determine authentication method
alt Non-SSO Flow
User->>PasswordForm: Enter current, new, confirm password
PasswordForm->>API: Execute password update mutation with credentials
API-->>PasswordForm: Return success/error response
PasswordForm->>User: Display update result
else SSO Flow
User->>PasswordForm: Initiate password setup
PasswordForm->>API: Execute setPasswordMutation with email
API-->>PasswordForm: Return success response
PasswordForm->>User: Show success message via snackbar
end
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. 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: 2
🔭 Outside diff range comments (1)
app/(dashboard)/components/Icons/BrokenCircleCheckIcon.tsx (1)
31-32: Add missing React import and explicit return type.The component is missing necessary imports and type declarations.
Add the following at the top of the file:
+import React from 'react'; + interface BrokenCircleCheckIconProps extends React.SVGProps<SVGSVGElement> { // ... props interface }
🧹 Nitpick comments (4)
app/(dashboard)/components/Icons/BrokenCircleCheckIcon.tsx (1)
6-11: Consider making dimensions configurable via props.The icon has fixed dimensions which might limit its reusability.
Apply this diff to make dimensions configurable:
+interface BrokenCircleCheckIconProps extends React.SVGProps<SVGSVGElement> { color?: string; disabled?: boolean; + /** Width of the icon. Defaults to 20 */ + width?: number | string; + /** Height of the icon. Defaults to 20 */ + height?: number | string; } const BrokenCircleCheckIcon: React.FC<BrokenCircleCheckIconProps> = ({ color = "#039855", disabled, + width = 20, + height = 20, ...restProps }) => ( <svg - width="20" - height="20" + width={width} + height={height} viewBox="0 0 20 20"app/(dashboard)/settings/components/PasswordForm.tsx (3)
29-29: Ensure fallback logic for missing props.
Although this function component is expecting an 'email' prop, consider providing a fallback or validating the prop in case it's undefined.
89-181: Address the @ts-ignore and potential form usability concerns.
- The "@ts-ignore" on line 98 suggests a TypeScript mismatch. Remove or suppress it only after addressing the underlying type conflict to maintain consistency.
- Consider advanced validation rules or clarifications to handle usage constraints (e.g., “newPassword cannot match old password”).
183-238: Enhance SSO fallback error handling.
While showing a spinner for loading, the SSO form flow lacks explicit error handling for setPasswordMutation. Logging or presenting error messages would improve the user experience should something go wrong.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/(dashboard)/components/Icons/BrokenCircleCheckIcon.tsx(1 hunks)app/(dashboard)/settings/components/PasswordForm.tsx(2 hunks)app/(dashboard)/settings/page.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: package
🔇 Additional comments (7)
app/(dashboard)/components/Icons/BrokenCircleCheckIcon.tsx (2)
13-21: LGTM!The SVG path implementation is well-structured with proper clipping and conditional styling based on the disabled state.
22-26: LGTM!The SVG definitions are properly structured with consistent dimensions.
app/(dashboard)/settings/components/PasswordForm.tsx (4)
20-23: Imports are appropriately placed.
These additional imports for customer user API, MUI components, and the BrokenCircleCheckIcon look correct and facilitate the new password reset and SSO functionality.
25-27: Strongly typed props for better maintainability.
Defining the PasswordFormProps interface clarifies the expected prop structure and improves code readability.
32-34: Double-check localStorage usage for robust SSO checks.
Relying on localStorage to store "loggedInUsingSSO" may lead to stale or inconsistent data if the user’s session state changes. Consider a more authoritative source (e.g., server-side or tokens).
86-88: Consider confirming successful dispatch.
handleSetPassword simply triggers the mutation with no immediate feedback aside from the loading spinner. Ensure it’s sufficient or consider disabling the button until the email is confirmed.app/(dashboard)/settings/page.tsx (1)
112-112: Check for potential undefined email property.
If selectUser is null or doesn’t have an email, this line may throw an error. Consider using optional chaining (e.g., selectUser?.email) or other defensive checks before rendering.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/(public)/(main-image)/signin/components/SigninPage.tsx(1 hunks)app/(public)/idp-auth/page.tsx(1 hunks)src/hooks/useLogout.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: package
🔇 Additional comments (1)
src/hooks/useLogout.js (1)
20-20: 🛠️ Refactor suggestionConsider security implications of using localStorage for auth state.
While the implementation is functionally correct, storing authentication state in localStorage makes it vulnerable to XSS attacks. Consider these alternatives:
- Use HttpOnly cookies for improved security
- Use session storage instead of local storage for temporary auth state
✅ Verification successful
🌐 Web query:
What are the security implications of storing auth state in localStorage vs cookies?💡 Result:
Storing authentication state securely is crucial for web application security. Here's a detailed comparison of security implications when using localStorage vs cookies: ## Security Comparison | Aspect | LocalStorage | Cookies (with proper configuration) | |----------------------|------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------| | **XSS Vulnerability**| Highly vulnerable - accessible via JavaScript | Protected with `httpOnly` flag - inaccessible to JavaScript [1][2][5] | | **CSRF Vulnerability**| Immune - not automatically sent with requests | Vulnerable by default - mitigated via `sameSite` flag and anti-CSRF tokens [1][5] | | **Storage Limits** | ~5MB | 4KB per cookie | | **Automatic Transmission**| Not sent automatically - requires manual handling | Sent with every request - reduces implementation complexity | | **Secure Attributes**| No equivalent security flags | Supports `Secure` (HTTPS-only) and `httpOnly` flags [2][3][6] | ### Key Security Risks **For LocalStorage:** - **XSS Attacks:** Full exposure of tokens if malicious script executes [1][4] - **Persistence Risk:** Data remains after browser closure on public computers - **No Built-in Protections:** Lacks mechanisms like expiration or secure transmission [5] **For Cookies:** - **CSRF Attacks:** Requires mitigation through: - `sameSite=Strict/Lax` attribute [1][5] - Anti-CSRF tokens in requests [2] - **Implementation Complexity:** Proper configuration of multiple flags needed: ```http Set-Cookie: session=abc123; Secure; HttpOnly; SameSite=Lax ``` ## Best Practices 1. **Prefer httpOnly Cookies** for authentication tokens: - Blocks JavaScript access while maintaining automatic credential transmission [2][5][7] - Recommended by OWASP for session management [2][3] 2. **Use Refresh Token Rotation:** - Store short-lived access token in memory - Keep refresh token in httpOnly cookie [2][5] 3. **Defense-in-Depth Measures:** - Implement Content Security Policy (CSP) to reduce XSS risks [2] - Always use HTTPS with `Secure` cookie flag [3][6] - Validate token signatures server-side regardless of storage method [7] While both storage mechanisms have vulnerabilities, properly configured cookies provide better security through their built-in protections against common attack vectors. The automatic credential management and httpOnly/Secure flags make cookies the preferred choice for authentication state storage when implemented with CSRF protections[1][2][5]. Citations: - 1: https://dev.to/leduc1901/why-cookie-is-preferable-compared-to-localstorage-when-it-comes-to-authentication-48mb - 2: https://dev.to/cotter/localstorage-vs-cookies-all-you-need-to-know-about-storing-jwt-tokens-securely-in-the-front-end-15id - 3: https://www.pivotpointsecurity.com/local-storage-versus-cookies-which-to-use-to-securely-store-session-tokens/ - 4: https://stytch.com/blog/localstorage-vs-sessionstorage-vs-cookies/ - 5: https://supertokens.com/blog/cookies-vs-localstorage-for-sessions-everything-you-need-to-know - 6: http://www.cyberchief.ai/2023/05/secure-jwt-token-storage.html - 7: https://angular.love/localstorage-vs-cookies-all-you-need-to-know-about-storing-jwt-tokens-securely-in-the-front-end
Security Improvement Suggestion: Avoid using localStorage for sensitive auth state
The web query confirms the concerns raised in the review comment. While your use of localStorage for removing the SSO flag is functionally correct, storing sensitive authentication information in localStorage exposes it to XSS attacks. As a result, consider the following alternatives:
- Use HttpOnly Cookies: They are inaccessible to JavaScript, reducing the risk of XSS attacks.
- Use Session Storage: This provides temporary storage that reduces persistence risks on shared or public machines.
Implementing these changes would enhance the overall security of the authentication flow.
Summary by CodeRabbit