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

Remove application roles #9157

Merged
merged 4 commits into from
Oct 3, 2024
Merged

Remove application roles #9157

merged 4 commits into from
Oct 3, 2024

Conversation

leovalais
Copy link
Contributor

Follow up #8984

Tip

Review commits individually.

  • Adds the Superuser builtin role
  • Adds the --superuser cmd flag
  • Removes the RoleConfig
  • Harmonizes the serialization of builtin role: we use the serde representation for everything. It's easier to manage, less error-prone, and the OpenAPI description of the endpoints becomes much more ergonomic.

@leovalais leovalais requested review from a team as code owners October 2, 2024 13:18
@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 37.15%. Comparing base (efce7b7) to head (21b7e98).
Report is 16 commits behind head on dev.

Files with missing lines Patch % Lines
editoast/editoast_authz/src/authorizer.rs 75.00% 2 Missing ⚠️
editoast/src/client/mod.rs 0.00% 2 Missing ⚠️
editoast/src/views/authz.rs 0.00% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #9157      +/-   ##
============================================
- Coverage     37.22%   37.15%   -0.07%     
  Complexity     2242     2242              
============================================
  Files          1265     1265              
  Lines        116356   116174     -182     
  Branches       3279     3280       +1     
============================================
- Hits          43315    43166     -149     
+ Misses        71087    71053      -34     
- Partials       1954     1955       +1     
Flag Coverage Δ
core 74.78% <ø> (ø)
editoast 72.05% <79.31%> (-0.06%) ⬇️
front 15.32% <100.00%> (-0.01%) ⬇️
gateway 2.22% <ø> (ø)
osrdyne 2.56% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 86.46% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@leovalais leovalais force-pushed the lva/remove-application-roles branch 3 times, most recently from 82b27c8 to 681a34d Compare October 2, 2024 13:56
@Khoyo
Copy link
Contributor

Khoyo commented Oct 2, 2024

Adds the --superuser cmd flag

Shouldn't it rather be named --single-user ? Since it doesn't give you a superuser role, it treats all users as superusers (which is what single user mode usually is)

Or maybe something more explicit, like --disable-access-control or --disable-authorization

@leovalais leovalais self-assigned this Oct 2, 2024
…flag

Signed-off-by: Leo Valais <leo.valais97@gmail.com>
They will be added back with another implementation when we'll have an
admin panel where admins will be able to configure them.

Signed-off-by: Leo Valais <leo.valais97@gmail.com>
Signed-off-by: Leo Valais <leo.valais97@gmail.com>
Signed-off-by: Leo Valais <leo.valais97@gmail.com>
@github-actions github-actions bot added the area:editoast Work on Editoast Service label Oct 2, 2024
Copy link
Contributor

@flomonster flomonster left a comment

Choose a reason for hiding this comment

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

Great PR! 🙏

editoast/src/client/mod.rs Show resolved Hide resolved
@leovalais leovalais added this pull request to the merge queue Oct 3, 2024
Merged via the queue into dev with commit 1482e8c Oct 3, 2024
24 checks passed
@leovalais leovalais deleted the lva/remove-application-roles branch October 3, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:editoast Work on Editoast Service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants