feat(auth): Show Last used label on SSO sign-in method#17093
feat(auth): Show Last used label on SSO sign-in method#17093charlesBochet merged 14 commits intotwentyhq:mainfrom
Conversation
…cross sign-in components
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Pull request overview
This PR adds visual indicators ("Last" badges) to authentication buttons to help users identify which SSO method they used previously. The implementation creates a new Recoil atom that persists the last authenticated method to localStorage and displays a blue pill badge on the corresponding sign-in button.
Changes:
- Added
lastAuthenticatedMethodStateRecoil atom with localStorage persistence to track last used auth method - Updated Google, Microsoft, and SSO sign-in components to show "Last" pill badges when they were the last used method
- Modified logout flow in
useAuthto preserve the last authenticated method across sessions
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/twenty-front/src/modules/auth/states/lastAuthenticatedMethodState.ts | Defines new Recoil atom for tracking last authenticated method with localStorage persistence |
| packages/twenty-front/src/modules/auth/sign-in-up/components/internal/SignInUpWithSSO.tsx | Adds "Last" pill badge display and sets auth method on SSO button click |
| packages/twenty-front/src/modules/auth/sign-in-up/components/internal/SignInUpWithMicrosoft.tsx | Adds "Last" pill badge display and sets auth method on Microsoft button click |
| packages/twenty-front/src/modules/auth/sign-in-up/components/internal/SignInUpWithGoogle.tsx | Adds "Last" pill badge display and sets auth method on Google button click |
| packages/twenty-front/src/modules/auth/hooks/useAuth.ts | Preserves last authenticated method during logout by manually saving/restoring from localStorage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Save to localStorage synchronously before redirect to ensure it persists | ||
| localStorage.setItem( | ||
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | ||
| JSON.stringify('sso' as LastAuthenticatedMethod), | ||
| ); |
There was a problem hiding this comment.
The logic for saving the last authenticated method to localStorage is duplicated across all three sign-in components (SSO, Microsoft, and Google). This manual localStorage manipulation bypasses the Recoil atom's effects. Consider using useSetRecoilState to set the lastAuthenticatedMethodState value instead, which would automatically handle persistence through the localStorageEffect and eliminate code duplication.
| const StyledButtonContainer = styled.div` | ||
| position: relative; | ||
| width: 100%; | ||
| `; | ||
|
|
||
| const StyledLastUsedPill = styled(Pill)` | ||
| position: absolute; | ||
| right: -${({ theme }) => theme.spacing(2)}; | ||
| top: -${({ theme }) => theme.spacing(2)}; | ||
| background: ${({ theme }) => theme.color.blue}; | ||
| color: ${({ theme }) => theme.font.color.inverted}; | ||
| `; |
There was a problem hiding this comment.
The styled components StyledButtonContainer and StyledLastUsedPill are duplicated. This is the same issue as in the other sign-in components - these should be extracted into a shared styled component file.
| const handleClick = () => { | ||
| // Save to localStorage synchronously before redirect to ensure it persists | ||
| localStorage.setItem( | ||
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | ||
| JSON.stringify('google' as LastAuthenticatedMethod), | ||
| ); | ||
| signInWithGoogle({ action }); | ||
| }; |
There was a problem hiding this comment.
The localStorage manipulation logic is duplicated. Instead of directly using localStorage.setItem with JSON.stringify, consider using useSetRecoilState to update the lastAuthenticatedMethodState, which would handle persistence automatically through the existing localStorageEffect.
| variant={signInUpStep === SignInUpStep.Init ? undefined : 'secondary'} | ||
| fullWidth | ||
| /> | ||
| {isLastUsed && <StyledLastUsedPill label={t`Last`} />} |
There was a problem hiding this comment.
The pill label displays "Last" but should display "Last used" to match the PR description and be more descriptive.
| {isLastUsed && <StyledLastUsedPill label={t`Last`} />} | |
| {isLastUsed && <StyledLastUsedPill label={t`Last used`} />} |
| variant={signInUpStep === SignInUpStep.Init ? undefined : 'secondary'} | ||
| fullWidth | ||
| /> | ||
| {isLastUsed && <StyledLastUsedPill label={t`Last`} />} |
There was a problem hiding this comment.
The pill label displays "Last" but should display "Last used" to match the PR description and be more descriptive.
| {isLastUsed && <StyledLastUsedPill label={t`Last`} />} | |
| {isLastUsed && <StyledLastUsedPill label={t`Last used`} />} |
| const StyledButtonContainer = styled.div` | ||
| position: relative; | ||
| width: 100%; | ||
| `; | ||
|
|
||
| const StyledLastUsedPill = styled(Pill)` | ||
| position: absolute; | ||
| right: -${({ theme }) => theme.spacing(2)}; | ||
| top: -${({ theme }) => theme.spacing(2)}; | ||
| background: ${({ theme }) => theme.color.blue}; | ||
| color: ${({ theme }) => theme.font.color.inverted}; | ||
| `; |
There was a problem hiding this comment.
The styled components StyledButtonContainer and StyledLastUsedPill are duplicated across SignInUpWithSSO, SignInUpWithMicrosoft, and SignInUpWithGoogle components. Consider extracting these into a shared styled component file to reduce code duplication and ensure consistency.
| const handleClick = () => { | ||
| // Save to localStorage synchronously before redirect to ensure it persists | ||
| localStorage.setItem( | ||
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | ||
| JSON.stringify('microsoft' as LastAuthenticatedMethod), | ||
| ); | ||
| signInWithMicrosoft({ action }); | ||
| }; |
There was a problem hiding this comment.
The localStorage manipulation logic is duplicated. Instead of directly using localStorage.setItem with JSON.stringify, consider using useSetRecoilState to update the lastAuthenticatedMethodState, which would handle persistence automatically through the existing localStorageEffect.
| const StyledButtonContainer = styled.div` | ||
| position: relative; | ||
| width: 100%; | ||
| `; | ||
|
|
||
| const StyledLastUsedPill = styled(Pill)` | ||
| position: absolute; | ||
| right: -${({ theme }) => theme.spacing(2)}; | ||
| top: -${({ theme }) => theme.spacing(2)}; | ||
| background: ${({ theme }) => theme.color.blue}; | ||
| color: ${({ theme }) => theme.font.color.inverted}; | ||
| `; |
There was a problem hiding this comment.
The styled components StyledButtonContainer and StyledLastUsedPill are duplicated. This is the same issue as in the other sign-in components - these should be extracted into a shared styled component file.
| variant={signInUpStep === SignInUpStep.Init ? undefined : 'secondary'} | ||
| fullWidth | ||
| /> | ||
| {isLastUsed && <StyledLastUsedPill label={t`Last`} />} |
There was a problem hiding this comment.
The pill label displays "Last" but according to the PR title and description, it should display "Last used". Consider updating the label to be more descriptive and match the intended design.
| {isLastUsed && <StyledLastUsedPill label={t`Last`} />} | |
| {isLastUsed && <StyledLastUsedPill label={t`Last used`} />} |
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:32091 This environment will automatically shut down when the PR is closed or after 5 hours. |
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-front/src/modules/auth/hooks/useAuth.ts">
<violation number="1" location="packages/twenty-front/src/modules/auth/hooks/useAuth.ts:181">
P2: Sign-out uses unguarded localStorage/sessionStorage calls that can throw (private mode/quota), aborting cleanup before tokens/cookies are cleared.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const lastAuthenticatedMethod = localStorage.getItem( | ||
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | ||
| ); | ||
|
|
||
| sessionStorage.clear(); | ||
| localStorage.clear(); | ||
|
|
||
| // Restore the last authenticated method after clearing | ||
| if (lastAuthenticatedMethod) { | ||
| localStorage.setItem( | ||
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | ||
| lastAuthenticatedMethod, | ||
| ); | ||
| } |
There was a problem hiding this comment.
P2: Sign-out uses unguarded localStorage/sessionStorage calls that can throw (private mode/quota), aborting cleanup before tokens/cookies are cleared.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/auth/hooks/useAuth.ts, line 181:
<comment>Sign-out uses unguarded localStorage/sessionStorage calls that can throw (private mode/quota), aborting cleanup before tokens/cookies are cleared.</comment>
<file context>
@@ -176,8 +177,22 @@ export const useAuth = () => {
goToRecoilSnapshot(initialSnapshot);
+ // Preserve the last authenticated method before clearing localStorage
+ const lastAuthenticatedMethod = localStorage.getItem(
+ LAST_AUTHENTICATED_METHOD_STORAGE_KEY,
+ );
</file context>
| const lastAuthenticatedMethod = localStorage.getItem( | |
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | |
| ); | |
| sessionStorage.clear(); | |
| localStorage.clear(); | |
| // Restore the last authenticated method after clearing | |
| if (lastAuthenticatedMethod) { | |
| localStorage.setItem( | |
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | |
| lastAuthenticatedMethod, | |
| ); | |
| } | |
| let lastAuthenticatedMethod: string | null = null; | |
| try { | |
| lastAuthenticatedMethod = localStorage.getItem( | |
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | |
| ); | |
| } catch { | |
| lastAuthenticatedMethod = null; | |
| } | |
| try { | |
| sessionStorage.clear(); | |
| localStorage.clear(); | |
| } catch { | |
| // ignore storage errors during sign-out | |
| } | |
| if (lastAuthenticatedMethod) { | |
| try { | |
| localStorage.setItem( | |
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | |
| lastAuthenticatedMethod, | |
| ); | |
| } catch { | |
| // ignore failures preserving last auth method | |
| } | |
| } |
✅ Addressed in 6b9e078
| const { redirectToSSOLoginPage } = useSSO(); | ||
|
|
||
| const signInWithSSO = () => { | ||
| // Save to localStorage synchronously before redirect to ensure it persists |
| import { useSignInWithMicrosoft } from '@/auth/sign-in-up/hooks/useSignInWithMicrosoft'; | ||
| import { | ||
| SignInUpStep, | ||
| signInUpStepState, |
charlesBochet
left a comment
There was a problem hiding this comment.
@ManikanthMartha Thanks for opening a PR, could you first make sure the CI is red, it looks like you have added a lot of indentation that shouldn't be there
Also we don't add comments unless they are really needeed
Lastly, the design from your screenshot does not seems to be matching the one from the issue in term of colour/border. Please make sure it's pixel perfect by looking at the Figma. Could you also add a screenshot in light mode?
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-front/src/modules/auth/sign-in-up/hooks/useLastAuthenticatedMethod.ts">
<violation number="1" location="packages/twenty-front/src/modules/auth/sign-in-up/hooks/useLastAuthenticatedMethod.ts:11">
P2: Setter bypasses Recoil state, writing only to localStorage so `lastAuthenticatedMethodState` stays stale until reload</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| export const useLastAuthenticatedMethod = () => { | ||
| const lastAuthenticatedMethod = useRecoilValue(lastAuthenticatedMethodState); | ||
| const setLastAuthenticatedMethod = (method: LastAuthenticatedMethod) => { |
There was a problem hiding this comment.
P2: Setter bypasses Recoil state, writing only to localStorage so lastAuthenticatedMethodState stays stale until reload
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/auth/sign-in-up/hooks/useLastAuthenticatedMethod.ts, line 11:
<comment>Setter bypasses Recoil state, writing only to localStorage so `lastAuthenticatedMethodState` stays stale until reload</comment>
<file context>
@@ -0,0 +1,22 @@
+
+export const useLastAuthenticatedMethod = () => {
+ const lastAuthenticatedMethod = useRecoilValue(lastAuthenticatedMethodState);
+ const setLastAuthenticatedMethod = (method: LastAuthenticatedMethod) => {
+ localStorage.setItem(
+ LAST_AUTHENTICATED_METHOD_STORAGE_KEY,
</file context>
✅ Addressed in 7d62fbd
|
Could you please add screen recordings too? :) |
|
yea here is the screen recording. Untitled.video.-.Made.with.Clipchamp.mp4 |
| import { atom } from 'recoil'; | ||
| import { localStorageEffect } from '~/utils/recoil/localStorageEffect'; | ||
|
|
||
| export type LastAuthenticatedMethod = 'google' | 'microsoft' | 'sso' | null; |
There was a problem hiding this comment.
One export max per file.
All caps enum AuthenticatedMethod = {
GOOGLE = 'GOOGLE'
...
}
| export const useLastAuthenticatedMethod = () => { | ||
| const lastAuthenticatedMethod = useRecoilValue(lastAuthenticatedMethodState); | ||
| const setLastAuthenticatedMethod = (method: LastAuthenticatedMethod) => { | ||
| localStorage.setItem( |
There was a problem hiding this comment.
The effect takes care of writing to local storage directly via the atom, you shouldn't access local storage directly
| variant={signInUpStep === SignInUpStep.Init ? undefined : 'secondary'} | ||
| fullWidth | ||
| /> | ||
| {isLastUsed && <StyledLastUsedPill label={t`Last`} />} |
There was a problem hiding this comment.
If only a single method is enabled on that workspace then we probably shouldn't show last used
| setSignInUpStep(SignInUpStep.SSOIdentityProviderSelection); | ||
| }; | ||
|
|
||
| const isLastUsed = lastAuthenticatedMethod === 'sso'; |
There was a problem hiding this comment.
Never hardcode strings like that, instead reference the enum (e.g. AuthenticationMethod.SSO ...)
| const { signInWithMicrosoft } = useSignInWithMicrosoft(); | ||
|
|
||
| const handleClick = () => { | ||
| setLastAuthenticatedMethod('microsoft'); |
There was a problem hiding this comment.
use ennum don't hardcode the string (same for other places)
| lastAuthenticatedMethod = localStorage.getItem( | ||
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | ||
| ); | ||
| } catch { |
There was a problem hiding this comment.
No need for try/catch like this
| extraLight: GRAY_SCALE_DARK.gray7, | ||
| inverted: GRAY_SCALE_DARK.gray1, | ||
| danger: COLOR_DARK.red, | ||
| indigo: COLOR_DARK.blue9, |
There was a problem hiding this comment.
We already use these colors, try maybe theme.color.blue? etc ; I don't think we need to introduce these
There was a problem hiding this comment.
I had to introduce indigo as blue was not working for fonts
There was a problem hiding this comment.
My bad, we can directly give the font color as color: ${({ theme }) => theme.color.blue}; , no need to introduce indigo
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-front/src/modules/auth/hooks/useAuth.ts">
<violation number="1" location="packages/twenty-front/src/modules/auth/hooks/useAuth.ts:181">
P1: Sign-out now unguardedly uses localStorage/sessionStorage; DOMException will abort sign-out flow leaving auth state uncleared</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const lastAuthenticatedMethod = localStorage.getItem( | ||
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | ||
| ); | ||
|
|
||
| sessionStorage.clear(); | ||
| localStorage.clear(); | ||
|
|
||
| if (lastAuthenticatedMethod !== null) { | ||
| localStorage.setItem( | ||
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | ||
| lastAuthenticatedMethod, | ||
| ); | ||
| } |
There was a problem hiding this comment.
P1: Sign-out now unguardedly uses localStorage/sessionStorage; DOMException will abort sign-out flow leaving auth state uncleared
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/auth/hooks/useAuth.ts, line 181:
<comment>Sign-out now unguardedly uses localStorage/sessionStorage; DOMException will abort sign-out flow leaving auth state uncleared</comment>
<file context>
@@ -177,31 +178,18 @@ export const useAuth = () => {
- } catch {
- // Ignore storage errors - last auth method is non-critical
- }
+ const lastAuthenticatedMethod = localStorage.getItem(
+ LAST_AUTHENTICATED_METHOD_STORAGE_KEY,
+ );
</file context>
| const lastAuthenticatedMethod = localStorage.getItem( | |
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | |
| ); | |
| sessionStorage.clear(); | |
| localStorage.clear(); | |
| if (lastAuthenticatedMethod !== null) { | |
| localStorage.setItem( | |
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | |
| lastAuthenticatedMethod, | |
| ); | |
| } | |
| let lastAuthenticatedMethod: string | null = null; | |
| try { | |
| lastAuthenticatedMethod = localStorage.getItem( | |
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | |
| ); | |
| } catch { | |
| lastAuthenticatedMethod = null; | |
| } | |
| try { | |
| sessionStorage.clear(); | |
| localStorage.clear(); | |
| } catch { | |
| // Ignore storage errors during sign-out | |
| } | |
| if (lastAuthenticatedMethod !== null) { | |
| try { | |
| localStorage.setItem( | |
| LAST_AUTHENTICATED_METHOD_STORAGE_KEY, | |
| lastAuthenticatedMethod, | |
| ); | |
| } catch { | |
| // Ignore failures preserving last auth method - it's non-critical | |
| } | |
| } |
✅ Addressed in aae2da5
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
packages/twenty-front/src/modules/auth/sign-in-up/hooks/useHasMultipleAuthMethods.ts
Show resolved
Hide resolved
ff6116e to
70d5527
Compare
| signInWithGoogle({ action }); | ||
| }; | ||
|
|
||
| const isLastUsed = lastAuthenticatedMethod === AuthenticatedMethod.GOOGLE; |
There was a problem hiding this comment.
Inconsistent step check for Last Used pill display
Medium Severity
The isLastUsed calculation in SignInUpWithGoogle, SignInUpWithMicrosoft, and SignInUpWithSSO doesn't check signInUpStep === SignInUpStep.Init, while SignInUpWithCredentials does include this check. This causes inconsistent behavior: when a user progresses to the Email or Password step, the Google/Microsoft/SSO buttons will still display the "Last" pill, but the credentials button won't. All authentication method buttons visible in the same view behave differently regarding when they show the indicator.
Additional Locations (2)
|
|
||
| const token = readCaptchaToken(); | ||
| try { | ||
| setLastAuthenticatedMethod(AuthenticatedMethod.EMAIL); |
There was a problem hiding this comment.
Last authenticated method set before success confirmation
Medium Severity
The setLastAuthenticatedMethod(AuthenticatedMethod.EMAIL) call in submitCredentials is placed before the authentication calls, inside the try block. If the sign-in fails (e.g., wrong password), the catch block shows an error but doesn't revert the state. This results in the "Last" indicator incorrectly showing email as the last used method even when the authentication attempt failed.
|
Hey @charlesBochet! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
|
Thanks @ManikanthMartha for your contribution! |

Fixes #17006
Changes
Added
lastAuthenticateWorkspaceSsoMethodStateRecoil atom to track the last used SSO method, persisted in localStorageUpdated
SignInUpWithGooglecomponent to display a "Last" pill badge when Google was the last auth methodUpdated
SignInUpWithMicrosoftcomponent to display a "Last" pill badge when Microsoft was the last auth methodUpdated
SignInUpWithSSOcomponent to display a "Last" pill badge when SSO was the last auth method.The state is saved after successful authentication redirect, ensuring it persists across sessions
Implementation Details
Uses existing
localStorageEffectfor persistence across browser sessionsLeverages the existing Pill component from twenty-ui with blue accent color
The label is positioned on the right border of the button using absolute positioning
State is saved in the
useAuthhook during Google/Microsoft sign-in and in the SSO login componentNote
Highlights the most recently used authentication method and preserves it across sessions.
AuthenticatedMethodenum andlastAuthenticatedMethodState(persisted vialocalStorageEffect) and preserves it throughuseAuth.clearSessionLastUsedPillonSignInUpWithGoogle,SignInUpWithMicrosoft,SignInUpWithSSO, andSignInUpWithCredentialswhen appropriate; addsStyledSSOButtonContainerfor badge positioninguseHasMultipleAuthMethodsto detect when to show the badge; threadsisGlobalScopeto relevant componentsuseSignInUp, SSO button handlers)SignInUpGlobalScopeFormto useSignInUpWithCredentialsand updatesSignInUptitle logic for global scope (e.g., "Welcome to Twenty")Written by Cursor Bugbot for commit 70d5527. This will update automatically on new commits. Configure here.