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

fix: add authorization for UI routes #917

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

jescalada
Copy link
Contributor

@jescalada jescalada commented Feb 21, 2025

Fixes #918.

This PR fixes the UI route access issue (unlogged users and non-admins can no longer access any route). In other words, only admins should be able to access the list of users, and only logged-in users can see the rest. Unlogged users can only access the login page.

You'll see an Unauthorized page when accessing a route you don't have access to.

Another issue we'd like to fix is the layout and the admin routes, which are coupled. Thus, all routes are called /admin/* when we only want the actual admin routes to have this naming (and replace regular routes with /dashboard/). To keep things bite-sized, this will be pushed in a separate PR.

It builds on top of the OIDC implementation at #905. As mentioned in that PR, testing with OIDC will require editing the config.

Tests will be modified/added in the layout fix PR since the routes will change.

Thank you!

Changelog

  • Added AuthProvider
    • To provide context with the current user
  • Added PrivateRoute
    • A wrapper that handles route authorization/redirection
  • Added NotAuthorized and NotFound pages
    • To prevent showing blank pages or redirecting (which is confusing)

Copy link

linux-foundation-easycla bot commented Feb 21, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@github-actions github-actions bot added the fix label Feb 21, 2025
Copy link

netlify bot commented Feb 21, 2025

Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name Link
🔨 Latest commit 9e245c1
🔍 Latest deploy log https://app.netlify.com/sites/endearing-brigadeiros-63f9d0/deploys/67e41ae2b53fe900070a1ef7

@jescalada jescalada changed the title fix: add extra checks for admin routes fix: add authorization for UI routes Feb 21, 2025
@JamieSlome JamieSlome self-requested a review February 21, 2025 10:05
Copy link

codecov bot commented Mar 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.88%. Comparing base (585cda6) to head (9e245c1).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #917   +/-   ##
=======================================
  Coverage   61.88%   61.88%           
=======================================
  Files          49       49           
  Lines        1805     1805           
=======================================
  Hits         1117     1117           
  Misses        688      688           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

Add UI route authorization and decouple admin routes
2 participants