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

Insufficient logging in /oauth2/callback handler when using SSO Authentication #11369

Closed
umi0410 opened this issue Jul 16, 2023 · 2 comments · Fixed by #11370
Closed

Insufficient logging in /oauth2/callback handler when using SSO Authentication #11369

umi0410 opened this issue Jul 16, 2023 · 2 comments · Fixed by #11370
Labels

Comments

@umi0410
Copy link
Contributor

umi0410 commented Jul 16, 2023

Summary

Desire for more detailed error logging in the /oauth2/callback handler when using SSO

Use Cases

When enabling SSO Authentication for the first time, we may encounter subtle errors and misconifucations that can delay our setup process.

In such cases, having more detailed logs in the server would greatly assist us in identifying and resolving these misconfigurations and errors.

For instance, I deployed Keycloak and used it as an Identity Provider. I attempted to configure Argo Workflows SSO using Keycloak. However, misconfigured the OIDC issuer setting by mistake. Instead of setting it to http://keycloak-http.keycloak/auth/realms/myrealm, I incorrectly set it to http://keycloak-http.keycloak:80/auth/realms/myrealm. (i.e., I should have omitted the port).

Thie misconfiguration resulted in a simple 401 error without any logs in the server and lacked a detailed error message in the HTTP response to clients.

To gather more information, I added logging on every line in the HandleCallback function (ref:

func (s *sso) HandleCallback(w http.ResponseWriter, r *http.Request) {
).
Through this, I managed to identify that the error occurred during the verfication of ID tokens after their issuance.

Notably, I discovered that the HandleCallback function, unlike other functions, simply responds with w.WriteHeader(401), which led to insufficient logging or information about the errors specific to this function.

I believe there are others who may face similar challenges in setting up SSO, and having more detailed logging woulde be immensely helpful.


Message from the maintainers:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.

@umi0410 umi0410 added the type/feature Feature request label Jul 16, 2023
@terrytangyuan
Copy link
Member

This is excellent feedback. Thank you! Since you already went through the debugging process, would you like to put together a PR that improves the logging?

@umi0410
Copy link
Contributor Author

umi0410 commented Jul 16, 2023

@terrytangyuan Sure! I'd love to. Thank you for suggesting it first. I'll work on this ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants