diff options
author | 2022-06-08 20:38:03 +0200 | |
---|---|---|
committer | 2022-06-08 20:38:03 +0200 | |
commit | 1ede54ddf6dfd2d4ba039eb7e23b74bcac65b643 (patch) | |
tree | 727436fb9bf9da25e30c5ded65c5b5ccaffe0cf0 /internal/web | |
parent | [bugfix] #621: add weak type handing to mapstructure decode (#625) (diff) | |
download | gotosocial-1ede54ddf6dfd2d4ba039eb7e23b74bcac65b643.tar.xz |
[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
Diffstat (limited to 'internal/web')
-rw-r--r-- | internal/web/base.go | 28 | ||||
-rw-r--r-- | internal/web/confirmemail.go | 16 | ||||
-rw-r--r-- | internal/web/profile.go | 57 | ||||
-rw-r--r-- | internal/web/thread.go | 96 |
4 files changed, 116 insertions, 81 deletions
diff --git a/internal/web/base.go b/internal/web/base.go index d203522ae..a8d99619c 100644 --- a/internal/web/base.go +++ b/internal/web/base.go @@ -19,6 +19,7 @@ package web import ( + "errors" "fmt" "io/ioutil" "net/http" @@ -29,6 +30,7 @@ import ( "github.com/sirupsen/logrus" "github.com/superseriousbusiness/gotosocial/internal/api" "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/processing" "github.com/superseriousbusiness/gotosocial/internal/router" "github.com/superseriousbusiness/gotosocial/internal/uris" @@ -118,24 +120,6 @@ func (m *Module) baseHandler(c *gin.Context) { }) } -// NotFoundHandler serves a 404 html page instead of a blank 404 error. -func (m *Module) NotFoundHandler(c *gin.Context) { - l := logrus.WithField("func", "404") - l.Trace("serving 404 html") - - host := config.GetHost() - instance, err := m.processor.InstanceGet(c.Request.Context(), host) - if err != nil { - l.Debugf("error getting instance from processor: %s", err) - c.JSON(http.StatusInternalServerError, gin.H{"error": "internal server error"}) - return - } - - c.HTML(404, "404.tmpl", gin.H{ - "instance": instance, - }) -} - // Route satisfies the RESTAPIModule interface func (m *Module) Route(s router.Router) error { // serve static files from assets dir at /assets @@ -152,16 +136,18 @@ func (m *Module) Route(s router.Router) error { s.AttachHandler(http.MethodGet, "/", m.baseHandler) // serve profile pages at /@username - s.AttachHandler(http.MethodGet, profilePath, m.profileTemplateHandler) + s.AttachHandler(http.MethodGet, profilePath, m.profileGETHandler) // serve statuses - s.AttachHandler(http.MethodGet, statusPath, m.threadTemplateHandler) + s.AttachHandler(http.MethodGet, statusPath, m.threadGETHandler) // serve email confirmation page at /confirm_email?token=whatever s.AttachHandler(http.MethodGet, confirmEmailPath, m.confirmEmailGETHandler) // 404 handler - s.AttachNoRouteHandler(m.NotFoundHandler) + s.AttachNoRouteHandler(func(c *gin.Context) { + api.ErrorHandler(c, gtserror.NewErrorNotFound(errors.New(http.StatusText(http.StatusNotFound))), m.processor.InstanceGet) + }) return nil } diff --git a/internal/web/confirmemail.go b/internal/web/confirmemail.go index 7c8aa464f..58f932bde 100644 --- a/internal/web/confirmemail.go +++ b/internal/web/confirmemail.go @@ -19,35 +19,35 @@ package web import ( + "errors" "net/http" "github.com/gin-gonic/gin" - "github.com/sirupsen/logrus" + "github.com/superseriousbusiness/gotosocial/internal/api" "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" ) func (m *Module) confirmEmailGETHandler(c *gin.Context) { + ctx := c.Request.Context() + // if there's no token in the query, just serve the 404 web handler token := c.Query(tokenParam) if token == "" { - m.NotFoundHandler(c) + api.ErrorHandler(c, gtserror.NewErrorNotFound(errors.New(http.StatusText(http.StatusNotFound))), m.processor.InstanceGet) return } - ctx := c.Request.Context() - user, errWithCode := m.processor.UserConfirmEmail(ctx, token) if errWithCode != nil { - logrus.Debugf("error confirming email: %s", errWithCode.Error()) - // if something goes wrong, just log it and direct to the 404 handler to not give anything away - m.NotFoundHandler(c) + api.ErrorHandler(c, errWithCode, m.processor.InstanceGet) return } host := config.GetHost() instance, err := m.processor.InstanceGet(ctx, host) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + api.ErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGet) return } diff --git a/internal/web/profile.go b/internal/web/profile.go index 3155c022d..051d55d28 100644 --- a/internal/web/profile.go +++ b/internal/web/profile.go @@ -21,59 +21,60 @@ package web import ( "context" "encoding/json" + "errors" "fmt" "math/rand" "net/http" + "strings" "github.com/gin-gonic/gin" - "github.com/sirupsen/logrus" "github.com/superseriousbusiness/gotosocial/internal/ap" "github.com/superseriousbusiness/gotosocial/internal/api" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/oauth" ) -func (m *Module) profileTemplateHandler(c *gin.Context) { - l := logrus.WithField("func", "profileTemplateHandler") - l.Trace("rendering profile template") +func (m *Module) profileGETHandler(c *gin.Context) { ctx := c.Request.Context() - username := c.Param(usernameKey) + authed, err := oauth.Authed(c, false, false, false, false) + if err != nil { + api.ErrorHandler(c, gtserror.NewErrorUnauthorized(err, err.Error()), m.processor.InstanceGet) + return + } + + // usernames on our instance will always be lowercase + username := strings.ToLower(c.Param(usernameKey)) if username == "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "no account username specified"}) + err := errors.New("no account username specified") + api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet) return } - authed, err := oauth.Authed(c, false, false, false, false) + host := config.GetHost() + instance, err := m.processor.InstanceGet(ctx, host) if err != nil { - l.Errorf("error authing profile GET request: %s", err) - c.JSON(http.StatusInternalServerError, gin.H{"error": "internal server error"}) + api.ErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGet) return } - instance, errWithCode := m.processor.InstanceGet(ctx, config.GetHost()) - if errWithCode != nil { - l.Debugf("error getting instance from processor: %s", errWithCode.Error()) - c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()}) - return + instanceGet := func(ctx context.Context, domain string) (*apimodel.Instance, gtserror.WithCode) { + return instance, nil } account, errWithCode := m.processor.AccountGetLocalByUsername(ctx, authed, username) if errWithCode != nil { - l.Debugf("error getting account from processor: %s", errWithCode.Error()) - if errWithCode.Code() == http.StatusNotFound { - m.NotFoundHandler(c) - return - } - c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()}) + api.ErrorHandler(c, errWithCode, instanceGet) return } - // if we're getting an AP request on this endpoint we should render the account's AP representation instead + // if we're getting an AP request on this endpoint we + // should render the account's AP representation instead accept := c.NegotiateFormat(string(api.TextHTML), string(api.AppActivityJSON), string(api.AppActivityLDJSON)) if accept == string(api.AppActivityJSON) || accept == string(api.AppActivityLDJSON) { - m.returnAPRepresentation(ctx, c, username, accept) + m.returnAPProfile(ctx, c, username, accept) return } @@ -82,8 +83,7 @@ func (m *Module) profileTemplateHandler(c *gin.Context) { // with or without media statusResp, errWithCode := m.processor.AccountStatusesGet(ctx, authed, account.ID, 10, true, true, "", "", false, false, true) if errWithCode != nil { - l.Debugf("error getting statuses from processor: %s", errWithCode.Error()) - c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()}) + api.ErrorHandler(c, errWithCode, instanceGet) return } @@ -114,7 +114,7 @@ func (m *Module) profileTemplateHandler(c *gin.Context) { }) } -func (m *Module) returnAPRepresentation(ctx context.Context, c *gin.Context, username string, accept string) { +func (m *Module) returnAPProfile(ctx context.Context, c *gin.Context, username string, accept string) { verifier, signed := c.Get(string(ap.ContextRequestingPublicKeyVerifier)) if signed { ctx = context.WithValue(ctx, ap.ContextRequestingPublicKeyVerifier, verifier) @@ -125,17 +125,16 @@ func (m *Module) returnAPRepresentation(ctx context.Context, c *gin.Context, use ctx = context.WithValue(ctx, ap.ContextRequestingPublicKeySignature, signature) } - user, errWithCode := m.processor.GetFediUser(ctx, username, c.Request.URL) // GetFediUser handles auth as well + user, errWithCode := m.processor.GetFediUser(ctx, username, c.Request.URL) if errWithCode != nil { - logrus.Infof(errWithCode.Error()) - c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()}) + api.ErrorHandler(c, errWithCode, m.processor.InstanceGet) return } b, mErr := json.Marshal(user) if mErr != nil { err := fmt.Errorf("could not marshal json: %s", mErr) - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + api.ErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGet) return } diff --git a/internal/web/thread.go b/internal/web/thread.go index 3a9839281..5e953ee06 100644 --- a/internal/web/thread.go +++ b/internal/web/thread.go @@ -19,65 +19,88 @@ package web import ( + "context" + "encoding/json" + "errors" + "fmt" "net/http" "strings" - "github.com/sirupsen/logrus" - "github.com/gin-gonic/gin" + "github.com/superseriousbusiness/gotosocial/internal/ap" + "github.com/superseriousbusiness/gotosocial/internal/api" + apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/oauth" ) -func (m *Module) threadTemplateHandler(c *gin.Context) { - l := logrus.WithField("func", "threadTemplateGET") - l.Trace("rendering thread template") - +func (m *Module) threadGETHandler(c *gin.Context) { ctx := c.Request.Context() + authed, err := oauth.Authed(c, false, false, false, false) + if err != nil { + api.ErrorHandler(c, gtserror.NewErrorUnauthorized(err, err.Error()), m.processor.InstanceGet) + return + } + // usernames on our instance will always be lowercase username := strings.ToLower(c.Param(usernameKey)) if username == "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "no account username specified"}) + err := errors.New("no account username specified") + api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet) return } // status ids will always be uppercase statusID := strings.ToUpper(c.Param(statusIDKey)) if statusID == "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "no status id specified"}) + err := errors.New("no status id specified") + api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet) return } - authed, err := oauth.Authed(c, false, false, false, false) + host := config.GetHost() + instance, err := m.processor.InstanceGet(ctx, host) if err != nil { - l.Errorf("error authing status GET request: %s", err) - c.JSON(http.StatusBadRequest, gin.H{"error": "status not found"}) + api.ErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGet) return } - host := config.GetHost() - instance, err := m.processor.InstanceGet(ctx, host) - if err != nil { - l.Debugf("error getting instance from processor: %s", err) - c.JSON(http.StatusInternalServerError, gin.H{"error": "internal server error"}) + instanceGet := func(ctx context.Context, domain string) (*apimodel.Instance, gtserror.WithCode) { + return instance, nil + } + + // do this check to make sure the status is actually from a local account, + // we shouldn't render threads from statuses that don't belong to us! + if _, errWithCode := m.processor.AccountGetLocalByUsername(ctx, authed, username); errWithCode != nil { + api.ErrorHandler(c, errWithCode, instanceGet) return } - status, err := m.processor.StatusGet(ctx, authed, statusID) - if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "status not found"}) + status, errWithCode := m.processor.StatusGet(ctx, authed, statusID) + if errWithCode != nil { + api.ErrorHandler(c, errWithCode, instanceGet) return } if !strings.EqualFold(username, status.Account.Username) { - c.JSON(http.StatusBadRequest, gin.H{"error": "status not found"}) + err := gtserror.NewErrorNotFound(errors.New("path username not equal to status author username")) + api.ErrorHandler(c, gtserror.NewErrorNotFound(err), instanceGet) return } - context, err := m.processor.StatusGetContext(ctx, authed, statusID) - if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "status not found"}) + // if we're getting an AP request on this endpoint we + // should render the status's AP representation instead + accept := c.NegotiateFormat(string(api.TextHTML), string(api.AppActivityJSON), string(api.AppActivityLDJSON)) + if accept == string(api.AppActivityJSON) || accept == string(api.AppActivityLDJSON) { + m.returnAPStatus(ctx, c, username, statusID, accept) + return + } + + context, errWithCode := m.processor.StatusGetContext(ctx, authed, statusID) + if errWithCode != nil { + api.ErrorHandler(c, errWithCode, instanceGet) return } @@ -88,3 +111,30 @@ func (m *Module) threadTemplateHandler(c *gin.Context) { "stylesheets": []string{"/assets/Fork-Awesome/css/fork-awesome.min.css", "/assets/status.css"}, }) } + +func (m *Module) returnAPStatus(ctx context.Context, c *gin.Context, username string, statusID string, accept string) { + verifier, signed := c.Get(string(ap.ContextRequestingPublicKeyVerifier)) + if signed { + ctx = context.WithValue(ctx, ap.ContextRequestingPublicKeyVerifier, verifier) + } + + signature, signed := c.Get(string(ap.ContextRequestingPublicKeySignature)) + if signed { + ctx = context.WithValue(ctx, ap.ContextRequestingPublicKeySignature, signature) + } + + status, errWithCode := m.processor.GetFediStatus(ctx, username, statusID, c.Request.URL) + if errWithCode != nil { + api.ErrorHandler(c, errWithCode, m.processor.InstanceGet) + return + } + + b, mErr := json.Marshal(status) + if mErr != nil { + err := fmt.Errorf("could not marshal json: %s", mErr) + api.ErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGet) + return + } + + c.Data(http.StatusOK, accept, b) +} |