Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(auth, settings): allow to impersonate on mono workspace mode #9451

Closed
wants to merge 2 commits into from

Conversation

AMoreaux
Copy link
Contributor

@AMoreaux AMoreaux commented Jan 8, 2025

Added support for multi-workspace checks during impersonation and streamlined token verification flows. Adjusted session clearing and navigation logic to improve user transition between workspaces.

Added support for multi-workspace checks during impersonation and streamlined token verification flows. Adjusted session clearing and navigation logic to improve user transition between workspaces.
@AMoreaux AMoreaux self-assigned this Jan 8, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Enhanced impersonation functionality to support both single and multi-workspace environments, with improved session management and authentication flows.

  • Added clearSession call before redirecting in single-workspace impersonation in useImpersonate.ts
  • Simplified VerifyEffect component by removing getTokens() wrapper and flattening verification logic
  • Moved setIsVerifyPendingState management into handleVerify in useAuth.ts for better state control
  • Added isMultiWorkspaceEnabled checks in impersonation flow to handle workspace-specific redirections
  • Streamlined token verification by removing unnecessary data returns from auth handler functions

3 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +36 to +39
if (isDefined(loginToken)) {
setIsAppWaitingForFreshObjectMetadata(true);
verify(loginToken);
} else if (!isLogged) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: verify() returns a Promise but is not being awaited - this could cause race conditions if the verification fails

@@ -268,6 +268,8 @@ export const useAuth = () => {

const handleVerify = useCallback(
async (loginToken: string) => {
setIsVerifyPendingState(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: setIsVerifyPendingState(true) should be wrapped in try/catch to ensure it gets set back to false if verification fails

@AMoreaux AMoreaux removed the request for review from charlesBochet January 8, 2025 13:28
@AMoreaux AMoreaux marked this pull request as draft January 8, 2025 13:28
…nation flow

Simplified test assertions in `useAuth.test.tsx` by removing redundant property checks from the signup method. Enhanced `useImpersonate` hook by refactoring workspace condition logic and introducing fresh metadata states to improve session handling.
@AMoreaux
Copy link
Contributor Author

AMoreaux commented Jan 8, 2025

@ehconitin feel free to close it when you cherry-picked the commits

@ehconitin
Copy link
Contributor

Thanks @AMoreaux

@ehconitin ehconitin closed this Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants