Merge pull request #1697 from vmware-tanzu/show_errors_on_formpost
Show errors from the form_post POST request on the page
This commit is contained in:
Ryan Richard
GitHub
commit
af7d3092a5
|
|
@ -442,8 +442,11 @@ else
|
|||
fi
|
||||
|
||||
# Test that the federation domain is working before we proceed.
|
||||
echo "Fetching FederationDomain discovery info via command: ${proxy_env_vars}curl -fLsS --cacert \"$root_ca_crt_path\" \"$issuer/.well-known/openid-configuration\""
|
||||
https_proxy="$proxy_server" no_proxy="$proxy_except" curl -fLsS --cacert "$root_ca_crt_path" "$issuer/.well-known/openid-configuration" | jq .
|
||||
echo "Fetching FederationDomain discovery info via command:" \
|
||||
"${proxy_env_vars}curl -fLsS --retry-all-errors --retry 5 --cacert \"$root_ca_crt_path\" \"$issuer/.well-known/openid-configuration\""
|
||||
https_proxy="$proxy_server" no_proxy="$proxy_except" curl -fLsS \
|
||||
--retry-all-errors --retry 5 --cacert "$root_ca_crt_path" \
|
||||
"$issuer/.well-known/openid-configuration" | jq .
|
||||
|
||||
if [[ "$OSTYPE" == "darwin"* ]]; then
|
||||
certificateAuthorityData=$(cat "$root_ca_crt_path" | base64)
|
||||
|
|
|
|||
|
|
@ -67,6 +67,10 @@ code {
|
|||
background-image: url("data:image/svg+xml,%3Csvg version='1.1' width='36' height='36' viewBox='0 0 36 36' preserveAspectRatio='xMidYMid meet' xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E%3Ctitle%3Ecopy-to-clipboard-line%3C/title%3E%3Cpath d='M22.6,4H21.55a3.89,3.89,0,0,0-7.31,0H13.4A2.41,2.41,0,0,0,11,6.4V10H25V6.4A2.41,2.41,0,0,0,22.6,4ZM23,8H13V6.25A.25.25,0,0,1,13.25,6h2.69l.12-1.11A1.24,1.24,0,0,1,16.61,4a2,2,0,0,1,3.15,1.18l.09.84h2.9a.25.25,0,0,1,.25.25Z' class='clr-i-outline clr-i-outline-path-1'%3E%3C/path%3E%3Cpath d='M33.25,18.06H21.33l2.84-2.83a1,1,0,1,0-1.42-1.42L17.5,19.06l5.25,5.25a1,1,0,0,0,.71.29,1,1,0,0,0,.71-1.7l-2.84-2.84H33.25a1,1,0,0,0,0-2Z' class='clr-i-outline clr-i-outline-path-2'%3E%3C/path%3E%3Cpath d='M29,16h2V6.68A1.66,1.66,0,0,0,29.35,5H27.08V7H29Z' class='clr-i-outline clr-i-outline-path-3'%3E%3C/path%3E%3Cpath d='M29,31H7V7H9V5H6.64A1.66,1.66,0,0,0,5,6.67V31.32A1.66,1.66,0,0,0,6.65,33H29.36A1.66,1.66,0,0,0,31,31.33V22.06H29Z' class='clr-i-outline clr-i-outline-path-4'%3E%3C/path%3E%3Crect x='0' y='0' width='36' height='36' fill-opacity='0'/%3E%3C/svg%3E");
|
||||
}
|
||||
|
||||
.error {
|
||||
font-family: monospace;
|
||||
}
|
||||
|
||||
@keyframes loader {
|
||||
to {
|
||||
transform: rotate(360deg);
|
||||
|
|
|
|||
|
|
@ -30,5 +30,10 @@ SPDX-License-Identifier: Apache-2.0
|
|||
<code id="manual-auth-code">{{ .Parameters.Get "code" }}</code>
|
||||
</button>
|
||||
</div>
|
||||
<div id="error" class="state" data-favicon="⛔" data-title="Error during login" hidden>
|
||||
<h1>Error during login</h1>
|
||||
<p id="message" class="error"></p>
|
||||
<p>Please try again.</p>
|
||||
</div>
|
||||
</body>
|
||||
</html>
|
||||
|
|
|
|||
|
|
@ -2,7 +2,12 @@
|
|||
// SPDX-License-Identifier: Apache-2.0
|
||||
|
||||
window.onload = () => {
|
||||
const transitionToState = (id) => {
|
||||
const transitionToState = (id, message) => {
|
||||
// For the error state, there is also a message to show.
|
||||
if (id === 'error') {
|
||||
document.getElementById('message').innerText = message
|
||||
}
|
||||
|
||||
// Hide all the other ".state" <div>s.
|
||||
Array.from(document.querySelectorAll('.state')).forEach(e => e.hidden = true);
|
||||
|
||||
|
|
@ -44,22 +49,31 @@ window.onload = () => {
|
|||
responseParams['redirect_uri'].value,
|
||||
{
|
||||
method: 'POST',
|
||||
mode: 'no-cors', // in the future, we could change this to "cors" (see comment below)
|
||||
mode: 'cors', // Using 'cors' is required to get actual response status codes.
|
||||
headers: {'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8'},
|
||||
body: responseParams['encoded_params'].value,
|
||||
})
|
||||
.then(response => {
|
||||
clearTimeout(timeout);
|
||||
// Requests made using "no-cors" mode will hide the real response.status by making it 0
|
||||
// and the real response.ok by making it false.
|
||||
// If the real response was success, then we would like to show the success state.
|
||||
// If the real response was an error, then we wish we could do something else (maybe show the error?),
|
||||
// but we have no way to know the real response as long as we are making "no-cors" requests.
|
||||
// For now, show the success status for all responses.
|
||||
// In the future, we could make this request in "cors" mode once old versions of our CLI
|
||||
// which did not handle CORS are upgraded out by our users. That would allow us to use
|
||||
// a conditional statement based on response.ok here to decide which state to transition into.
|
||||
transitionToState('success');
|
||||
if (response.ok) {
|
||||
// Got 2XX http response status, so the user has logged in successfully.
|
||||
transitionToState('success');
|
||||
} else {
|
||||
// Got non-2XX http response status. Show the error after trying to read the response body.
|
||||
// These are not recoverable errors. The CLI stop listening and is no longer prompting for authcode.
|
||||
response.text()
|
||||
.then(function (text) {
|
||||
transitionToState('error', response.status + ": " + text);
|
||||
})
|
||||
.catch((reason) => {
|
||||
console.error("error while reading response.text()", reason);
|
||||
transitionToState('error', response.status + ": [could not read response body]");
|
||||
})
|
||||
}
|
||||
})
|
||||
// A network error is encountered or CORS is misconfigured on the server-side.
|
||||
// This could happen in the case where the CLI is running on a different machine (e.g. ssh jumphost).
|
||||
// This always happens in Safari because that browser always prevents an https (TLS) web site from making
|
||||
// fetch calls to an http (non-TLS) localhost site (see https://bugs.webkit.org/show_bug.cgi?id=171934).
|
||||
.catch(() => transitionToState('manual'));
|
||||
};
|
||||
|
|
|
|||
|
|
@ -29,8 +29,8 @@ var (
|
|||
<html lang="en">
|
||||
<head>
|
||||
<meta charset="UTF-8">
|
||||
<style>body{font-family:metropolis-light,Helvetica,sans-serif}h1{font-size:20px}.state{position:absolute;top:100px;left:50%;width:400px;height:80px;margin-top:-40px;margin-left:-200px;font-size:14px;line-height:24px}button{margin:-10px;padding:10px;text-align:left;width:100%;display:inline;border:none;background:0 0;cursor:pointer;transition:all .1s}button:hover{background-color:#eee;transform:scale(1.01)}button:active{background-color:#ddd;transform:scale(.99)}code{display:block;word-wrap:break-word;word-break:break-all;font-size:12px;font-family:monospace;color:#333}.copy-icon{float:left;width:36px;height:36px;margin-top:-3px;margin-right:10px;background-size:contain;background-repeat:no-repeat;background-image:url("data:image/svg+xml,%3Csvg version='1.1' width='36' height='36' viewBox='0 0 36 36' preserveAspectRatio='xMidYMid meet' xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E%3Ctitle%3Ecopy-to-clipboard-line%3C/title%3E%3Cpath d='M22.6,4H21.55a3.89,3.89,0,0,0-7.31,0H13.4A2.41,2.41,0,0,0,11,6.4V10H25V6.4A2.41,2.41,0,0,0,22.6,4ZM23,8H13V6.25A.25.25,0,0,1,13.25,6h2.69l.12-1.11A1.24,1.24,0,0,1,16.61,4a2,2,0,0,1,3.15,1.18l.09.84h2.9a.25.25,0,0,1,.25.25Z' class='clr-i-outline clr-i-outline-path-1'%3E%3C/path%3E%3Cpath d='M33.25,18.06H21.33l2.84-2.83a1,1,0,1,0-1.42-1.42L17.5,19.06l5.25,5.25a1,1,0,0,0,.71.29,1,1,0,0,0,.71-1.7l-2.84-2.84H33.25a1,1,0,0,0,0-2Z' class='clr-i-outline clr-i-outline-path-2'%3E%3C/path%3E%3Cpath d='M29,16h2V6.68A1.66,1.66,0,0,0,29.35,5H27.08V7H29Z' class='clr-i-outline clr-i-outline-path-3'%3E%3C/path%3E%3Cpath d='M29,31H7V7H9V5H6.64A1.66,1.66,0,0,0,5,6.67V31.32A1.66,1.66,0,0,0,6.65,33H29.36A1.66,1.66,0,0,0,31,31.33V22.06H29Z' class='clr-i-outline clr-i-outline-path-4'%3E%3C/path%3E%3Crect x='0' y='0' width='36' height='36' fill-opacity='0'/%3E%3C/svg%3E")}@keyframes loader{to{transform:rotate(360deg)}}#loading{content:'';box-sizing:border-box;width:80px;height:80px;margin-top:-40px;margin-left:-40px;border-radius:50%;border:2px solid #fff;border-top-color:#1b3951;animation:loader .6s linear infinite}</style>
|
||||
<script>window.onload=()=>{const e=e=>{Array.from(document.querySelectorAll(".state")).forEach(e=>e.hidden=!0);const t=document.getElementById(e);t.hidden=!1,document.title=t.dataset.title,document.getElementById("favicon").setAttribute("href","data:image/svg+xml,<svg xmlns=%22http://www.w3.org/2000/svg%22 viewBox=%220 0 100 100%22><text y=%22.9em%22 font-size=%2290%22>"+t.dataset.favicon+"</text></svg>")};e("loading"),window.history.replaceState(null,"","./"),document.getElementById("manual-copy-button").onclick=()=>{const e=document.getElementById("manual-copy-button").innerText;navigator.clipboard.writeText(e).then(()=>console.info("copied authorization code "+e+" to clipboard")).catch(t=>console.error("failed to copy code "+e+" to clipboard: "+t))};const n=setTimeout(()=>e("manual"),2e3),t=document.forms[0].elements;fetch(t.redirect_uri.value,{method:"POST",mode:"no-cors",headers:{"Content-Type":"application/x-www-form-urlencoded;charset=UTF-8"},body:t.encoded_params.value}).then(t=>{clearTimeout(n),e("success")}).catch(()=>e("manual"))}</script>
|
||||
<style>body{font-family:metropolis-light,Helvetica,sans-serif}h1{font-size:20px}.state{position:absolute;top:100px;left:50%;width:400px;height:80px;margin-top:-40px;margin-left:-200px;font-size:14px;line-height:24px}button{margin:-10px;padding:10px;text-align:left;width:100%;display:inline;border:none;background:0 0;cursor:pointer;transition:all .1s}button:hover{background-color:#eee;transform:scale(1.01)}button:active{background-color:#ddd;transform:scale(.99)}code{display:block;word-wrap:break-word;word-break:break-all;font-size:12px;font-family:monospace;color:#333}.copy-icon{float:left;width:36px;height:36px;margin-top:-3px;margin-right:10px;background-size:contain;background-repeat:no-repeat;background-image:url("data:image/svg+xml,%3Csvg version='1.1' width='36' height='36' viewBox='0 0 36 36' preserveAspectRatio='xMidYMid meet' xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink'%3E%3Ctitle%3Ecopy-to-clipboard-line%3C/title%3E%3Cpath d='M22.6,4H21.55a3.89,3.89,0,0,0-7.31,0H13.4A2.41,2.41,0,0,0,11,6.4V10H25V6.4A2.41,2.41,0,0,0,22.6,4ZM23,8H13V6.25A.25.25,0,0,1,13.25,6h2.69l.12-1.11A1.24,1.24,0,0,1,16.61,4a2,2,0,0,1,3.15,1.18l.09.84h2.9a.25.25,0,0,1,.25.25Z' class='clr-i-outline clr-i-outline-path-1'%3E%3C/path%3E%3Cpath d='M33.25,18.06H21.33l2.84-2.83a1,1,0,1,0-1.42-1.42L17.5,19.06l5.25,5.25a1,1,0,0,0,.71.29,1,1,0,0,0,.71-1.7l-2.84-2.84H33.25a1,1,0,0,0,0-2Z' class='clr-i-outline clr-i-outline-path-2'%3E%3C/path%3E%3Cpath d='M29,16h2V6.68A1.66,1.66,0,0,0,29.35,5H27.08V7H29Z' class='clr-i-outline clr-i-outline-path-3'%3E%3C/path%3E%3Cpath d='M29,31H7V7H9V5H6.64A1.66,1.66,0,0,0,5,6.67V31.32A1.66,1.66,0,0,0,6.65,33H29.36A1.66,1.66,0,0,0,31,31.33V22.06H29Z' class='clr-i-outline clr-i-outline-path-4'%3E%3C/path%3E%3Crect x='0' y='0' width='36' height='36' fill-opacity='0'/%3E%3C/svg%3E")}.error{font-family:monospace}@keyframes loader{to{transform:rotate(360deg)}}#loading{content:'';box-sizing:border-box;width:80px;height:80px;margin-top:-40px;margin-left:-40px;border-radius:50%;border:2px solid #fff;border-top-color:#1b3951;animation:loader .6s linear infinite}</style>
|
||||
<script>window.onload=()=>{const e=(e,t)=>{e==="error"&&(document.getElementById("message").innerText=t),Array.from(document.querySelectorAll(".state")).forEach(e=>e.hidden=!0);const n=document.getElementById(e);n.hidden=!1,document.title=n.dataset.title,document.getElementById("favicon").setAttribute("href","data:image/svg+xml,<svg xmlns=%22http://www.w3.org/2000/svg%22 viewBox=%220 0 100 100%22><text y=%22.9em%22 font-size=%2290%22>"+n.dataset.favicon+"</text></svg>")};e("loading"),window.history.replaceState(null,"","./"),document.getElementById("manual-copy-button").onclick=()=>{const e=document.getElementById("manual-copy-button").innerText;navigator.clipboard.writeText(e).then(()=>console.info("copied authorization code "+e+" to clipboard")).catch(t=>console.error("failed to copy code "+e+" to clipboard: "+t))};const n=setTimeout(()=>e("manual"),2e3),t=document.forms[0].elements;fetch(t.redirect_uri.value,{method:"POST",mode:"cors",headers:{"Content-Type":"application/x-www-form-urlencoded;charset=UTF-8"},body:t.encoded_params.value}).then(t=>{clearTimeout(n),t.ok?e("success"):t.text().then(function(n){e("error",t.status+": "+n)}).catch(n=>{console.error("error while reading response.text()",n),e("error",t.status+": [could not read response body]")})}).catch(()=>e("manual"))}</script>
|
||||
<link id="favicon" rel="icon"/>
|
||||
</head>
|
||||
<body>
|
||||
|
|
@ -54,6 +54,11 @@ var (
|
|||
<code id="manual-auth-code">test-S629KHsCCBYV0PQ6FDSrn6iEXtVImQRBh7NCAk.JezyUSdCiSslYjtUmv7V5VAgiCz3ZkES9mYldg9GhqU</code>
|
||||
</button>
|
||||
</div>
|
||||
<div id="error" class="state" data-favicon="⛔" data-title="Error during login" hidden>
|
||||
<h1>Error during login</h1>
|
||||
<p id="message" class="error"></p>
|
||||
<p>Please try again.</p>
|
||||
</div>
|
||||
</body>
|
||||
</html>
|
||||
`)
|
||||
|
|
@ -61,8 +66,8 @@ var (
|
|||
// It's okay if this changes in the future, but this gives us a chance to eyeball the formatting.
|
||||
// Our browser-based integration tests should find any incompatibilities.
|
||||
testExpectedCSP = `default-src 'none'; ` +
|
||||
`script-src 'sha256-uIWC0J7wd7tWtcXmugZCkKsQpqOsQzqBI/mfQMtUde0='; ` +
|
||||
`style-src 'sha256-kXh6OrB2z7wkx7v1N3ay9deQhV5edwuogARaUtvNYN4='; ` +
|
||||
`script-src 'sha256-fiAdxAQHPoodG4cbENki/1TI+cjBOXxw+ADCoCtepQo='; ` +
|
||||
`style-src 'sha256-p+fPKq5SYyVeT46EkDVZx28MRQ6wlWHdDm3o3qZFGTA='; ` +
|
||||
`img-src data:; ` +
|
||||
`connect-src *; ` +
|
||||
`frame-ancestors 'none'`
|
||||
|
|
|
|||
|
|
@ -922,7 +922,7 @@ func (h *handlerState) handleAuthCodeCallback(w http.ResponseWriter, r *http.Req
|
|||
w.Header().Set("Access-Control-Allow-Origin", allowOrigin)
|
||||
w.Header().Set("Vary", "*") // supposed to use Vary when Access-Control-Allow-Origin is a specific host
|
||||
} else {
|
||||
// Return HTTP 405 for anything that's not a GET.
|
||||
// When we are not using form_post, then return HTTP 405 for anything that's not a GET.
|
||||
if r.Method != http.MethodGet {
|
||||
h.logger.V(plog.KlogLevelDebug).Info("Pinniped: Got unexpected request on callback listener", "method", r.Method)
|
||||
w.WriteHeader(http.StatusMethodNotAllowed)
|
||||
|
|
@ -933,6 +933,9 @@ func (h *handlerState) handleAuthCodeCallback(w http.ResponseWriter, r *http.Req
|
|||
params = r.URL.Query()
|
||||
}
|
||||
|
||||
// At this point, it doesn't matter if we got the params from a form_post POST request or a regular GET request.
|
||||
// Next, validate the params, and if we got an authcode then try to use it to complete the login.
|
||||
|
||||
// Validate OAuth2 state and fail if it's incorrect (to block CSRF).
|
||||
if err := h.state.Validate(params.Get("state")); err != nil {
|
||||
return httperr.New(http.StatusForbidden, "missing or invalid state parameter")
|
||||
|
|
|
|||
|
|
@ -48,25 +48,15 @@ func TestFormPostHTML_Browser_Parallel(t *testing.T) {
|
|||
t.Run("callback server error", func(t *testing.T) {
|
||||
browser := browsertest.OpenBrowser(t)
|
||||
|
||||
// Serve the form_post template with a redirect URI that will return an HTTP 500 response.
|
||||
// Serve the form_post template with a redirect URI that will return an HTTP 400 response.
|
||||
responseParams := formpostRandomParams(t)
|
||||
formpostInitiate(t, browser, formpostTemplateServer(t, callbackURL+"?fail=500", responseParams))
|
||||
formpostInitiate(t, browser, formpostTemplateServer(t, callbackURL+"?fail=400", responseParams))
|
||||
|
||||
// Now we handle the callback and assert that we got what we expected.
|
||||
expectCallback(t, responseParams)
|
||||
|
||||
// This is not 100% the behavior we'd like, but because our JS is making
|
||||
// a cross-origin fetch() without CORS, we don't get to know anything
|
||||
// about the response (even whether it is 200 vs. 500), so this case
|
||||
// is the same as the success case.
|
||||
//
|
||||
// This case is fairly unlikely in practice, and if the CLI encounters
|
||||
// an error it can also expose it via stderr anyway.
|
||||
//
|
||||
// In the future, we could change the Javascript code to use mode 'cors'
|
||||
// because we have upgraded our CLI callback endpoint to handle CORS,
|
||||
// and then we could change this to formpostExpectManualState().
|
||||
formpostExpectSuccessState(t, browser)
|
||||
// This failure should cause the UI to enter the "error" state.
|
||||
formpostExpectErrorState(t, browser)
|
||||
})
|
||||
|
||||
t.Run("network failure", func(t *testing.T) {
|
||||
|
|
@ -113,7 +103,7 @@ func TestFormPostHTML_Browser_Parallel(t *testing.T) {
|
|||
// It returns the URL of the running test server and a function for fetching the next
|
||||
// received form POST parameters.
|
||||
//
|
||||
// The test server supports special `?fail=close` and `?fail=500` to force error cases.
|
||||
// The test server supports special `?fail=close` and `?fail=400` to force error cases.
|
||||
func formpostCallbackServer(t *testing.T) (string, func(*testing.T, url.Values)) {
|
||||
t.Helper()
|
||||
results := make(chan url.Values)
|
||||
|
|
@ -156,8 +146,9 @@ func formpostCallbackServer(t *testing.T) (string, func(*testing.T, url.Values))
|
|||
_ = conn.Close()
|
||||
}
|
||||
return
|
||||
case "500": // If "fail=500" is passed, return a 500 error.
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
case "400": // If "fail=400" is passed, return a 400 error.
|
||||
w.WriteHeader(http.StatusBadRequest)
|
||||
_, _ = w.Write([]byte("this is the text of the bad request error response"))
|
||||
return
|
||||
}
|
||||
}))
|
||||
|
|
@ -265,6 +256,19 @@ func formpostExpectSuccessState(t *testing.T, b *browsertest.Browser) {
|
|||
formpostExpectFavicon(t, b, "✅")
|
||||
}
|
||||
|
||||
// formpostExpectErrorState asserts that the page is in the "error" state.
|
||||
func formpostExpectErrorState(t *testing.T, b *browsertest.Browser) {
|
||||
t.Helper()
|
||||
t.Logf("expecting to see error message become visible...")
|
||||
b.WaitForVisibleElements(t, "div#error")
|
||||
errorDivText := b.TextOfFirstMatch(t, "div#error")
|
||||
require.Contains(t, errorDivText, "Error during login")
|
||||
require.Contains(t, errorDivText, "400: this is the text of the bad request error response")
|
||||
require.Contains(t, errorDivText, "Please try again.")
|
||||
require.Equal(t, "Error during login", b.Title(t))
|
||||
formpostExpectFavicon(t, b, "⛔")
|
||||
}
|
||||
|
||||
// formpostExpectManualState asserts that the page is in the "manual" state and returns the auth code.
|
||||
func formpostExpectManualState(t *testing.T, b *browsertest.Browser) string {
|
||||
t.Helper()
|
||||
|
|
|
|||
Loading…
Reference in New Issue