micronaut-projects / micronaut-security

The official Micronaut security solution
Apache License 2.0
168 stars 124 forks source link

Move login and refresh under the same endpoint #493

Open rafaelrenanpacheco opened 3 years ago

rafaelrenanpacheco commented 3 years ago

Hello!

Currently the login flow is handled by the /login endpoint. The OAuth2 Refresh Token is handled by the /oauth/access_token endpoint. The OAuth2 Password delegates everything to the normal login flow, which is the /login endpoint.

So, when using the password + refresh token to authenticate (do not mistake with authorization through 3rd party), we need to use 2 different endpoints. Since both flows are used to request access token, I think both should be under the same /oauth/access_token endpoint. Then, based on the grant_type value the proper handler would be selected accordingly.

This mix of endpoints causes simple problems like this in Insomnia:

1 - The fetch token works by sending a request to the /login endpoint:

image

2 - The refresh token has no clue the endpoint is /oauth/access_token, so it tries the /login and fails:

image

Is there a reason for this mix of endpoints to get access tokens?

Best regards, Rafael Pacheco.

sdelamo commented 3 years ago

If we choose to merge them, we could not do it until Micronaut security 3.0.0; the next mayor version. Since it will be a breaking change. I am not familiar with Insomnia

I don't know what to think about merging them. The endpoints are doing different things. The login endpoint sometimes will not issue an access token. E.g. you are login but you are using session based auth.

Having different routes allows us to do think such as restrict the refresh token cookie to the refresh path.

If you want to do this in your project now, you will need to create a bean which combines the logic of both Login and Refresh controller and don't enable the built-in endpoints. Let me know if you need help with that.

rafaelrenanpacheco commented 3 years ago

Hello @sdelamo!

I understand the complexity that you explained. IMO I think the bigger "issue" here is micronaut-security handling oauth + traditional authentication and authorization all together based on which property you have activated, which leads to this situation where the password access token and refresh access token having different routes. If you use keycloak, both password and refresh tokens are issued under the same route (and that is what insomnia (and postman?)) expect when trying to refresh an expired access token.

If this route merging goes to 3.0.0, I think it would need a bigger refactor to separate oauth authorization from other ways to authenticate using micronaut-security, for example:

I'm using Keycloak for now and I think this issue may be closed, unless you need it opened to further discuss this with your team.

Best regards, Rafael Pacheco.