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

Should we rename AzureManagedIdentityAuthorizationFilter? #149

Closed
stijnmoreels opened this issue Apr 27, 2020 · 9 comments · Fixed by #150
Closed

Should we rename AzureManagedIdentityAuthorizationFilter? #149

stijnmoreels opened this issue Apr 27, 2020 · 9 comments · Fixed by #150
Assignees
Labels
area:security All issues related to security documentation All issues related to documentation enhancement New feature or request good first issue Good for newcomers
Projects
Milestone

Comments

@stijnmoreels
Copy link
Member

stijnmoreels commented Apr 27, 2020

Describe the solution you'd like
Should we rename AzureManagedIdentityAuthorizationFilter since it supports more than just Azure Managed Identity?

@stijnmoreels stijnmoreels added enhancement New feature or request good first issue Good for newcomers documentation All issues related to documentation area:security All issues related to security labels Apr 27, 2020
@stijnmoreels stijnmoreels added this to the v1.0.0 milestone Apr 27, 2020
@stijnmoreels stijnmoreels added this to To do in Roadmap via automation Apr 27, 2020
@tomkerkhove tomkerkhove changed the title Rename and (XML) document the AzureManagedIdentityAuthorizationFilter Should we rename AzureManagedIdentityAuthorizationFilter? Apr 27, 2020
@tomkerkhove
Copy link
Contributor

Moving conversation here - Please share your ideas @gverstraete @fgheysels @krottiers.

If we don't yet, then we can still do it going forward.

@gverstraete
Copy link

BearerTokenAuthorizationFilter?

@tomkerkhove
Copy link
Contributor

So you think we should rename it? :)

@gverstraete
Copy link

So you think we should rename it? :)

yes :)

@fgheysels
Copy link
Member

fgheysels commented Apr 27, 2020

Indeed, I think we should rename it since the implementation is rather generic, but it is limited to JWT tokens, so I'd suggest JwtTokenAuthorizationFilter .

Is it even 'authorization' ? Now that I think of it, I'd say it is rather authentication ? We're just checking if the token is 'valid', not if the bearer of the token is allowed to perform action X of Y (except for: he's allowed to access the API).

@gverstraete
Copy link

I think for this component it's combining the two :) . But can I agree with the prefix JwtToken. No real preference..

@tomkerkhove
Copy link
Contributor

That's fine for me!

In terms of authorization or authentication, I think authorization is best approach as it is fully based on the JWT token reader which does the magic, it now validates that it's valid but could be extended and is using IAsyncAuthorizationFilter.

If that doesn't make sense, then it can be authentication filter but might be confusing.

@fgheysels
Copy link
Member

Authorization is fine; you can be authenticated and have a valid token, but the token might not represent the identity that we expect, and then you're authenticated bot not authorized. So, authorization is fine.

@tomkerkhove
Copy link
Contributor

Thanks for the input folks!

@stijnmoreels Let's rename everything to JwtTokenAuthorizationFilter then!

@stijnmoreels stijnmoreels self-assigned this Apr 28, 2020
Roadmap automation moved this from To do to Done Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:security All issues related to security documentation All issues related to documentation enhancement New feature or request good first issue Good for newcomers
Projects
Roadmap
  
Done
Development

Successfully merging a pull request may close this issue.

4 participants