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

feat: Improve logging in the oauth2 callback handler #11370

Merged

Conversation

umi0410
Copy link
Contributor

@umi0410 umi0410 commented Jul 16, 2023

Fixes #11369

Motivation

The current implementation of the OAuth2 callback handler lacks sufficient logging, making it difficult to identify errors and misconfigurations related to SSO (Single Sign-On). By adding detailed logs, we can simplify the process of configuring SSO and troubleshoot any issues that may arise.

Due to the insufficient logs in the oauth2 callback handler, it wasn't easy to identify the errors and misconfigurations about sso.

If the servers writes detailed logs, it would help the process configuring sso.

Modifications

I have added error logging to the /oauth2/callback handler.

Verification

To verify these changes, I performed the following tests:

  1. Test with an incorrect client secret - I set the client secret to foobarbaz, which is an invalid value.
# before
time="2023-07-16T12:04:06.600Z" level=info duration=8.976035ms method=GET path=/oauth2/callback size=0 status=401

# after
time="2023-07-16T12:20:25.064Z" level=error msg="failed to get oauth2Token by using code from the oauth2 server" error="oauth2: \"unauthorized_client\" \"Invalid client secret\""
time="2023-07-16T12:20:25.064Z" level=info duration=9.112831ms method=GET path=/oauth2/callback size=0 status=401
  1. Test with an incorrect ID token issuer - I set the issuer to include the unnecessary port (80).
# before
time="2023-07-16T12:08:36.733Z" level=info duration=65.410745ms method=GET path=/oauth2/callback size=0 status=401

# after
time="2023-07-16T12:18:16.333Z" level=error msg="failed to verify the id token issued" error="oidc: id token issued by a different provider, expected \"http://keycloak-http.keycloak:80/auth/realms/myrealm\" got \"http://keycloak-http.keycloak/auth/realms/myrealm\""
time="2023-07-16T12:18:16.333Z" level=info duration=19.251945ms method=GET path=/oauth2/callback size=0 status=401

@umi0410 umi0410 changed the title feat: Improve logging in the SSO callback handler feat: Improve logging in the oauth2 callback handler Jul 16, 2023
Signed-off-by: Jinsu Park <dev.umijs@gmail.com>
@umi0410 umi0410 force-pushed the improve-logging-in-oauth2-callback branch from b2b1afa to 7700170 Compare July 16, 2023 12:29
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

This is great! Thanks

@terrytangyuan terrytangyuan enabled auto-merge (squash) July 16, 2023 13:50
@terrytangyuan terrytangyuan merged commit 22d4e17 into argoproj:master Jul 16, 2023
22 checks passed
@argoproj argoproj locked as resolved and limited conversation to collaborators Jun 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insufficient logging in /oauth2/callback handler when using SSO Authentication
3 participants