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

[REF] Switch experimental auth to auth0 #380

Merged
merged 13 commits into from
Dec 2, 2024
Merged

[REF] Switch experimental auth to auth0 #380

merged 13 commits into from
Dec 2, 2024

Conversation

surchs
Copy link
Contributor

@surchs surchs commented Nov 28, 2024

Changes proposed in this pull request:

  • Replace Google ID with Auth0 (we have our own project there now)
  • Exclude the auth0-react library (equivalent of the google library) from vite optimization. For reasons I don't yet fully understand, not doing so results in all component tests breaking with:
[vite] Internal server error: Failed to parse source for import analysis because the content contains invalid JS syntax. If you are using JSX, make sure to name the file with the .jsx or .tsx extension.
  Plugin: vite:import-analysis
  File: /home/runner/work/query-tool/query-tool/node_modules/.vite/deps/@auth0_auth0-react.js?v=7ecd79c7:828:352189

Checklist

This section is for the PR reviewer

  • PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see our Contributing Guidelines for more info)
  • PR has a label for the release changelog or skip-release (to be applied by maintainers only)
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass
  • If the PR changes the participant-level and/or the dataset-level result file, the query-tool-results files in the neurobagel_examples repo have been regenerated

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

Summary by Sourcery

Enhancements:

  • Replace Google authentication with Auth0 for improved authentication management.

Copy link

sourcery-ai bot commented Nov 28, 2024

Reviewer's Guide by Sourcery

This 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 flow

sequenceDiagram
    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
Loading

Updated class diagram for authentication components

classDiagram
    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"
Loading

File-Level Changes

Change Details Files
Integrate Auth0 authentication system
  • Add Auth0Provider wrapper component in the app root
  • Configure Auth0 with domain and client ID settings
  • Implement loginWithRedirect for authentication flow
  • Add Auth0 hook usage for authentication state management
src/main.tsx
src/components/AuthDialog.tsx
Refactor authentication state management in main App component
  • Remove Google-specific authentication state
  • Add Auth0 hooks for authentication status and token claims
  • Update authentication dialog visibility logic
  • Simplify token management using Auth0's built-in functionality
src/App.tsx
Update Navbar component to use Auth0 user information
  • Remove manual user profile state management
  • Integrate Auth0 user profile data for avatar and name display
  • Update logout functionality to use Auth0's logout method
src/components/Navbar.tsx
Update test configurations and dependencies
  • Modify component tests to accommodate Auth0 changes
  • Add Auth0 package dependencies
  • Configure Vite to exclude Auth0 from dependency optimization
cypress/component/Navbar.cy.tsx
cypress/component/AuthDialog.cy.tsx
vite.config.ts
package.json

Assessment against linked issues

Issue Objective Addressed Explanation
#379 Configure Auth0 ID flow to receive an ID token
#379 Remove Google auth flow
#379 Test successful login/auth manually

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

netlify bot commented Nov 28, 2024

Deploy Preview for neurobagel-query ready!

Name Link
🔨 Latest commit d8fbcb5
🔍 Latest deploy log https://app.netlify.com/sites/neurobagel-query/deploys/674e01b80b3faf0008c61c10
😎 Deploy Preview https://deploy-preview-380--neurobagel-query.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@surchs surchs marked this pull request as ready for review November 29, 2024 03:23
Copy link

@sourcery-ai sourcery-ai bot left a 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

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

src/main.tsx Show resolved Hide resolved
@rmanaem rmanaem self-requested a review December 2, 2024 17:38
@rmanaem
Copy link
Contributor

rmanaem commented Dec 2, 2024

@surchs

Is this expected?
image

@surchs
Copy link
Contributor Author

surchs commented Dec 2, 2024

@rmanaem possibly yes. The callback URL is currently http://localhost:5173, so if you are running something different (e.g. different port), it wouldn't work. Let me add you to the Auth0 group

@surchs surchs added release Create a release when this PR is merged pr-internal Non-user-facing code improvement, will increment patch version when merged (0.0.+1) labels Dec 2, 2024
Copy link
Contributor

@rmanaem rmanaem left a comment

Choose a reason for hiding this comment

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

🧑‍🍳

@surchs surchs merged commit 51b2613 into main Dec 2, 2024
9 checks passed
@surchs surchs deleted the issue379 branch December 2, 2024 21:36
Copy link
Contributor

neurobagel-bot bot commented Dec 2, 2024

🚀 PR was released in v0.7.1 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-internal Non-user-facing code improvement, will increment patch version when merged (0.0.+1) release Create a release when this PR is merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch experimental auth to auth0
2 participants