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

Update RestEasy Classic mappers and Vert.x HTTP to log messages related to 401 #28157

Merged
merged 1 commit into from
Sep 23, 2022

Conversation

sberyozkin
Copy link
Member

One of the OIDC refresh tests failed again today in #28142 however the raw logs show nothing related to the source of 401 despite me covering all of the CodeAuthenticationMechanism code related to producing 401.
It may suggest that 401 is likely reported from elsewhere, either from Vert.x HTTP or ResteasyClassic exception mappers, so here is one more and very likely the final attempt to trace it before disabling it as I really see no other sources of 401 or any reasons for it in the OIDC code:

  • Log in Vert.x HTTP and Resteasy Classic mappers where 401 is reported without any log messages
  • Also update CodeFlowTest#testTokenAutoRefresh to use a realm different from the one used by the previous testRPInitiatedLogout test as it might be that logging out the user from a logout-realm and then signing in to the same realm and auto-refreshing might be causing some transient side-effects in KC

@sberyozkin sberyozkin requested a review from gsmet September 22, 2022 12:55
@sberyozkin sberyozkin marked this pull request as draft September 22, 2022 12:58
import io.quarkus.security.AuthenticationCompletionException;

@Provider
public class AuthenticationCompletionExceptionMapper implements ExceptionMapper<AuthenticationCompletionException> {

private static final Logger log = Logger.getLogger(AuthenticationCompletionExceptionMapper.class.getName());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you enable debug for this category?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I realized I missed it so converted to Draft :-), checking now with one of the tests expecting 401

@@ -25,6 +27,8 @@
*/
@Singleton
public class HttpAuthenticator {
private static final Logger log = Logger.getLogger(HttpAuthenticator.class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here.

@@ -92,6 +92,7 @@ public void accept(Throwable throwable) {
}
});
} else if (throwable instanceof AuthenticationCompletionException) {
log.debug("Authentication has failed, returning HTTP status 401");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here.

@sberyozkin sberyozkin marked this pull request as ready for review September 22, 2022 15:58
@sberyozkin
Copy link
Member Author

I enabled DEBUG for all these categories

@sberyozkin
Copy link
Member Author

sberyozkin commented Sep 22, 2022

was not sure about lower case log var (vs LOG) but one of Vert.x HTTP and reasteasy exception mappers were already using log so I guess either option is fine

@gsmet gsmet merged commit 2b3b4f2 into quarkusio:main Sep 23, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants