@jgrandja
The
code(authorization_code) is a temporary credential that can be used one-time only, hence, theAUTHORIZATION_REQUEST_NOT_FOUNDerror.
The problem is not in authorization_code. AuthenticationWebFilter has authenticationFailureHandler to handle invalid authentication(authorization_code). OidcAuthorizationCodeReactiveAuthenticationManager throws OAuth2AuthenticationException which is AuthenticationException and browser is redirected to /login?error
The problem is in
1. ServerOAuth2AuthorizationCodeAuthenticationTokenConverter it throws OAuth2AuthorizationException and
2. AuthenticationWebFilter doesn't handle any errors from ServerAuthenticationConverter
Only one ServerAuthenticationConverter implementation will redirect browser to login page
ServerHttpBasicAuthenticationConverter cause it return Mono.empty() in case of any authentication problem
Originally posted by @iilkevych in https://github.com/spring-projects/spring-security/issues/7884#issuecomment-634278181
Comment From: wjlc
https://github.com/spring-projects/spring-security/issues/7884#issue-558408713
Comment From: iilkevych
Some ReactiveAuthenticationManager implementations also can throw OAuth2AuthorizationException It seems to be right to check all them in AuthenticationWebFilter using one base exception AccessDeniedException
Which is actually handled already
Comment From: jgrandja
@wjlc @iilkevych Thank you for this report.
The issue is in OAuth2AuthorizationCodeGrantWebFilter as it does not handle OAuth2AuthorizationException, which can be thrown by ServerOAuth2AuthorizationCodeAuthenticationTokenConverter.
The fix should be in OAuth2AuthorizationCodeGrantWebFilter to handle OAuth2AuthorizationException and delegate to authenticationFailureHandler on errors.
Would either of you be interested in submitting a PR for this?
Comment From: wjlc
@shazin This is not the correct place for the fix. plz check the issue and replicate it. https://github.com/spring-projects/spring-security/issues/7884#issue-558408713
Comment From: wjlc
@jgrandja
OAuth2AuthorizationCodeGrantWebFilter is just used in ServerHttpSecurity. OAuth2Client, it's just a case.
but my WebFluxSecurityConfig like this:
......
.and()
.oauth2Login()
.authenticationSuccessHandler(new CustomAuthenticationSuccessHandler())
......
oauth2Login() configured OAuth2LoginAuthenticationWebFilter that extends AuthenticationWebFilter.
AuthenticationWebFilter only handle AuthenticationException now, that should also handleOAuth2AuthorizationException.
The bad thing is they're in different packages and no inheritance relationship between OAuth2AuthorizationException and AuthenticationException.
Comment From: jgrandja
@wjlc Thanks for the feedback.
It looks like ServerOAuth2AuthorizationCodeAuthenticationTokenConverter should throw OAuth2AuthenticationException instead of OAuth2AuthorizationException. Let me think this one through but I think that is what needs to change.
Comment From: iilkevych
@jgrandja
It looks like
ServerOAuth2AuthorizationCodeAuthenticationTokenConvertershould throwOAuth2AuthenticationExceptioninstead ofOAuth2AuthorizationException. Let me think this one through but I think that is what needs to change.
This is not only issue for ServerOAuth2AuthorizationCodeAuthenticationTokenConverter
As I mentioned above:
Some
ReactiveAuthenticationManagerimplementations also can throwOAuth2AuthorizationException
.flatMap(authenticationManager -> authenticationManager.authenticate(token))
and AuthenticationWebFilter will not catch them.
Comment From: jgrandja
@iilkevych
Some ReactiveAuthenticationManager implementations also can throw OAuth2AuthorizationException
Which specific implementations are you referring to?
Comment From: iilkevych
OAuth2AuthorizationCodeReactiveAuthenticationManager -> OAuth2AuthorizationExchangeValidator
* @author Joe Grandja
:)
Comment From: jgrandja
Ah yes. Ok let me think about what changes are required here.
Comment From: jgrandja
Thanks for all the feedback @wjlc @iilkevych.
I just pushed the fix in these commits 4c902bb da4b626. This has been backported to 5.1.x.
Comment From: iilkevych
@jgrandja just in case, will this redirect user to /login?
Comment From: jgrandja
@iilkevych OAuth2LoginAuthenticationWebFilter will redirect to /login but OAuth2AuthorizationCodeGrantWebFilter will not
Comment From: iilkevych
@jgrandja
Then what changed with this fix?
Main main concern was that application redirects user to identity provider when session expires.
If user did not finish login after session expired and his new session expired then application should redirect user to login again and as soon as user have now session with identity provider hi will login implicitly.
From your previous comment I guess it will simply crash. Same we have today. no?
Comment From: wjlc
@jgrandja Then what changed with this fix? Main main concern was that application redirects user to identity provider when session expires. If user did not finish login after session expired and his new session expired then application should redirect user to login again and as soon as user have now session with identity provider hi will login implicitly. From your previous comment I guess it will simply crash. Same we have today. no?
you can add custom authenticationFailureHandler that returns errorCode, your frontend receive code and redirect /oauth2/authorization/xxx, then it will be login because you had session of the identity provider.
Comment From: jgrandja
@iilkevych Regarding your comment
...application should redirect user to login again...
I think this is where the confusion is? OAuth2AuthorizationCodeGrantWebFilter processes the 4.1.2. Authorization Response for the OAuth 2.0 Authorization Code Grant. NOTE: The OAuth 2.0 Authorization Framework deals specifically with Authorization, it does not address Authentication. So it doesn't make sense for OAuth2AuthorizationCodeGrantWebFilter to redirect to /login for a failed authorization.
OpenID Connect 1.0 extends the OAuth 2.0 Authorization Framework and addresses authentication. The OAuth2LoginAuthenticationWebFilter implements Authentication using the Authorization Code Flow, and will redirect to /login for failed authentication.
Comment From: iilkevych
@jgrandja
Who is authorizing what in OAuth 2.0 Authorization Framework?
I believe resource owner authorizes access to resource by client. There is nothing about authorizing anything to anybody in client app.
It's authorization server and resource owner. They are authorizing access to resource by client app.
Authorization code is authentication from client app's user perspective.
That is why OAuth2AuthorizationCodeGrantWebFilter:
- This {@code Filter} will then create an {@link OAuth2AuthorizationCodeAuthenticationToken} with
- the {@link OAuth2ParameterNames#CODE code} received and
- delegate it to the {@link ReactiveAuthenticationManager} to authenticate.
Am I wrong?
Comment From: jgrandja
@iilkevych I would recommend reading the specs to understand the difference between OAuth 2.0 Authorization Framework (Authorization) and OpenID Connect 1.0 (Authentication). The links are provided in the previous comment.
Also, it's not clear to me if you are still seeing an issue (bug) or missing functionality? I feel like this conversation has gone down a different path from the original issue. If you feel the current functionality is missing something or there is a bug then I would propose that you put together a minimal sample that reproduces the issue so I can better understand what you are expecting.
Comment From: iilkevych
@jgrandja I’m afraid I may not find time to create and explain example. Although I believe if state expired before user authenticated then user should not authenticate again. This should happen without any users action. This was initial issue and it is not resolved yet.