-
Notifications
You must be signed in to change notification settings - Fork 66
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
Same error messages shown in CLI's callback web page and in terminal #1694
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1694 +/- ##
==========================================
- Coverage 79.16% 79.15% -0.02%
==========================================
Files 163 163
Lines 15769 15767 -2
==========================================
- Hits 12484 12480 -4
- Misses 2970 2972 +2
Partials 315 315
|
37e987f
to
cede640
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
I tested this change on a cluster which has a real DNS entry and TLS certs for the FederationDomain (not a
You can also see the preflight request (an OPTIONS request) in the Chrome debug tools: This works because the CLI was enhanced to respond appropriately to CORS preflight requests starting in version https://github.com/vmware-tanzu/pinniped/releases/tag/v0.14.0. However, this means that this login would presumably fail when using a Pinniped CLI older than v0.14.0. Since v0.14.0 was released Feb 2022 (1 year and 9 months ago), this seems like it should be okay. |
Also tried this in some other browsers:
In all cases, the user's login worked as expected. |
Addresses #1566. It turns out that the CLI was only hiding the error details in the CLI's callback web page response in the browser. The CLI was already printing the full error details at the terminal.
This PR ensures that all errors during the browser-based CLI login flow are shown with the same message and details in both the browser and in the terminal.
Note that when using the form_post method, as is used by the Supervisor, then the web page doesn't show any error message, but that is for an unrelated reason regarding what Javascript code is allowed to see in http responses. In that case, the user needs to look at the CLI's terminal to see the error message. This behavior is not changed by this PR.
To manually test the behavior changed by this PR, configure the Concierge to use a different issuer for its JWTAuthenticator (not the Supervisor, but something which does not support
form_post
) and then configure the OIDC client in such a way that the token endpoint of the issuer will return an error (e.g. configure the OIDC client to require a client secret, which is not supported by the Pinniped CLI). See https://pinniped.dev/docs/howto/concierge/configure-concierge-jwt/ for the basic steps of configuring the Concierge and CLI to use an issuer other than the Pinniped Supervisor.Release note: