-
Notifications
You must be signed in to change notification settings - Fork 256
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
feat(gateway): compare permission checks to permit #1706
Conversation
bbd06de
to
5cf5c53
Compare
f1fa172
to
e3c9bcb
Compare
03ecb3a
to
6c66c7b
Compare
6c66c7b
to
ef3f90b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit. If all tests are still passing then I don't see the need for updating any more tests.
@@ -32,18 +32,15 @@ where | |||
{ | |||
type Rejection = Error; | |||
|
|||
async fn from_request_parts(parts: &mut Parts, state: &S) -> Result<Self, Self::Rejection> { | |||
async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result<Self, Self::Rejection> { | |||
let claim = parts.extensions.get::<Claim>().ok_or(ErrorKind::Internal)?; | |||
let user_id = claim.sub.clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to still have a User
struct? We can just implement FromRequestParts
for the claim right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only traits defined in the current crate can be implemented for a type parameter
Claim can technically be moved to backends (it seems), but I'd skip it for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description of change
Compare project access to Permit project access.
Use Permit for project list.
Remove project list pagination.
How has this been tested? (if applicable)
Staging