-
Notifications
You must be signed in to change notification settings - Fork 3
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
[REF] Switch experimental auth to auth0 #380
Conversation
Just copied some stuff into App.tsx
Parial commit
Reviewer's Guide by SourceryThis PR replaces the existing Google authentication with Auth0 authentication. The implementation removes Google OAuth specific code and integrates Auth0's React SDK to handle authentication flows, user information, and token management. Sequence diagram for Auth0 authentication flowsequenceDiagram
actor User
participant App
participant Auth0
User->>App: Open application
App->>Auth0: Check isAuthenticated
alt isAuthenticated
App->>User: Access granted
else not isAuthenticated
App->>Auth0: loginWithRedirect()
Auth0-->>User: Redirect to Auth0 login
User->>Auth0: Provide credentials
Auth0-->>App: Redirect back with token
App->>User: Access granted
end
Updated class diagram for authentication componentsclassDiagram
class App {
- boolean openAuthDialog
- string IDToken
+ useAuth0()
+ useEffect()
}
class Navbar {
- string latestReleaseTag
- HTMLElement anchorEl
+ useAuth0()
}
class AuthDialog {
+ useAuth0()
}
App --> AuthDialog
App --> Navbar
Navbar --> AuthDialog
note for App "Replaced Google OAuth with Auth0"
note for Navbar "Uses Auth0 for user info and logout"
note for AuthDialog "Uses Auth0 for loginWithRedirect"
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Deploy Preview for neurobagel-query ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @surchs - I've reviewed your changes - here's some feedback:
Overall Comments:
- There are several TODOs in the code that should be addressed before merging, particularly around redirect URI handling, domain configuration, and the Vite dependency exclusion. Please resolve these or document why they need to remain.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@rmanaem possibly yes. The callback URL is currently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧑🍳
🚀 PR was released in |
Changes proposed in this pull request:
Checklist
This section is for the PR reviewer
[ENH]
,[FIX]
,[REF]
,[TST]
,[CI]
,[MNT]
,[INF]
,[MODEL]
,[DOC]
) (see our Contributing Guidelines for more info)skip-release
(to be applied by maintainers only)Closes #XXXX
query-tool-results
files in the neurobagel_examples repo have been regeneratedFor new features:
For bug fixes:
Summary by Sourcery
Enhancements: