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

refactor: Router scopes and permissions #1075

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cmaddox5
Copy link
Contributor

@cmaddox5 cmaddox5 commented Jan 6, 2025

Summary of changes

Asana Ticket: N/A

This PR represents a suggestion to how permissions are checked in requests. Before, we were using configuration to determine what permission applies to each route. This is easy to miss and I feel could be simplified by managing scopes a bit better in the router.

Tl;dr of changes:

  • The Permissions module and Authorize plug have been removed
  • New plugs have been created for checking Keycloak roles
  • Router scopes have been updated so the appropriate plug runs and authorizes the user
    • If a route can be accessed by anyone logged in through Keycloak, place it here
    • If a route can be accessed by anyone with read-only access or admins, place it here
    • If a route can be accessed only by admins, place it here

I find this easier to follow so wanted to present this as an option and open the topic for discussion.

Reviewer Checklist

  • Meets ticket's acceptance criteria
  • Any new or changed functions have typespecs
  • Tests were added for any new functionality (don't just rely on Codecov)
  • This branch was deployed to the staging environment and is currently running with no unexpected increase in warnings, and no errors or crashes.

@cmaddox5 cmaddox5 force-pushed the cm/auth-pipeline-tweak branch 3 times, most recently from 53f877f to 69610e3 Compare January 7, 2025 18:58
@cmaddox5 cmaddox5 force-pushed the cm/auth-pipeline-tweak branch from 69610e3 to 505d1fc Compare January 7, 2025 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant