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

@@ -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
 }

internal/api/s2s/user/user_test.go

@@ -28,6 +28,7 @@
 	"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")
 }

internal/api/s2s/webfinger/webfinger_test.go

@@ -33,6 +33,7 @@
 	"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")
 }

internal/api/security/security.go

@@ -24,6 +24,7 @@
 	"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 @@
 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
 }

internal/api/security/tokencheck.go

@@ -16,7 +16,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-package auth
+package security
 
 import (
 	"github.com/gin-gonic/gin"
@@ -26,55 +26,71 @@
 	"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 := &gtsmodel.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 := &gtsmodel.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()
 }

internal/cliactions/server/server.go

@@ -137,7 +137,7 @@
 	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)

internal/cliactions/testrig/testrig.go

@@ -97,7 +97,7 @@
 	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)

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 {