[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
This commit is contained in:
tobi
2022-06-08 20:38:03 +02:00
committed by GitHub
parent 91c0ed863a
commit 1ede54ddf6
130 changed files with 2154 additions and 1673 deletions

View File

@ -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)
claims, err := m.idp.HandleCallback(c.Request.Context(), code)
if err != nil {
if code == "" {
m.clearSession(s)
c.JSON(http.StatusForbidden, gin.H{"error": err.Error()})
err := fmt.Errorf("%s parameter not found on callback query", callbackCodeParam)
api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet)
return
}
// We can use the client_id on the session to retrieve info about the app associated with the client_id
claims, errWithCode := m.idp.HandleCallback(c.Request.Context(), code)
if errWithCode != nil {
m.clearSession(s)
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
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"})
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)})
err := fmt.Errorf("key %s was not found in session", sessionClientID)
api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, helpfulAdvice), m.processor.InstanceGet)
return
}
user, err := m.parseUserFromClaims(c.Request.Context(), claims, net.IP(c.ClientIP()), app.ID)
if err != nil {
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.StatusForbidden, gin.H{"error": err.Error()})
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, errWithCode := m.parseUserFromClaims(c.Request.Context(), claims, net.IP(c.ClientIP()), app.ID)
if errWithCode != nil {
m.clearSession(s)
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