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

Authorization middleware, fixes to API and User PATCH #87

Closed
wants to merge 3 commits into from

Conversation

kenpaicat
Copy link
Contributor

@kenpaicat kenpaicat commented Jul 29, 2023

This PR implements Role-Based Authorization middleware, where Role is stored in UserDTO encoded into JWT. It also implements PATCH endpoints required for promotion, and user edits. Fixes to API are cosmetic.

This was linked to issues Jul 29, 2023
@kenpaicat kenpaicat changed the title Authorization middleware and fixes to API Authorization middleware, fixes to API and User PATCH Jul 29, 2023
@kenpaicat
Copy link
Contributor Author

kenpaicat commented Jul 30, 2023

Two (possibly) issues so far:

  1. JWT needs to be regenerated when User is updated, because JWT is based on UserDTO. This is slightly weird. Need to read up on JWT regen or what to do in this case.

  2. Because (1) we have annoyances in (already singletoned) tests. Current fix is to "relogin" after User PATCH, but this is wrong.

Currently this is how middleware is wrapped: Authentication(Authorization(Site)) and it is annoying.

More: https://stackoverflow.com/questions/43978021/update-change-roles-claim-or-any-other-claim-in-jwt

@codecov
Copy link

codecov bot commented Jul 30, 2023

Codecov Report

Merging #87 (3832eed) into dev (c136f87) will decrease coverage by 0.08%.
Report is 13 commits behind head on dev.
The diff coverage is 45.71%.

❗ Current head 3832eed differs from pull request most recent head 71ed7ee. Consider uploading reports for the commit 71ed7ee to get more accurate results

@@            Coverage Diff             @@
##              dev      #87      +/-   ##
==========================================
- Coverage   48.12%   48.04%   -0.08%     
==========================================
  Files          16       18       +2     
  Lines         374      487     +113     
==========================================
+ Hits          180      234      +54     
- Misses        194      253      +59     
Files Changed Coverage Δ
crates/laguna-backend-api/src/error/peer.rs 0.00% <ø> (ø)
crates/laguna-backend-api/src/error/torrent.rs 0.00% <0.00%> (ø)
crates/laguna-backend-api/src/login.rs 100.00% <ø> (ø)
crates/laguna-backend-api/src/misc.rs 0.00% <0.00%> (ø)
crates/laguna-backend-api/src/register.rs 100.00% <ø> (ø)
crates/laguna-backend-api/src/torrent.rs 0.00% <0.00%> (-6.25%) ⬇️
crates/laguna-backend-dto/src/login.rs 100.00% <ø> (ø)
crates/laguna-backend-dto/src/torrent.rs 0.00% <0.00%> (ø)
crates/laguna-backend-dto/src/validators/login.rs 85.00% <ø> (ø)
...rates/laguna-backend-middleware/src/auth_helper.rs 0.00% <0.00%> (ø)
... and 7 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kenpaicat kenpaicat added A-API area of application programming interface T-Refactor refactor T-Impl implementation A-Middleware area of middleware labels Aug 3, 2023
@kenpaicat
Copy link
Contributor Author

merged locally n pushed to dev

@kenpaicat kenpaicat closed this Aug 3, 2023
@kenpaicat kenpaicat deleted the impl-admin branch August 3, 2023 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-API area of application programming interface A-Middleware area of middleware T-Impl implementation T-Refactor refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BE] Authorization middleware User API: PATCH
2 participants