cornflourblue / aspnet-core-3-signup-verification-api

ASP.NET Core 3.1 - Boilerplate API with Email Sign Up, Verification, Authentication & Forgot Password
https://jasonwatmore.com/post/2020/07/06/aspnet-core-3-boilerplate-api-with-email-sign-up-verification-authentication-forgot-password
MIT License
226 stars 93 forks source link

Added Swagger Authentication. Fixed Refresh Token Security Issue #9

Closed AcidRaZor closed 2 years ago

AcidRaZor commented 3 years ago

Added Authentication for Swagger to set the Bearer token for other Authorized API calls to test against.

Changed Authenticate function to only create a Refresh Token when no active Refresh Token is found so no Active Refresh Tokens are kept where a potential leak on the user could expose risk to the API

vascoferraz commented 3 years ago

I added the code for the Swagger authentication but it is accepting any string that I insert as the "Bearer token".

Also, if I don't do any authentication at all the Swagger is still accessible.

Am I doing something wrong?

Thanks.

AcidRaZor commented 3 years ago

I added the code for the Swagger authentication but it is accepting any string that I insert as the "Bearer token".

Also, if I don't do any authentication at all the Swagger is still accessible.

Am I doing something wrong?

Thanks.

Swagger will always be accessible, the problem with the current implementation was that you weren't able to execute on end-points that requires authorization. (requiring Postman, a write-up you did on your original post)

AFAIK, Swagger will accept any string (it doesn't itself know what the API token should be or authorizes against it, it just forwards that to the authorized request you're executing)

That doesn't mean your end-point will be authorized to fire as this needs to be a proper bearer token that you get through authorization. So if you're saying that Swagger is accepting any bearer token AND you can execute on an end-point that's marked as [authorize] and get valid data back, then there must be something wrong.

In my tests, it didn't allow execution on those end-points with no bearer/invalid bearer details.

vascoferraz commented 3 years ago

I added the code for the Swagger authentication but it is accepting any string that I insert as the "Bearer token". Also, if I don't do any authentication at all the Swagger is still accessible. Am I doing something wrong? Thanks.

Swagger will always be accessible, the problem with the current implementation was that you weren't able to execute on end-points that requires authorization. (requiring Postman, a write-up you did on your original post)

AFAIK, Swagger will accept any string (it doesn't itself know what the API token should be or authorizes against it, it just forwards that to the authorized request you're executing)

That doesn't mean your end-point will be authorized to fire as this needs to be a proper bearer token that you get through authorization. So if you're saying that Swagger is accepting any bearer token AND you can execute on an end-point that's marked as [authorize] and get valid data back, then there must be something wrong.

In my tests, it didn't allow execution on those end-points with no bearer/invalid bearer details.

Thank you so much for your reply.

I misunderstood the use of the bearer token on Swagger.

I was expecting that the changes you made was to implement an authentication on Swagger, that is, without a valid bearer token all endpoints would be disabled.

I tested with an invalid bearer token and the login endpoint was still accessible and I got confused because I was expecting that with a wrong bearer token no endpoint would be accessible. Again, I misunderstood the use of the bearer token on Swagger.

So, to conclude, your code works perfectly, that is, with an invalid Bearer token I cannot access any endpoint that uses a bearer token for authorization and with a valid bearer token I can access any endpoint that uses a bearer token for authorization.

Thank you for your help.

AcidRaZor commented 3 years ago

No problem :) it should make it easier for anyone to test the API without having to dig into Postman, and with the fix for the refresh token, it's more secure as well :)

AcidRaZor commented 2 years ago

@vascoferraz Just fixed up the branch a little since I'm revisiting open PR's I have and saw there's been some changes (one in another PR) that was applied to the code already.

Are you not interested in people contributing to the project via PR's? I believe my contribution to be quite helpful and solves an outstanding issue with new refresh tokens being created even though there's an active refresh token already? You possibly tried solving this issue with the addition of your remove old tokens method to keep it a bit clean, but doesn't fix the issue where multiple refresh tokens on the same day could be created (and possibly used by an exploiter that has a previous access token, even for 24 hours)

Anyway, hope you're doing well.