[feature] More consistent API error handling (#637)

* update templates

* start reworking api error handling

* update template

* return AP status at web endpoint if negotiated

* start making api error handling much more consistent

* update account endpoints to new error handling

* use new api error handling in admin endpoints

* go fmt ./...

* use api error logic in app

* use generic error handling in auth

* don't export generic error handler

* don't defer clearing session

* user nicer error handling on oidc callback handler

* tidy up the sign in handler

* tidy up the token handler

* use nicer error handling in blocksget

* auth emojis endpoint

* fix up remaining api endpoints

* fix whoopsie during login flow

* regenerate swagger docs

* change http error logging to debug

1 files changed, 60 insertions, 35 deletions

diff --git a/internal/api/client/auth/callback.go b/internal/api/client/auth/callback.go
index a5c58647c..34a4995c8 100644
--- a/internal/api/client/auth/callback.go
+++ b/internal/api/client/auth/callback.go
@@ -30,7 +30,9 @@ import (
 	"github.com/gin-contrib/sessions"
 	"github.com/gin-gonic/gin"
 	"github.com/google/uuid"
+	"github.com/superseriousbusiness/gotosocial/internal/api"
 	"github.com/superseriousbusiness/gotosocial/internal/db"
+	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/oidc"
 	"github.com/superseriousbusiness/gotosocial/internal/validate"
@@ -40,11 +42,14 @@ import (
 func (m *Module) CallbackGETHandler(c *gin.Context) {
 	s := sessions.Default(c)
 
-	// first make sure the state set in the cookie is the same as the state returned from the external provider
+	// check the query vs session state parameter to mitigate csrf
+	// https://auth0.com/docs/secure/attack-protection/state-parameters
+
 	state := c.Query(callbackStateParam)
 	if state == "" {
 		m.clearSession(s)
-		c.JSON(http.StatusForbidden, gin.H{"error": "state query not found on callback"})
+		err := fmt.Errorf("%s parameter not found on callback query", callbackStateParam)
+		api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet)
 		return
 	}
 
@@ -52,84 +57,104 @@ func (m *Module) CallbackGETHandler(c *gin.Context) {
 	savedState, ok := savedStateI.(string)
 	if !ok {
 		m.clearSession(s)
-		c.JSON(http.StatusForbidden, gin.H{"error": "state not found in session"})
+		err := fmt.Errorf("key %s was not found in session", sessionState)
+		api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet)
 		return
 	}
 
 	if state != savedState {
 		m.clearSession(s)
-		c.JSON(http.StatusForbidden, gin.H{"error": "state mismatch"})
+		err := errors.New("mismatch between query state and session state")
+		api.ErrorHandler(c, gtserror.NewErrorUnauthorized(err, err.Error()), m.processor.InstanceGet)
 		return
 	}
 
+	// retrieve stored claims using code
 	code := c.Query(callbackCodeParam)
+	if code == "" {
+		m.clearSession(s)
+		err := fmt.Errorf("%s parameter not found on callback query", callbackCodeParam)
+		api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet)
+		return
+	}
 
-	claims, err := m.idp.HandleCallback(c.Request.Context(), code)
-	if err != nil {
+	claims, errWithCode := m.idp.HandleCallback(c.Request.Context(), code)
+	if errWithCode != nil {
 		m.clearSession(s)
-		c.JSON(http.StatusForbidden, gin.H{"error": err.Error()})
+		api.ErrorHandler(c, errWithCode, m.processor.InstanceGet)
 		return
 	}
 
-	// We can use the client_id on the session to retrieve info about the app associated with the client_id
+	// We can use the client_id on the session to retrieve
+	// info about the app associated with the client_id
 	clientID, ok := s.Get(sessionClientID).(string)
 	if !ok || clientID == "" {
 		m.clearSession(s)
-		c.JSON(http.StatusInternalServerError, gin.H{"error": "no client_id found in session during callback"})
+		err := fmt.Errorf("key %s was not found in session", sessionClientID)
+		api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, helpfulAdvice), m.processor.InstanceGet)
 		return
 	}
+
 	app := &gtsmodel.Application{}
 	if err := m.db.GetWhere(c.Request.Context(), []db.Where{{Key: sessionClientID, Value: clientID}}, app); err != nil {
 		m.clearSession(s)
-		c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("no application found for client id %s", clientID)})
+		safe := fmt.Sprintf("application for %s %s could not be retrieved", sessionClientID, clientID)
+		var errWithCode gtserror.WithCode
+		if err == db.ErrNoEntries {
+			errWithCode = gtserror.NewErrorBadRequest(err, safe, helpfulAdvice)
+		} else {
+			errWithCode = gtserror.NewErrorInternalError(err, safe, helpfulAdvice)
+		}
+		api.ErrorHandler(c, errWithCode, m.processor.InstanceGet)
 		return
 	}
 
-	user, err := m.parseUserFromClaims(c.Request.Context(), claims, net.IP(c.ClientIP()), app.ID)
-	if err != nil {
+	user, errWithCode := m.parseUserFromClaims(c.Request.Context(), claims, net.IP(c.ClientIP()), app.ID)
+	if errWithCode != nil {
 		m.clearSession(s)
-		c.JSON(http.StatusForbidden, gin.H{"error": err.Error()})
+		api.ErrorHandler(c, errWithCode, m.processor.InstanceGet)
 		return
 	}
 
 	s.Set(sessionUserID, user.ID)
 	if err := s.Save(); err != nil {
 		m.clearSession(s)
-		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+		api.ErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGet)
 		return
 	}
 
 	c.Redirect(http.StatusFound, OauthAuthorizePath)
 }
 
