Tweak status codes

canonical · May 8, 2024 · 1b9841f · 1b9841f
1 parent dac9a5f
commit 1b9841f
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 10 deletions.
diff --git a/internal/jimmhttp/auth_handler.go b/internal/jimmhttp/auth_handler.go
@@ -123,39 +123,39 @@ func (oah *OAuthHandler) Callback(w http.ResponseWriter, r *http.Request) {
 	stateByCookie, err := r.Cookie(auth.StateKey)
 	if err != nil {
 		usrErr := errors.E("no state cookie present")
-		writeError(ctx, w, http.StatusBadRequest, usrErr, "no state cookie present")
+		writeError(ctx, w, http.StatusForbidden, usrErr, "no state cookie present")
 		return
 	}
 	stateByURL := r.URL.Query().Get("state")
 	if stateByCookie.Value != stateByURL {
 		err := errors.E("state does not match")
-		writeError(ctx, w, http.StatusBadRequest, err, "state does not match")
+		writeError(ctx, w, http.StatusForbidden, err, "state does not match")
 		return
 	}
 
 	code := r.URL.Query().Get("code")
 	if code == "" {
 		err := errors.E("missing auth code")
-		writeError(ctx, w, http.StatusBadRequest, err, "no authorisation code present")
+		writeError(ctx, w, http.StatusForbidden, err, "no authorisation code present")
 		return
 	}
 
 	authSvc := oah.authenticator
 
 	token, err := authSvc.Exchange(ctx, code)
 	if err != nil {
-		writeError(ctx, w, http.StatusBadRequest, err, "failed to exchange authcode")
+		writeError(ctx, w, http.StatusForbidden, err, "failed to exchange authcode")
 		return
 	}
 
 	email, err := authSvc.UserInfo(ctx, token)
 	if err != nil {
-		writeError(ctx, w, http.StatusBadRequest, err, "failed to retrieve user info")
+		writeError(ctx, w, http.StatusInternalServerError, err, "failed to retrieve user info")
 		return
 	}
 
 	if err := authSvc.UpdateIdentity(ctx, email, token); err != nil {
-		writeError(ctx, w, http.StatusBadRequest, err, "failed to update identity")
+		writeError(ctx, w, http.StatusInternalServerError, err, "failed to update identity")
 		return
 	}
 
@@ -166,7 +166,7 @@ func (oah *OAuthHandler) Callback(w http.ResponseWriter, r *http.Request) {
 		oah.secureCookies,
 		email,
 	); err != nil {
-		writeError(ctx, w, http.StatusBadRequest, err, "failed to setup session")
+		writeError(ctx, w, http.StatusInternalServerError, err, "failed to setup session")
 	}
 
 	http.Redirect(w, r, oah.dashboardFinalRedirectURL, http.StatusPermanentRedirect)

diff --git a/internal/jimmhttp/auth_handler_test.go b/internal/jimmhttp/auth_handler_test.go
@@ -138,7 +138,26 @@ func TestCallbackFailsNoState(t *testing.T) {
 
 	b, err := io.ReadAll(res.Body)
 	c.Assert(err, qt.IsNil)
-	c.Assert(string(b), qt.Equals, http.StatusText(http.StatusBadRequest)+" - no state cookie present")
+	c.Assert(string(b), qt.Equals, http.StatusText(http.StatusForbidden)+" - no state cookie present")
+}
+
+func TestCallbackFailsStateNoMatch(t *testing.T) {
+	c := qt.New(t)
+
+	db, sessionStore := setupDbAndSessionStore(c)
+	s, err := jimmtest.SetupTestDashboardCallbackHandler("<no dashboard needed for this test>", db, sessionStore)
+	c.Assert(err, qt.IsNil)
+	defer s.Close()
+
+	client := createClientWithStateCookie(c, s)
+	callbackURL := s.URL + jimmhttp.AuthResourceBasePath + jimmhttp.CallbackEndpoint
+	res, err := client.Get(callbackURL + "?state=567")
+	c.Assert(err, qt.IsNil)
+
+	defer res.Body.Close()
+	b, err := io.ReadAll(res.Body)
+	c.Assert(err, qt.IsNil)
+	c.Assert(string(b), qt.Equals, http.StatusText(http.StatusForbidden)+" - state does not match")
 }
 
 func TestCallbackFailsNoCodePresent(t *testing.T) {
@@ -159,7 +178,7 @@ func TestCallbackFailsNoCodePresent(t *testing.T) {
 
 	b, err := io.ReadAll(res.Body)
 	c.Assert(err, qt.IsNil)
-	c.Assert(string(b), qt.Equals, http.StatusText(http.StatusBadRequest)+" - missing auth code")
+	c.Assert(string(b), qt.Equals, http.StatusText(http.StatusForbidden)+" - missing auth code")
 }
 
 func TestCallbackFailsExchange(t *testing.T) {
@@ -180,5 +199,5 @@ func TestCallbackFailsExchange(t *testing.T) {
 
 	b, err := io.ReadAll(res.Body)
 	c.Assert(err, qt.IsNil)
-	c.Assert(string(b), qt.Equals, http.StatusText(http.StatusBadRequest)+" - authorisation code exchange failed")
+	c.Assert(string(b), qt.Equals, http.StatusText(http.StatusForbidden)+" - authorisation code exchange failed")
 }