From ce22e03f9deaf424aa21c5cc0ef85e58992a7882 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Sat, 27 Nov 2021 14:53:34 +0100 Subject: [PATCH] Require confirmed email when checking oauth token (#332) * move token checker to security package * update tests with new security package * add oauth token checking to security package * check if user email confirmed when parsing token --- internal/api/client/auth/auth.go | 2 - internal/api/s2s/user/user_test.go | 5 +- internal/api/s2s/webfinger/webfinger_test.go | 5 +- internal/api/security/security.go | 6 +- .../middleware.go => security/tokencheck.go} | 62 ++++++++++++------- internal/cliactions/server/server.go | 2 +- internal/cliactions/testrig/testrig.go | 2 +- internal/oauth/util.go | 3 + 8 files changed, 57 insertions(+), 30 deletions(-) rename internal/api/{client/auth/middleware.go => security/tokencheck.go} (58%) diff --git a/internal/api/client/auth/auth.go b/internal/api/client/auth/auth.go index 4b8b7101e..7d9a0caf5 100644 --- a/internal/api/client/auth/auth.go +++ b/internal/api/client/auth/auth.go @@ -81,7 +81,5 @@ func (m *Module) Route(s router.Router) error { s.AttachHandler(http.MethodPost, OauthAuthorizePath, m.AuthorizePOSTHandler) s.AttachHandler(http.MethodGet, CallbackPath, m.CallbackGETHandler) - - s.AttachMiddleware(m.OauthTokenMiddleware) return nil } diff --git a/internal/api/s2s/user/user_test.go b/internal/api/s2s/user/user_test.go index 3d4697b36..da305488f 100644 --- a/internal/api/s2s/user/user_test.go +++ b/internal/api/s2s/user/user_test.go @@ -28,6 +28,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/email" "github.com/superseriousbusiness/gotosocial/internal/federation" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/oauth" "github.com/superseriousbusiness/gotosocial/internal/processing" "github.com/superseriousbusiness/gotosocial/internal/typeutils" "github.com/superseriousbusiness/gotosocial/testrig" @@ -43,6 +44,7 @@ type UserStandardTestSuite struct { emailSender email.Sender processor processing.Processor storage *kv.KVStore + oauthServer oauth.Server securityModule *security.Module // standard suite models @@ -80,7 +82,8 @@ func (suite *UserStandardTestSuite) SetupTest() { suite.emailSender = testrig.NewEmailSender("../../../../web/template/", nil) suite.processor = testrig.NewTestProcessor(suite.db, suite.storage, suite.federator, suite.emailSender) suite.userModule = user.New(suite.config, suite.processor).(*user.Module) - suite.securityModule = security.New(suite.config, suite.db).(*security.Module) + suite.oauthServer = testrig.NewTestOauthServer(suite.db) + suite.securityModule = security.New(suite.config, suite.db, suite.oauthServer).(*security.Module) testrig.StandardDBSetup(suite.db, suite.testAccounts) testrig.StandardStorageSetup(suite.storage, "../../../../testrig/media") } diff --git a/internal/api/s2s/webfinger/webfinger_test.go b/internal/api/s2s/webfinger/webfinger_test.go index 26a47c861..679dbcd5d 100644 --- a/internal/api/s2s/webfinger/webfinger_test.go +++ b/internal/api/s2s/webfinger/webfinger_test.go @@ -33,6 +33,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/email" "github.com/superseriousbusiness/gotosocial/internal/federation" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/oauth" "github.com/superseriousbusiness/gotosocial/internal/processing" "github.com/superseriousbusiness/gotosocial/internal/typeutils" "github.com/superseriousbusiness/gotosocial/testrig" @@ -48,6 +49,7 @@ type WebfingerStandardTestSuite struct { emailSender email.Sender processor processing.Processor storage *kv.KVStore + oauthServer oauth.Server securityModule *security.Module // standard suite models @@ -83,7 +85,8 @@ func (suite *WebfingerStandardTestSuite) SetupTest() { suite.emailSender = testrig.NewEmailSender("../../../../web/template/", nil) suite.processor = testrig.NewTestProcessor(suite.db, suite.storage, suite.federator, suite.emailSender) suite.webfingerModule = webfinger.New(suite.config, suite.processor).(*webfinger.Module) - suite.securityModule = security.New(suite.config, suite.db).(*security.Module) + suite.oauthServer = testrig.NewTestOauthServer(suite.db) + suite.securityModule = security.New(suite.config, suite.db, suite.oauthServer).(*security.Module) testrig.StandardDBSetup(suite.db, suite.testAccounts) testrig.StandardStorageSetup(suite.storage, "../../../../testrig/media") } diff --git a/internal/api/security/security.go b/internal/api/security/security.go index 5ed677467..0379b2d53 100644 --- a/internal/api/security/security.go +++ b/internal/api/security/security.go @@ -24,6 +24,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/api" "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/oauth" "github.com/superseriousbusiness/gotosocial/internal/router" ) @@ -33,13 +34,15 @@ const robotsPath = "/robots.txt" type Module struct { config *config.Config db db.DB + server oauth.Server } // New returns a new security module -func New(config *config.Config, db db.DB) api.ClientModule { +func New(config *config.Config, db db.DB, server oauth.Server) api.ClientModule { return &Module{ config: config, db: db, + server: server, } } @@ -49,6 +52,7 @@ func (m *Module) Route(s router.Router) error { s.AttachMiddleware(m.FlocBlock) s.AttachMiddleware(m.ExtraHeaders) s.AttachMiddleware(m.UserAgentBlock) + s.AttachMiddleware(m.TokenCheck) s.AttachHandler(http.MethodGet, robotsPath, m.RobotsGETHandler) return nil } diff --git a/internal/api/client/auth/middleware.go b/internal/api/security/tokencheck.go similarity index 58% rename from internal/api/client/auth/middleware.go rename to internal/api/security/tokencheck.go index 1b84458b0..5a47e3833 100644 --- a/internal/api/client/auth/middleware.go +++ b/internal/api/security/tokencheck.go @@ -16,7 +16,7 @@ along with this program. If not, see . */ -package auth +package security import ( "github.com/gin-gonic/gin" @@ -26,55 +26,71 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/oauth" ) -// OauthTokenMiddleware checks if the client has presented a valid oauth Bearer token. +// TokenCheck checks if the client has presented a valid oauth Bearer token. // If so, it will check the User that the token belongs to, and set that in the context of // the request. Then, it will look up the account for that user, and set that in the request too. // If user or account can't be found, then the handler won't *fail*, in case the server wants to allow // public requests that don't have a Bearer token set (eg., for public instance information and so on). -func (m *Module) OauthTokenMiddleware(c *gin.Context) { +func (m *Module) TokenCheck(c *gin.Context) { l := logrus.WithField("func", "OauthTokenMiddleware") - l.Trace("entering OauthTokenMiddleware") + ctx := c.Request.Context() + defer c.Next() + + if c.Request.Header.Get("Authorization") == "" { + // no token set in the header, we can just bail + return + } ti, err := m.server.ValidationBearerToken(c.Copy().Request) if err != nil { - l.Tracef("could not validate token: %s", err) + l.Infof("token was passed in Authorization header but we could not validate it: %s", err) return } - l.Trace("continuing with unauthenticated request") c.Set(oauth.SessionAuthorizedToken, ti) - l.Tracef("set gin context %s to %+v", oauth.SessionAuthorizedToken, ti) // check for user-level token - if uid := ti.GetUserID(); uid != "" { - l.Tracef("authenticated user %s with bearer token, scope is %s", uid, ti.GetScope()) + if userID := ti.GetUserID(); userID != "" { + l.Tracef("authenticated user %s with bearer token, scope is %s", userID, ti.GetScope()) - // fetch user's and account for this user id + // fetch user for this token user := >smodel.User{} - if err := m.db.GetByID(c.Request.Context(), uid, user); err != nil || user == nil { - l.Warnf("no user found for validated uid %s", uid) + if err := m.db.GetByID(ctx, userID, user); err != nil { + if err != db.ErrNoEntries { + l.Errorf("database error looking for user with id %s: %s", userID, err) + return + } + l.Warnf("no user found for userID %s", userID) return } c.Set(oauth.SessionAuthorizedUser, user) - l.Tracef("set gin context %s to %+v", oauth.SessionAuthorizedUser, user) - acct, err := m.db.GetAccountByID(c.Request.Context(), user.AccountID) - if err != nil || acct == nil { - l.Warnf("no account found for validated user %s", uid) + // fetch account for this token + acct, err := m.db.GetAccountByID(ctx, user.AccountID) + if err != nil { + if err != db.ErrNoEntries { + l.Errorf("database error looking for account with id %s: %s", user.AccountID, err) + return + } + l.Warnf("no account found for userID %s", userID) return } c.Set(oauth.SessionAuthorizedAccount, acct) - l.Tracef("set gin context %s to %+v", oauth.SessionAuthorizedAccount, acct) } // check for application token - if cid := ti.GetClientID(); cid != "" { - l.Tracef("authenticated client %s with bearer token, scope is %s", cid, ti.GetScope()) + if clientID := ti.GetClientID(); clientID != "" { + l.Tracef("authenticated client %s with bearer token, scope is %s", clientID, ti.GetScope()) + + // fetch app for this token app := >smodel.Application{} - if err := m.db.GetWhere(c.Request.Context(), []db.Where{{Key: "client_id", Value: cid}}, app); err != nil { - l.Tracef("no app found for client %s", cid) + if err := m.db.GetWhere(ctx, []db.Where{{Key: "client_id", Value: clientID}}, app); err != nil { + if err != db.ErrNoEntries { + l.Errorf("database error looking for application with clientID %s: %s", clientID, err) + return + } + l.Warnf("no app found for client %s", clientID) + return } c.Set(oauth.SessionAuthorizedApplication, app) - l.Tracef("set gin context %s to %+v", oauth.SessionAuthorizedApplication, app) } - c.Next() } diff --git a/internal/cliactions/server/server.go b/internal/cliactions/server/server.go index b77efe07e..3e6acfe84 100644 --- a/internal/cliactions/server/server.go +++ b/internal/cliactions/server/server.go @@ -137,7 +137,7 @@ var Start cliactions.GTSAction = func(ctx context.Context, c *config.Config) err fileServerModule := fileserver.New(c, processor) adminModule := admin.New(c, processor) statusModule := status.New(c, processor) - securityModule := security.New(c, dbService) + securityModule := security.New(c, dbService, oauthServer) streamingModule := streaming.New(c, processor) favouritesModule := favourites.New(c, processor) blocksModule := blocks.New(c, processor) diff --git a/internal/cliactions/testrig/testrig.go b/internal/cliactions/testrig/testrig.go index d13f9e17b..5e55c3d03 100644 --- a/internal/cliactions/testrig/testrig.go +++ b/internal/cliactions/testrig/testrig.go @@ -97,7 +97,7 @@ var Start cliactions.GTSAction = func(ctx context.Context, _ *config.Config) err fileServerModule := fileserver.New(c, processor) adminModule := admin.New(c, processor) statusModule := status.New(c, processor) - securityModule := security.New(c, dbService) + securityModule := security.New(c, dbService, oauthServer) streamingModule := streaming.New(c, processor) favouritesModule := favourites.New(c, processor) blocksModule := blocks.New(c, processor) diff --git a/internal/oauth/util.go b/internal/oauth/util.go index 2520fc784..540045f80 100644 --- a/internal/oauth/util.go +++ b/internal/oauth/util.go @@ -85,6 +85,9 @@ func Authed(c *gin.Context, requireToken bool, requireApp bool, requireUser bool if a.User.Disabled || !a.User.Approved { return nil, errors.New("user disabled or not approved") } + if a.User.Email == "" { + return nil, errors.New("user has no confirmed email address") + } } if requireAccount {