-func (m *Module) parseUserFromClaims(ctx context.Context, claims *oidc.Claims, ip net.IP, appID string) (*gtsmodel.User, error) {
+func (m *Module) parseUserFromClaims(ctx context.Context, claims *oidc.Claims, ip net.IP, appID string) (*gtsmodel.User, gtserror.WithCode) {
 	if claims.Email == "" {
-		return nil, errors.New("no email returned in claims")
+		err := errors.New("no email returned in claims")
+		return nil, gtserror.NewErrorBadRequest(err, err.Error())
 	}
 
 	// see if we already have a user for this email address
+	// if so, we don't need to continue + create one
 	user := &gtsmodel.User{}
 	err := m.db.GetWhere(ctx, []db.Where{{Key: "email", Value: claims.Email}}, user)
 	if err == nil {
-		// we do! so we can just return it
 		return user, nil
 	}
 
 	if err != db.ErrNoEntries {
-		// we have an actual error in the database
-		return nil, fmt.Errorf("error checking database for email %s: %s", claims.Email, err)
+		err := fmt.Errorf("error checking database for email %s: %s", claims.Email, err)
+		return nil, gtserror.NewErrorInternalError(err)
 	}
 
 	// maybe we have an unconfirmed user
 	err = m.db.GetWhere(ctx, []db.Where{{Key: "unconfirmed_email", Value: claims.Email}}, user)
 	if err == nil {
-		// user is unconfirmed so return an error
-		return nil, fmt.Errorf("user with email address %s is unconfirmed", claims.Email)
+		err := fmt.Errorf("user with email address %s is unconfirmed", claims.Email)
+		return nil, gtserror.NewErrorForbidden(err, err.Error())
 	}
 
 	if err != db.ErrNoEntries {
-		// we have an actual error in the database
-		return nil, fmt.Errorf("error checking database for email %s: %s", claims.Email, err)
+		err := fmt.Errorf("error checking database for email %s: %s", claims.Email, err)
+		return nil, gtserror.NewErrorInternalError(err)
 	}
 
 	// we don't have a confirmed or unconfirmed user with the claimed email address
@@ -138,10 +163,10 @@ func (m *Module) parseUserFromClaims(ctx context.Context, claims *oidc.Claims, i
 	// check if the email address is available for use; if it's not there's nothing we can so
 	emailAvailable, err := m.db.IsEmailAvailable(ctx, claims.Email)
 	if err != nil {
-		return nil, fmt.Errorf("email %s not available: %s", claims.Email, err)
+		return nil, gtserror.NewErrorBadRequest(err)
 	}
 	if !emailAvailable {
-		return nil, fmt.Errorf("email %s in use", claims.Email)
+		return nil, gtserror.NewErrorConflict(fmt.Errorf("email address %s is not available", claims.Email))
 	}
 
 	// now we need a username
@@ -149,12 +174,12 @@ func (m *Module) parseUserFromClaims(ctx context.Context, claims *oidc.Claims, i
 
 	// make sure claims.Name is defined since we'll be using that for the username
 	if claims.Name == "" {
-		return nil, errors.New("no name returned in claims")
+		err := errors.New("no name returned in claims")
+		return nil, gtserror.NewErrorBadRequest(err, err.Error())
 	}
 
 	// check if we can just use claims.Name as-is
-	err = validate.Username(claims.Name)
-	if err == nil {
+	if err = validate.Username(claims.Name); err == nil {
 		// the name we have on the claims is already a valid username
 		username = claims.Name
 	} else {
@@ -166,12 +191,12 @@ func (m *Module) parseUserFromClaims(ctx context.Context, claims *oidc.Claims, i
 		// lowercase the whole thing
 		lower := strings.ToLower(underscored)
 		// see if this is valid....
-		if err := validate.Username(lower); err == nil {
-			// we managed to get a valid username
-			username = lower
-		} else {
-			return nil, fmt.Errorf("couldn't parse a valid username from claims.Name value of %s", claims.Name)
+		if err := validate.Username(lower); err != nil {
+			err := fmt.Errorf("couldn't parse a valid username from claims.Name value of %s: %s", claims.Name, err)
+			return nil, gtserror.NewErrorBadRequest(err, err.Error())
 		}
+		// we managed to get a valid username
+		username = lower
 	}
 
 	var iString string
@@ -185,7 +210,7 @@ func (m *Module) parseUserFromClaims(ctx context.Context, claims *oidc.Claims, i
 	for i := 1; !found; i++ {
 		usernameAvailable, err := m.db.IsUsernameAvailable(ctx, username+iString)
 		if err != nil {
-			return nil, err
+			return nil, gtserror.NewErrorInternalError(err)
 		}
 		if usernameAvailable {
 			// no error so we've found a username that works
@@ -223,7 +248,7 @@ func (m *Module) parseUserFromClaims(ctx context.Context, claims *oidc.Claims, i
 	// create the user! this will also create an account and store it in the database so we don't need to do that here
 	user, err = m.db.NewSignup(ctx, username, "", requireApproval, claims.Email, password, ip, "", appID, emailVerified, admin)
 	if err != nil {
-		return nil, fmt.Errorf("error creating user: %s", err)
+		return nil, gtserror.NewErrorInternalError(err)
 	}
 
 	return user, nil