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

Same error messages shown in CLI's callback web page and in terminal #1694

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

cfryanr
Copy link
Member

@cfryanr cfryanr commented Sep 25, 2023

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:

Fix a bug that did not show some details of some error messages coming from an OIDC identity provider
in the browser during the login process. These missing details in the error message only happened when
the Concierge was configured to use an OIDC provider aside from the Pinniped Supervisor.

@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

Merging #1694 (cede640) into main (e25ecea) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            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              
Files Coverage Δ
pkg/oidcclient/login.go 90.30% <100.00%> (-0.04%) ⬇️

... and 1 file with indirect coverage changes

Copy link
Member

@benjaminapetersen benjaminapetersen left a 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!

@benjaminapetersen benjaminapetersen merged commit d44882f into main Sep 26, 2023
37 checks passed
@benjaminapetersen benjaminapetersen deleted the cli_login_page_errors branch September 26, 2023 18:54
@cfryanr
Copy link
Member Author

cfryanr commented Oct 9, 2023

I tested this change on a cluster which has a real DNS entry and TLS certs for the FederationDomain (not a .local hostname). I tried it using the current latest version of Chrome, which is v117. On this cluster, this change caused Chrome to run a CORS preflight request to the CLI's localhost listener before the Javascript was allowed to POST the authcode to the CLI's localhost listener. You can see it in the CLI's log statements here:

❯ rm -f ~/.config/pinniped/credentials.yaml && rm -f ~/.config/pinniped/sessions.yaml && PINNIPED_DEBUG=true kubectl get pods -A --kubeconfig ./kubeconfig.yaml
Mon, 09 Oct 2023 09:21:00 PDT  pinniped-login  cmd/login_oidc.go:243  Performing OIDC login  {"issuer": "https://le.test.pinniped.dev/test-issuer", "client id": "pinniped-cli"}
Mon, 09 Oct 2023 09:21:00 PDT  oidcclient/login.go:710  Pinniped: Performing OIDC discovery  {"issuer": "https://le.test.pinniped.dev/test-issuer"}
Log in by visiting this link:

    https://le.test.pinniped.dev/test-issuer/oauth2/authorize?access_type=offline&client_id=pinniped-cli&code_challenge=htTj9VGAti3ZNUjaKMcWU3R5FFx1c9h3kEiQDBTK5YU&code_challenge_method=S256&nonce=b0d76b9fb23a41ee94542f7520561e81&pinniped_idp_name=okta&pinniped_idp_type=oidc&redirect_uri=http%3A%2F%2F127.0.0.1%3A58345%2Fcallback&response_mode=form_post&response_type=code&scope=groups+offline_access+openid+pinniped%3Arequest-audience+username&state=8f132be319f0c701dc610a8a18944775

    Optionally, paste your authorization code: Mon, 09 Oct 2023 09:21:20 PDT  oidcclient/login.go:892  Pinniped: Got CORS preflight request from browser  {"origin": "https://le.test.pinniped.dev"}
Mon, 09 Oct 2023 09:21:21 PDT  upstreamoidc/upstreamoidc.go:426  claims from ID token  {"providerName": "", "keys": ["at_hash", "aud", "auth_time", "azp", "exp", "groups", "iat", "iss", "jti", "nonce", "rat", "sub", "username"]}
[...]

Mon, 09 Oct 2023 09:21:21 PDT  oidcclient/login.go:759  Pinniped: Performing RFC8693 token exchange  {"requestedAudience": "b11b46b2-9e6b-4f3e-8b99-5d7b98d5065"}
Mon, 09 Oct 2023 09:21:21 PDT  pinniped-login  cmd/login_oidc.go:253  Exchanging token for cluster credential  {"endpoint": "https://34.123.129.105", "authenticator type": "jwt", "authenticator name": "manual-testing"}
Mon, 09 Oct 2023 09:21:21 PDT  pinniped-login  cmd/login_oidc.go:261  Successfully exchanged token for cluster credential.
Mon, 09 Oct 2023 09:21:21 PDT  pinniped-login  cmd/login_oidc.go:268  caching cluster credential for future use.
Error from server (Forbidden): pods is forbidden: User "walrus@example.com" cannot list resource "pods" in API group "" at the cluster scope: decision made by impersonation-proxy.concierge.pinniped.dev

You can also see the preflight request (an OPTIONS request) in the Chrome debug tools:
Screenshot 2023-10-09 at 9 27 52 AM

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.

@cfryanr
Copy link
Member Author

cfryanr commented Oct 9, 2023

Also tried this in some other browsers:

  • The latest Microsoft Edge for MacOS browser (v117) sent the exact same CORS preflight request as Chrome.
  • The latest Firefox (v118) did not send a CORS preflight request.
  • The latest Safari (v16.6) still entirely blocks Javascript's fetch POST request, so it does not send a preflight request for it either.

In all cases, the user's login worked as expected.

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.

3 participants