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

Show errors from the form_post POST request on the page #1697

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

cfryanr
Copy link
Member

@cfryanr cfryanr commented Sep 28, 2023

During a Supervisor login, the Supervisor uses a form_post web page to send its authcode to the CLI's localhost http listener via a Javascript fetch request. Although it is not common, this request could fail. For example, the authcode exchange could return an error. This PR changes the Javascript on that page to show the error response on the page.

Here is an example of what an error will look like when rendered on the page:

Screenshot 2023-09-28 at 1 05 41 PM

I manually tested this in Chrome, Firefox, Edge (for Mac), and Safari. Note that Safari will never get these errors, because it will never send the post request due to a security limitation of Safari. Instead, Safari will always show the authcode for the user to copy/paste.

NOTE: Before releasing this, we should test it on a real Kubernetes cluster where the Supervisor has a real DNS entry. We can use the long-lived acceptance testing cluster for this. I'm curious if the browsers will behave differently in that case, since in my local testing it was using a domain which resolved to localhost (in my /etc/hosts file). I'm especially interested if the browser will cause CORS preflight requests for the Javascript POST request, which did not happen in my local testing.

Release note:

Show certain login errors on the Supervisor's login web page which previously were only shown at the CLI's terminal.

@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Merging #1697 (62c597e) into main (78cb862) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1697      +/-   ##
==========================================
- Coverage   79.16%   79.15%   -0.02%     
==========================================
  Files         163      163              
  Lines       15767    15767              
==========================================
- Hits        12482    12480       -2     
- Misses       2970     2972       +2     
  Partials      315      315              
Files Coverage Δ
pkg/oidcclient/login.go 90.30% <100.00%> (ø)

... and 1 file with indirect coverage changes

@cfryanr cfryanr merged commit af7d309 into main Oct 4, 2023
37 checks passed
@cfryanr cfryanr deleted the show_errors_on_formpost branch October 4, 2023 15:54
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