Show the error_description when it is included in authorization response

This commit is contained in:
Ryan Richard 2021-04-19 18:08:52 -07:00
parent c176d15aa7
commit ddc632b99c
2 changed files with 18 additions and 10 deletions

View File

@ -423,9 +423,15 @@ func (h *handlerState) cliBasedAuth(authorizeOptions *[]oauth2.AuthCodeOption) (
return nil, fmt.Errorf("error getting authorization: redirected to the wrong location: %s", rawLocation)
}
// Validate OAuth2 state and fail if it's incorrect (to block CSRF).
if err := h.state.Validate(location.Query().Get("state")); err != nil {
return nil, fmt.Errorf("missing or invalid state parameter in authorization response: %s", rawLocation)
}
// Get the auth code or return the error from the server.
authCode := location.Query().Get("code")
if authCode == "" {
// Check for error response parameters. See https://openid.net/specs/openid-connect-core-1_0.html#AuthError.
requiredErrorCode := location.Query().Get("error")
optionalErrorDescription := location.Query().Get("error_description")
if optionalErrorDescription == "" {
@ -434,11 +440,6 @@ func (h *handlerState) cliBasedAuth(authorizeOptions *[]oauth2.AuthCodeOption) (
return nil, fmt.Errorf("login failed with code %q: %s", requiredErrorCode, optionalErrorDescription)
}
// Validate OAuth2 state and fail if it's incorrect (to block CSRF).
if err := h.state.Validate(location.Query().Get("state")); err != nil {
return nil, fmt.Errorf("missing or invalid state parameter in authorization response: %s", rawLocation)
}
// Exchange the authorization code for access, ID, and refresh tokens and perform required
// validations on the returned ID token.
tokenCtx, tokenCtxCancelFunc := context.WithTimeout(context.Background(), httpRequestTimeout)
@ -655,10 +656,11 @@ func (h *handlerState) handleAuthCodeCallback(w http.ResponseWriter, r *http.Req
return httperr.New(http.StatusForbidden, "missing or invalid state parameter")
}
// Check for error response parameters.
// Check for error response parameters. See https://openid.net/specs/openid-connect-core-1_0.html#AuthError.
if errorParam := params.Get("error"); errorParam != "" {
// TODO This should also show the value of the optional "error_description" param if it exists.
// See https://openid.net/specs/openid-connect-core-1_0.html#AuthError
if errorDescParam := params.Get("error_description"); errorDescParam != "" {
return httperr.Newf(http.StatusBadRequest, "login failed with code %q: %s", errorParam, errorDescParam)
}
return httperr.Newf(http.StatusBadRequest, "login failed with code %q", errorParam)
}

View File

@ -724,7 +724,7 @@ func TestLogin(t *testing.T) { // nolint:gocyclo
return defaultLDAPTestOpts(t, h, &http.Response{
StatusCode: http.StatusFound,
Header: http.Header{"Location": []string{
"http://127.0.0.1:0/callback?error=access_denied&error_description=optional-error-description",
"http://127.0.0.1:0/callback?error=access_denied&error_description=optional-error-description&state=test-state",
}},
}, nil)
}
@ -756,7 +756,7 @@ func TestLogin(t *testing.T) { // nolint:gocyclo
return defaultLDAPTestOpts(t, h, &http.Response{
StatusCode: http.StatusFound,
Header: http.Header{"Location": []string{
"http://127.0.0.1:0/callback?error=access_denied",
"http://127.0.0.1:0/callback?error=access_denied&state=test-state",
}},
}, nil)
}
@ -1279,6 +1279,12 @@ func TestHandleAuthCodeCallback(t *testing.T) {
wantErr: `login failed with code "some_error"`,
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`,
wantHTTPStatus: http.StatusBadRequest,
},
{
name: "invalid code",
query: "state=test-state&code=invalid",