git.iko.soy/typeable/oauth2-proxy
on master help sign in
Merge pull request #1404 from oauth2-proxy/improve-no-auth-error

Improve error message when no cookie is found

commit9d8093f470e7fac5f1f699a4df9d7d586e16e6f4 authorJoel Speed <Joel.speed@hotmail.co.uk> dateMon 18 Oct 2021, 18:16 (5 years ago) parent6cc7da8d8deaa1
5 files changed+34−14
compare with master
unifiedsplit
CHANGELOG.md+1 oauthproxy.go+2 pkg/middleware/stored_session.go+1−1 pkg/sessions/cookie/session_store.go+2−2 pkg/sessions/tests/session_store_tests.go+28−11
CHANGELOG.md+1
@@ -18,6 +18,7 @@
1818
1919## Changes since v7.1.3
2020
21+- [#1404](https://github.com/oauth2-proxy/oauth2-proxy/pull/1404) Improve error message when no cookie is found (@JoelSpeed)
2122- [#1315](https://github.com/oauth2-proxy/oauth2-proxy/pull/1315) linkedin: Update provider to v2 (@wuurrd)
2223- [#1348](https://github.com/oauth2-proxy/oauth2-proxy/pull/1348) Using the native httputil proxy code for websockets rather than yhat/wsutil to properly handle HTTP-level failures (@thetrime)
2324- [#1379](https://github.com/oauth2-proxy/oauth2-proxy/pull/1379) Fix the manual sign in with --htpasswd-user-group switch (@janrotter)
oauthproxy.go+2
@@ -853,11 +853,13 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) {
853853 case ErrNeedsLogin:
854854 // we need to send the user to a login screen
855855 if p.forceJSONErrors || isAjax(req) {
856+ logger.Printf("No valid authentication in request. Access Denied.")
856857 // no point redirecting an AJAX request
857858 p.errorJSON(rw, http.StatusUnauthorized)
858859 return
859860 }
860861
862+ logger.Printf("No valid authentication in request. Initiating login.")
861863 if p.SkipProviderButton {
862864 p.OAuthStart(rw, req)
863865 } else {
pkg/middleware/stored_session.go+1−1
@@ -71,7 +71,7 @@ func (s *storedSessionLoader) loadSession(next http.Handler) http.Handler {
7171 }
7272
7373 session, err := s.getValidatedSession(rw, req)
74− if err != nil {
74+ if err != nil && !errors.Is(err, http.ErrNoCookie) {
7575 // In the case when there was an error loading the session,
7676 // we should clear the session
7777 logger.Errorf("Error loading cookied session: %v, removing session", err)
pkg/sessions/cookie/session_store.go+2−2
@@ -51,7 +51,7 @@ func (s *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) {
5151 c, err := loadCookie(req, s.Cookie.Name)
5252 if err != nil {
5353 // always http.ErrNoCookie
54− return nil, fmt.Errorf("cookie %q not present", s.Cookie.Name)
54+ return nil, err
5555 }
5656 val, _, ok := encryption.Validate(c, s.Cookie.Secret, s.Cookie.Expire)
5757 if !ok {
@@ -216,7 +216,7 @@ func loadCookie(req *http.Request, cookieName string) (*http.Cookie, error) {
216216 }
217217 }
218218 if len(cookies) == 0 {
219− return nil, fmt.Errorf("could not find cookie %s", cookieName)
219+ return nil, http.ErrNoCookie
220220 }
221221 return joinCookies(cookies, cookieName)
222222}
pkg/sessions/tests/session_store_tests.go+28−11
@@ -452,21 +452,38 @@ func SessionStoreInterfaceTests(in *testInput) {
452452 })
453453
454454 Context("when Load is called", func() {
455− BeforeEach(func() {
456− req := httptest.NewRequest("GET", "http://example.com/", nil)
457− resp := httptest.NewRecorder()
458− err := in.ss().Save(resp, req, in.session)
459− Expect(err).ToNot(HaveOccurred())
455+ Context("with a valid session cookie in the request", func() {
456+ BeforeEach(func() {
457+ req := httptest.NewRequest("GET", "http://example.com/", nil)
458+ resp := httptest.NewRecorder()
459+ err := in.ss().Save(resp, req, in.session)
460+ Expect(err).ToNot(HaveOccurred())
461+ for _, cookie := range resp.Result().Cookies() {
462+ in.request.AddCookie(cookie)
463+ }
464+ })
460465
461− for _, cookie := range resp.Result().Cookies() {
462− in.request.AddCookie(cookie)
463− }
466+ Context("before the refresh period", func() {
467+ LoadSessionTests(in)
468+ })
464469 })
465470
466− Context("before the refresh period", func() {
467− LoadSessionTests(in)
468− })
471+ Context("with no cookies in the request", func() {
472+ var loadedSession *sessionsapi.SessionState
473+ var loadErr error
474+
475+ BeforeEach(func() {
476+ loadedSession, loadErr = in.ss().Load(in.request)
477+ })
469478
479+ It("returns an empty session", func() {
480+ Expect(loadedSession).To(BeNil())
481+ })
482+
483+ It("should return a no cookie error", func() {
484+ Expect(loadErr).To(MatchError(http.ErrNoCookie))
485+ })
486+ })
470487 })
471488}
472489