From c274112ecca48fabb3cfa8920bd857dbf018c491 Mon Sep 17 00:00:00 2001 From: u5surf Date: Mon, 27 May 2019 20:32:25 +0900 Subject: [PATCH] all: fix incorrect http status codes #135 --- authenticate/handlers.go | 5 +++-- authenticate/handlers_test.go | 4 ++-- proxy/handlers.go | 4 ++-- proxy/handlers_test.go | 8 ++++---- proxy/proxy_test.go | 2 +- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/authenticate/handlers.go b/authenticate/handlers.go index 5401d4112..b1d279f46 100644 --- a/authenticate/handlers.go +++ b/authenticate/handlers.go @@ -97,7 +97,7 @@ func (a *Authenticate) SignIn(w http.ResponseWriter, r *http.Request) { // original `state` parameter received from the proxy application. state := r.Form.Get("state") if state == "" { - httputil.ErrorResponse(w, r, "no state parameter supplied", http.StatusForbidden) + httputil.ErrorResponse(w, r, "no state parameter supplied", http.StatusBadRequest) return } @@ -227,6 +227,7 @@ func (a *Authenticate) getOAuthCallback(w http.ResponseWriter, r *http.Request) if code == "" { log.FromRequest(r).Error().Err(err).Msg("authenticate: provider missing code") return "", httputil.HTTPError{Code: http.StatusBadRequest, Message: "Missing Code"} + } // validate the returned code with the identity provider @@ -261,7 +262,7 @@ func (a *Authenticate) getOAuthCallback(w http.ResponseWriter, r *http.Request) } // sanity check, we are redirecting back to the same subdomain right? if !middleware.SameSubdomain(redirectURL, a.RedirectURL) { - return "", httputil.HTTPError{Code: http.StatusForbidden, Message: "Invalid Redirect URI domain"} + return "", httputil.HTTPError{Code: http.StatusBadRequest, Message: "Invalid Redirect URI domain"} } err = a.sessionStore.SaveSession(w, r, session) diff --git a/authenticate/handlers_test.go b/authenticate/handlers_test.go index 316201dc2..8bb8f32f1 100644 --- a/authenticate/handlers_test.go +++ b/authenticate/handlers_test.go @@ -77,7 +77,7 @@ func TestAuthenticate_SignIn(t *testing.T) { RefreshDeadline: time.Now().Add(10 * time.Second), }}, identity.MockProvider{ValidateResponse: true}, - http.StatusForbidden}, + http.StatusBadRequest}, {"session not valid", &sessions.MockSessionStore{ Session: &sessions.SessionState{ @@ -94,7 +94,7 @@ func TestAuthenticate_SignIn(t *testing.T) { RefreshToken: "RefreshToken", RefreshDeadline: time.Now().Add(10 * time.Second), }}, identity.MockProvider{ValidateResponse: true}, - http.StatusForbidden}, + http.StatusBadRequest}, {"session refresh error", &sessions.MockSessionStore{ Session: &sessions.SessionState{ diff --git a/proxy/handlers.go b/proxy/handlers.go index 681973b37..e44809248 100644 --- a/proxy/handlers.go +++ b/proxy/handlers.go @@ -120,7 +120,7 @@ func (p *Proxy) OAuthCallback(w http.ResponseWriter, r *http.Request) { } errorString := r.Form.Get("error") if errorString != "" { - httputil.ErrorResponse(w, r, errorString, http.StatusForbidden) + httputil.ErrorResponse(w, r, errorString, http.StatusBadRequest) return } @@ -228,7 +228,7 @@ func (p *Proxy) Proxy(w http.ResponseWriter, r *http.Request) { authorized, err := p.AuthorizeClient.Authorize(r.Context(), r.Host, session) if err != nil || !authorized { log.FromRequest(r).Warn().Err(err).Msg("proxy: user unauthorized") - httputil.ErrorResponse(w, r, "Access unauthorized", http.StatusForbidden) + httputil.ErrorResponse(w, r, "Access unauthorized", http.StatusUnauthorized) return } } diff --git a/proxy/handlers_test.go b/proxy/handlers_test.go index 9cdc3d311..65d549769 100644 --- a/proxy/handlers_test.go +++ b/proxy/handlers_test.go @@ -299,20 +299,20 @@ func TestProxy_Proxy(t *testing.T) { {"good", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusOK}, {"good cors preflight", optsCORS, http.MethodOptions, goodCORSHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusOK}, // same request as above, but with cors_allow_preflight=false in the policy - {"valid cors, but not allowed", opts, http.MethodOptions, goodCORSHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden}, + {"valid cors, but not allowed", opts, http.MethodOptions, goodCORSHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusUnauthorized}, // cors allowed, but the request is missing proper headers - {"invalid cors headers", optsCORS, http.MethodOptions, badCORSHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden}, + {"invalid cors headers", optsCORS, http.MethodOptions, badCORSHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusUnauthorized}, {"unexpected error", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{LoadError: errors.New("ok")}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusInternalServerError}, // redirect to start auth process {"unknown host", opts, http.MethodGet, defaultHeaders, "https://nothttpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusNotFound}, - {"user forbidden", opts, http.MethodGet, defaultHeaders, "https://nothttpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden}, + {"user forbidden", opts, http.MethodGet, defaultHeaders, "https://nothttpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusUnauthorized}, // authenticate errors {"weird load session error", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{LoadError: errors.New("weird"), Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusInternalServerError}, {"failed refreshed session", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: &sessions.SessionState{RefreshDeadline: time.Now().Add(-10 * time.Second)}}, clients.MockAuthenticate{RefreshError: errors.New("refresh error")}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusForbidden}, {"cannot resave refreshed session", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{SaveError: errors.New("weird"), Session: &sessions.SessionState{RefreshDeadline: time.Now().Add(-10 * time.Second)}}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusForbidden}, {"authenticate validation error", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: false}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusForbidden}, {"public access", optsPublic, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusOK}, - {"public access, but unknown host", optsPublic, http.MethodGet, defaultHeaders, "https://nothttpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden}, + {"public access, but unknown host", optsPublic, http.MethodGet, defaultHeaders, "https://nothttpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusUnauthorized}, // no session, redirect to login {"no http found (no session)", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{LoadError: http.ErrNoCookie}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusBadRequest}, } diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go index 7f7320184..f0e0b7553 100644 --- a/proxy/proxy_test.go +++ b/proxy/proxy_test.go @@ -251,7 +251,7 @@ func TestProxy_OAuthCallback(t *testing.T) { wantCode int }{ {"good", sessions.MockCSRFStore{ResponseCSRF: "ok", GetError: nil, Cookie: &http.Cookie{Name: "something_csrf", Value: "csrf_state"}}, sessions.MockSessionStore{Session: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken", RefreshDeadline: time.Now().Add(-10 * time.Second)}}, clients.MockAuthenticate{RedeemResponse: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken"}}, map[string]string{"code": "code", "state": "state"}, http.StatusFound}, - {"error", sessions.MockCSRFStore{ResponseCSRF: "ok", GetError: nil, Cookie: &http.Cookie{Name: "something_csrf", Value: "csrf_state"}}, sessions.MockSessionStore{Session: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken", RefreshDeadline: time.Now().Add(-10 * time.Second)}}, clients.MockAuthenticate{RedeemResponse: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken"}}, map[string]string{"error": "some error"}, http.StatusForbidden}, + {"error", sessions.MockCSRFStore{ResponseCSRF: "ok", GetError: nil, Cookie: &http.Cookie{Name: "something_csrf", Value: "csrf_state"}}, sessions.MockSessionStore{Session: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken", RefreshDeadline: time.Now().Add(-10 * time.Second)}}, clients.MockAuthenticate{RedeemResponse: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken"}}, map[string]string{"error": "some error"}, http.StatusBadRequest}, {"state err", sessions.MockCSRFStore{ResponseCSRF: "ok", GetError: nil, Cookie: &http.Cookie{Name: "something_csrf", Value: "csrf_state"}}, sessions.MockSessionStore{Session: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken", RefreshDeadline: time.Now().Add(-10 * time.Second)}}, clients.MockAuthenticate{RedeemResponse: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken"}}, map[string]string{"code": "code", "state": "error"}, http.StatusInternalServerError}, {"csrf err", sessions.MockCSRFStore{GetError: errors.New("error")}, sessions.MockSessionStore{Session: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken", RefreshDeadline: time.Now().Add(-10 * time.Second)}}, clients.MockAuthenticate{RedeemResponse: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken"}}, map[string]string{"code": "code", "state": "state"}, http.StatusBadRequest}, {"unmarshal err", sessions.MockCSRFStore{Cookie: &http.Cookie{Name: "something_csrf", Value: "unmarshal error"}}, sessions.MockSessionStore{Session: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken", RefreshDeadline: time.Now().Add(-10 * time.Second)}}, clients.MockAuthenticate{RedeemResponse: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken"}}, map[string]string{"code": "code", "state": "state"}, http.StatusInternalServerError},