Fix form_post.js mistake from recent commit; Better CORS on callback
(cherry picked from commit 5d79d4b9dc
)
This commit is contained in:
parent
8698d71809
commit
dab653f8df
@ -44,18 +44,22 @@ window.onload = () => {
|
||||
responseParams['redirect_uri'].value,
|
||||
{
|
||||
method: 'POST',
|
||||
mode: 'no-cors',
|
||||
mode: 'no-cors', // in the future, we could change this to "cors" (see comment below)
|
||||
headers: {'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8'},
|
||||
body: responseParams['encoded_params'].value,
|
||||
})
|
||||
.then(response => {
|
||||
clearTimeout(timeout);
|
||||
if (response.ok) {
|
||||
transitionToState('success');
|
||||
} else {
|
||||
// Got non-2XX http response status.
|
||||
transitionToState('manual');
|
||||
}
|
||||
// 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 show the manual
|
||||
// state, but we have no way to know that, 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');
|
||||
})
|
||||
.catch(() => transitionToState('manual'));
|
||||
};
|
||||
|
@ -30,7 +30,7 @@ var (
|
||||
<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 width=%2236%22 height=%2236%22 viewBox=%220 0 36 36%22 xmlns=%22http://www.w3.org/2000/svg%22 xmlns:xlink=%22http://www.w3.org/1999/xlink%22%3E%3Ctitle%3Ecopy-to-clipboard-line%3C/title%3E%3Cpath d=%22M22.6 4H21.55a3.89 3.89.0 00-7.31.0H13.4A2.41 2.41.0 0011 6.4V10H25V6.4A2.41 2.41.0 0022.6 4zM23 8H13V6.25A.25.25.0 0113.25 6h2.69l.12-1.11A1.24 1.24.0 0116.61 4a2 2 0 013.15 1.18l.09.84h2.9a.25.25.0 01.25.25z%22 class=%22clr-i-outline clr-i-outline-path-1%22/%3E%3Cpath d=%22M33.25 18.06H21.33l2.84-2.83a1 1 0 10-1.42-1.42L17.5 19.06l5.25 5.25a1 1 0 00.71.29 1 1 0 00.71-1.7l-2.84-2.84H33.25a1 1 0 000-2z%22 class=%22clr-i-outline clr-i-outline-path-2%22/%3E%3Cpath d=%22M29 16h2V6.68A1.66 1.66.0 0029.35 5H27.08V7H29z%22 class=%22clr-i-outline clr-i-outline-path-3%22/%3E%3Cpath d=%22M29 31H7V7H9V5H6.64A1.66 1.66.0 005 6.67V31.32A1.66 1.66.0 006.65 33H29.36A1.66 1.66.0 0031 31.33V22.06H29z%22 class=%22clr-i-outline clr-i-outline-path-4%22/%3E%3Crect x=%220%22 y=%220%22 width=%2236%22 height=%2236%22 fill-opacity=%220%22/%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 a=b=>{Array.from(document.querySelectorAll('.state')).forEach(a=>a.hidden=!0);const a=document.getElementById(b);a.hidden=!1,document.title=a.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>'+a.dataset.favicon+'</text></svg>')};a('loading'),window.history.replaceState(null,'','./'),document.getElementById('manual-copy-button').onclick=()=>{const a=document.getElementById('manual-copy-button').innerText;navigator.clipboard.writeText(a).then(()=>console.info('copied authorization code '+a+' to clipboard')).catch(b=>console.error('failed to copy code '+a+' to clipboard: '+b))};const c=setTimeout(()=>a('manual'),2e3),b=document.forms[0].elements;fetch(b.redirect_uri.value,{method:'POST',mode:'no-cors',headers:{'Content-Type':'application/x-www-form-urlencoded;charset=UTF-8'},body:b.encoded_params.value}).then(b=>{clearTimeout(c),b.ok?a('success'):a('manual')}).catch(()=>a('manual'))}</script>
|
||||
<script>window.onload=()=>{const a=b=>{Array.from(document.querySelectorAll('.state')).forEach(a=>a.hidden=!0);const a=document.getElementById(b);a.hidden=!1,document.title=a.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>'+a.dataset.favicon+'</text></svg>')};a('loading'),window.history.replaceState(null,'','./'),document.getElementById('manual-copy-button').onclick=()=>{const a=document.getElementById('manual-copy-button').innerText;navigator.clipboard.writeText(a).then(()=>console.info('copied authorization code '+a+' to clipboard')).catch(b=>console.error('failed to copy code '+a+' to clipboard: '+b))};const c=setTimeout(()=>a('manual'),2e3),b=document.forms[0].elements;fetch(b.redirect_uri.value,{method:'POST',mode:'no-cors',headers:{'Content-Type':'application/x-www-form-urlencoded;charset=UTF-8'},body:b.encoded_params.value}).then(b=>{clearTimeout(c),a('success')}).catch(()=>a('manual'))}</script>
|
||||
<link id="favicon" rel="icon"/>
|
||||
</head>
|
||||
<body>
|
||||
@ -61,7 +61,7 @@ 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-A3Wb0nDQrxXF07tExs31mVq68ObC+TMpvX8GUFw4SZk='; ` +
|
||||
`script-src 'sha256-+M/LwI0kltqjqTbsYcEYpN4nMkcCMkOmJcr1pbUSP2Q='; ` +
|
||||
`style-src 'sha256-CtfkX7m8x2UdGYvGgDq+6b6yIAQsASW9pbQK+sG8fNA='; ` +
|
||||
`img-src data:; ` +
|
||||
`connect-src *; ` +
|
||||
@ -83,6 +83,7 @@ func TestTemplate(t *testing.T) {
|
||||
Parameters: testResponseParams,
|
||||
}))
|
||||
|
||||
// t.Logf("actual value:\n%s", buf2.String()) // useful when updating minify library causes new output
|
||||
require.Equal(t, buf.String(), buf2.String())
|
||||
require.Equal(t, testExpectedFormPostOutput, buf.String())
|
||||
}
|
||||
|
@ -1,4 +1,4 @@
|
||||
// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
|
||||
// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
|
||||
// Package oidcclient implements a CLI OIDC login flow.
|
||||
@ -831,6 +831,20 @@ func (h *handlerState) handleAuthCodeCallback(w http.ResponseWriter, r *http.Req
|
||||
|
||||
var params url.Values
|
||||
if h.useFormPost { // nolint:nestif
|
||||
// Return HTTP 405 for anything that's not a POST or an OPTIONS request.
|
||||
if r.Method != http.MethodPost && r.Method != http.MethodOptions {
|
||||
h.logger.V(debugLogLevel).Info("Pinniped: Got unexpected request on callback listener", "method", r.Method)
|
||||
w.WriteHeader(http.StatusMethodNotAllowed)
|
||||
return nil // keep listening for more requests
|
||||
}
|
||||
|
||||
// For POST and OPTIONS requests, calculate the allowed origin for CORS.
|
||||
issuerURL, parseErr := url.Parse(h.issuer)
|
||||
if parseErr != nil {
|
||||
return httperr.Wrap(http.StatusInternalServerError, "invalid issuer url", parseErr)
|
||||
}
|
||||
allowOrigin := issuerURL.Scheme + "://" + issuerURL.Host
|
||||
|
||||
if r.Method == http.MethodOptions {
|
||||
// Google Chrome decided that it should do CORS preflight checks for this Javascript form submission POST request.
|
||||
// See https://developer.chrome.com/blog/private-network-access-preflight/
|
||||
@ -842,12 +856,9 @@ func (h *handlerState) handleAuthCodeCallback(w http.ResponseWriter, r *http.Req
|
||||
return nil // keep listening for more requests
|
||||
}
|
||||
h.logger.V(debugLogLevel).Info("Pinniped: Got CORS preflight request from browser", "origin", origin)
|
||||
issuerURL, parseErr := url.Parse(h.issuer)
|
||||
if parseErr != nil {
|
||||
return httperr.Wrap(http.StatusInternalServerError, "invalid issuer url", parseErr)
|
||||
}
|
||||
// To tell the browser that it is okay to make the real POST request, return the following response.
|
||||
w.Header().Set("Access-Control-Allow-Origin", issuerURL.Scheme+"://"+issuerURL.Host)
|
||||
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
|
||||
w.Header().Set("Access-Control-Allow-Credentials", "false")
|
||||
w.Header().Set("Access-Control-Allow-Methods", "POST, OPTIONS")
|
||||
w.Header().Set("Access-Control-Allow-Private-Network", "true")
|
||||
@ -860,20 +871,21 @@ func (h *handlerState) handleAuthCodeCallback(w http.ResponseWriter, r *http.Req
|
||||
}
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
return nil // keep listening for more requests
|
||||
}
|
||||
|
||||
// Return HTTP 405 for anything that's not a POST.
|
||||
if r.Method != http.MethodPost {
|
||||
h.logger.V(debugLogLevel).Info("Pinniped: Got unexpected request on callback listener", "method", r.Method)
|
||||
w.WriteHeader(http.StatusMethodNotAllowed)
|
||||
return nil // keep listening for more requests
|
||||
}
|
||||
} // Otherwise, this is a POST request...
|
||||
|
||||
// Parse and pull the response parameters from an application/x-www-form-urlencoded request body.
|
||||
if err := r.ParseForm(); err != nil {
|
||||
return httperr.Wrap(http.StatusBadRequest, "invalid form", err)
|
||||
}
|
||||
params = r.Form
|
||||
|
||||
// Allow CORS requests for POST so in the future our Javascript code can be updated to use the fetch API's
|
||||
// mode "cors", and still be compatible with older CLI versions starting with those that have this code
|
||||
// for CORS headers. Updating to use CORS would allow our Javascript code (form_post.js) to see the true
|
||||
// http response status from this endpoint. Note that the POST response does not need to set as many CORS
|
||||
// headers as the OPTIONS preflight response.
|
||||
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.
|
||||
if r.Method != http.MethodGet {
|
||||
|
@ -1,4 +1,4 @@
|
||||
// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
|
||||
// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
|
||||
package oidcclient
|
||||
@ -1757,6 +1757,7 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
method: http.MethodPost,
|
||||
query: "",
|
||||
wantNoCallbacks: true,
|
||||
wantHeaders: map[string][]string{},
|
||||
wantHTTPStatus: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
@ -1765,6 +1766,7 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
query: "",
|
||||
opt: withFormPostMode,
|
||||
wantNoCallbacks: true,
|
||||
wantHeaders: map[string][]string{},
|
||||
wantHTTPStatus: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
@ -1775,24 +1777,28 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
body: []byte(`%`),
|
||||
opt: withFormPostMode,
|
||||
wantErr: `invalid form: invalid URL escape "%"`,
|
||||
wantHeaders: map[string][]string{},
|
||||
wantHTTPStatus: http.StatusBadRequest,
|
||||
},
|
||||
{
|
||||
name: "invalid state",
|
||||
query: "state=invalid",
|
||||
wantErr: "missing or invalid state parameter",
|
||||
wantHeaders: map[string][]string{},
|
||||
wantHTTPStatus: http.StatusForbidden,
|
||||
},
|
||||
{
|
||||
name: "error code from provider",
|
||||
query: "state=test-state&error=some_error",
|
||||
wantErr: `login failed with code "some_error"`,
|
||||
wantHeaders: map[string][]string{},
|
||||
wantHTTPStatus: http.StatusBadRequest,
|
||||
},
|
||||
{
|
||||
name: "error code with a description from provider",
|
||||
query: "state=test-state&error=some_error&error_description=optional%20error%20description",
|
||||
wantErr: `login failed with code "some_error": optional error description`,
|
||||
wantHeaders: map[string][]string{},
|
||||
wantHTTPStatus: http.StatusBadRequest,
|
||||
},
|
||||
{
|
||||
@ -1801,6 +1807,23 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
query: "",
|
||||
headers: map[string][]string{"Origin": {"https://some-origin.com"}},
|
||||
wantErr: `invalid issuer url: parse "://bad-url": missing protocol scheme`,
|
||||
wantHeaders: map[string][]string{},
|
||||
wantHTTPStatus: http.StatusInternalServerError,
|
||||
opt: func(t *testing.T) Option {
|
||||
return func(h *handlerState) error {
|
||||
h.useFormPost = true
|
||||
h.issuer = "://bad-url"
|
||||
return nil
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "in form post mode, invalid issuer url config during POST request returns an error",
|
||||
method: http.MethodPost,
|
||||
query: "",
|
||||
headers: map[string][]string{"Origin": {"https://some-origin.com"}},
|
||||
wantErr: `invalid issuer url: parse "://bad-url": missing protocol scheme`,
|
||||
wantHeaders: map[string][]string{},
|
||||
wantHTTPStatus: http.StatusInternalServerError,
|
||||
opt: func(t *testing.T) Option {
|
||||
return func(h *handlerState) error {
|
||||
@ -1816,6 +1839,7 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
query: "",
|
||||
opt: withFormPostMode,
|
||||
wantNoCallbacks: true,
|
||||
wantHeaders: map[string][]string{},
|
||||
wantHTTPStatus: http.StatusBadRequest,
|
||||
},
|
||||
{
|
||||
@ -1829,6 +1853,7 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
"Access-Control-Allow-Credentials": {"false"},
|
||||
"Access-Control-Allow-Methods": {"POST, OPTIONS"},
|
||||
"Access-Control-Allow-Origin": {"https://valid-issuer.com"},
|
||||
"Vary": {"*"},
|
||||
"Access-Control-Allow-Private-Network": {"true"},
|
||||
},
|
||||
opt: func(t *testing.T) Option {
|
||||
@ -1853,6 +1878,7 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
"Access-Control-Allow-Credentials": {"false"},
|
||||
"Access-Control-Allow-Methods": {"POST, OPTIONS"},
|
||||
"Access-Control-Allow-Origin": {"https://valid-issuer.com"},
|
||||
"Vary": {"*"},
|
||||
"Access-Control-Allow-Private-Network": {"true"},
|
||||
"Access-Control-Allow-Headers": {"header1, header2, header3"},
|
||||
},
|
||||
@ -1868,6 +1894,7 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
name: "invalid code",
|
||||
query: "state=test-state&code=invalid",
|
||||
wantErr: "could not complete code exchange: some exchange error",
|
||||
wantHeaders: map[string][]string{},
|
||||
wantHTTPStatus: http.StatusBadRequest,
|
||||
opt: func(t *testing.T) Option {
|
||||
return func(h *handlerState) error {
|
||||
@ -1887,6 +1914,7 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
name: "valid",
|
||||
query: "state=test-state&code=valid",
|
||||
wantHTTPStatus: http.StatusOK,
|
||||
wantHeaders: map[string][]string{"Content-Type": {"text/plain; charset=utf-8"}},
|
||||
opt: func(t *testing.T) Option {
|
||||
return func(h *handlerState) error {
|
||||
h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI}
|
||||
@ -1902,10 +1930,44 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "valid form_post",
|
||||
method: http.MethodPost,
|
||||
headers: map[string][]string{"Content-Type": {"application/x-www-form-urlencoded"}},
|
||||
body: []byte(`state=test-state&code=valid`),
|
||||
name: "valid form_post",
|
||||
method: http.MethodPost,
|
||||
headers: map[string][]string{"Content-Type": {"application/x-www-form-urlencoded"}},
|
||||
body: []byte(`state=test-state&code=valid`),
|
||||
wantHeaders: map[string][]string{
|
||||
"Access-Control-Allow-Origin": {"https://valid-issuer.com"},
|
||||
"Vary": {"*"},
|
||||
"Content-Type": {"text/plain; charset=utf-8"},
|
||||
},
|
||||
wantHTTPStatus: http.StatusOK,
|
||||
opt: func(t *testing.T) Option {
|
||||
return func(h *handlerState) error {
|
||||
h.useFormPost = true
|
||||
h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI}
|
||||
h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI {
|
||||
mock := mockUpstream(t)
|
||||
mock.EXPECT().
|
||||
ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI).
|
||||
Return(&oidctypes.Token{IDToken: &oidctypes.IDToken{Token: "test-id-token"}}, nil)
|
||||
return mock
|
||||
}
|
||||
return nil
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "valid form_post made with the same origin headers that would be used by a Javascript fetch client using mode=cors",
|
||||
method: http.MethodPost,
|
||||
headers: map[string][]string{
|
||||
"Content-Type": {"application/x-www-form-urlencoded"},
|
||||
"Origin": {"https://some-origin.com"},
|
||||
},
|
||||
body: []byte(`state=test-state&code=valid`),
|
||||
wantHeaders: map[string][]string{
|
||||
"Access-Control-Allow-Origin": {"https://valid-issuer.com"},
|
||||
"Vary": {"*"},
|
||||
"Content-Type": {"text/plain; charset=utf-8"},
|
||||
},
|
||||
wantHTTPStatus: http.StatusOK,
|
||||
opt: func(t *testing.T) Option {
|
||||
return func(h *handlerState) error {
|
||||
@ -1934,6 +1996,7 @@ func TestHandleAuthCodeCallback(t *testing.T) {
|
||||
pkce: pkce.Code("test-pkce"),
|
||||
nonce: nonce.Nonce("test-nonce"),
|
||||
logger: testlogger.New(t).Logger,
|
||||
issuer: "https://valid-issuer.com/with/some/path",
|
||||
}
|
||||
if tt.opt != nil {
|
||||
require.NoError(t, tt.opt(t)(h))
|
||||
|
@ -1,4 +1,4 @@
|
||||
// Copyright 2021 the Pinniped contributors. All Rights Reserved.
|
||||
// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
|
||||
package integration
|
||||
@ -60,6 +60,10 @@ func TestFormPostHTML_Parallel(t *testing.T) {
|
||||
//
|
||||
// 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, page)
|
||||
})
|
||||
|
||||
@ -108,6 +112,19 @@ func formpostCallbackServer(t *testing.T) (string, func(*testing.T, url.Values))
|
||||
results := make(chan url.Values)
|
||||
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
// 404 for any other requests aside from POSTs. We do not need to support CORS preflight OPTIONS
|
||||
// requests for this test because both the web page and the callback are on 127.0.0.1 (same origin).
|
||||
if r.Method != http.MethodPost {
|
||||
t.Logf("test callback server got unexpeted request method")
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
|
||||
// Allow CORS requests. This will be needed for this test in the future if we change
|
||||
// the Javascript code from using mode 'no-cors' to instead use mode 'cors'. At the
|
||||
// moment it should be ignored by the browser.
|
||||
w.Header().Set("Access-Control-Allow-Origin", "*")
|
||||
|
||||
assert.NoError(t, r.ParseForm())
|
||||
|
||||
// Extract only the POST parameters (r.Form also contains URL query parameters).
|
||||
|
Loading…
Reference in New Issue
Block a user