[feature] Refactor tokens, allow multiple app redirect_uris (#3849)

* [feature] Refactor tokens, allow multiple app redirect_uris

* move + tweak handlers a bit

* return error for unset oauth2.ClientStore funcs

* wrap UpdateToken with cache

* panic handling

* cheeky little time optimization

* unlock on error

internal/processing/account/account_test.go

@@ -55,7 +55,6 @@ type AccountStandardTestSuite struct {
 
 	// standard suite models
 	testTokens       map[string]*gtsmodel.Token
-	testClients      map[string]*gtsmodel.Client
 	testApplications map[string]*gtsmodel.Application
 	testUsers        map[string]*gtsmodel.User
 	testAccounts     map[string]*gtsmodel.Account
@@ -76,7 +75,6 @@ func (suite *AccountStandardTestSuite) getClientMsg(timeout time.Duration) (*mes
 
 func (suite *AccountStandardTestSuite) SetupSuite() {
 	suite.testTokens = testrig.NewTestTokens()
-	suite.testClients = testrig.NewTestClients()
 	suite.testApplications = testrig.NewTestApplications()
 	suite.testUsers = testrig.NewTestUsers()
 	suite.testAccounts = testrig.NewTestAccounts()

internal/processing/account/delete.go

@@ -113,11 +113,6 @@ func (p *Processor) deleteUserAndTokensForAccount(ctx context.Context, account *
 	}
 
 	for _, t := range tokens {
-		// Delete any OAuth clients associated with this token.
-		if err := p.state.DB.DeleteByID(ctx, t.ClientID, &[]*gtsmodel.Client{}); err != nil {
-			return gtserror.Newf("db error deleting client: %w", err)
-		}
-
 		// Delete any OAuth applications associated with this token.
 		if err := p.state.DB.DeleteApplicationByClientID(ctx, t.ClientID); err != nil {
 			return gtserror.Newf("db error deleting application: %w", err)

internal/processing/admin/admin_test.go

@@ -58,7 +58,6 @@ type AdminStandardTestSuite struct {
 
 	// standard suite models
 	testTokens       map[string]*gtsmodel.Token
-	testClients      map[string]*gtsmodel.Client
 	testApplications map[string]*gtsmodel.Application
 	testUsers        map[string]*gtsmodel.User
 	testAccounts     map[string]*gtsmodel.Account
@@ -73,7 +72,6 @@ type AdminStandardTestSuite struct {
 
 func (suite *AdminStandardTestSuite) SetupSuite() {
 	suite.testTokens = testrig.NewTestTokens()
-	suite.testClients = testrig.NewTestClients()
 	suite.testApplications = testrig.NewTestApplications()
 	suite.testUsers = testrig.NewTestUsers()
 	suite.testAccounts = testrig.NewTestAccounts()
@@ -103,7 +101,7 @@ func (suite *AdminStandardTestSuite) SetupTest() {
 	suite.storage = testrig.NewInMemoryStorage()
 	suite.state.Storage = suite.storage
 	suite.mediaManager = testrig.NewTestMediaManager(&suite.state)
-	suite.oauthServer = testrig.NewTestOauthServer(suite.db)
+	suite.oauthServer = testrig.NewTestOauthServer(&suite.state)
 
 	suite.transportController = testrig.NewTestTransportController(&suite.state, testrig.NewMockHTTPClient(nil, "../../../testrig/media"))
 	suite.federator = testrig.NewTestFederator(&suite.state, suite.transportController, suite.mediaManager)

internal/processing/app.go

@@ -19,6 +19,9 @@ package processing
 
 import (
 	"context"
+	"fmt"
+	"net/url"
+	"strings"
 
 	"github.com/google/uuid"
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
@@ -26,10 +29,12 @@ import (
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/id"
+	"github.com/superseriousbusiness/gotosocial/internal/oauth"
 )
 
 func (p *Processor) AppCreate(ctx context.Context, authed *apiutil.Auth, form *apimodel.ApplicationCreateRequest) (*apimodel.Application, gtserror.WithCode) {
-	// set default 'read' for scopes if it's not set
+	// Set default 'read' for
+	// scopes if it's not set.
 	var scopes string
 	if form.Scopes == "" {
 		scopes = "read"
@@ -37,48 +42,47 @@ func (p *Processor) AppCreate(ctx context.Context, authed *apiutil.Auth, form *a
 		scopes = form.Scopes
 	}
 
-	// generate new IDs for this application and its associated client
+	// Normalize + parse requested redirect URIs.
+	form.RedirectURIs = strings.TrimSpace(form.RedirectURIs)
+	var redirectURIs []string
+	if form.RedirectURIs != "" {
+		// Redirect URIs can be just one value, or can be passed
+		// as a newline-separated list of strings. Ensure each URI
+		// is parseable + normalize it by reconstructing from *url.URL.
+		for _, redirectStr := range strings.Split(form.RedirectURIs, "\n") {
+			redirectURI, err := url.Parse(redirectStr)
+			if err != nil {
+				errText := fmt.Sprintf("error parsing redirect URI: %v", err)
+				return nil, gtserror.NewErrorBadRequest(err, errText)
+			}
+			redirectURIs = append(redirectURIs, redirectURI.String())
+		}
+	} else {
+		// No redirect URI(s) provided, just set default oob.
+		redirectURIs = append(redirectURIs, oauth.OOBURI)
+	}
+
+	// Generate random client ID.
 	clientID, err := id.NewRandomULID()
 	if err != nil {
 		return nil, gtserror.NewErrorInternalError(err)
 	}
-	clientSecret := uuid.NewString()
 
-	appID, err := id.NewRandomULID()
-	if err != nil {
-		return nil, gtserror.NewErrorInternalError(err)
-	}
-
-	// generate the application to put in the database
+	// Generate + store app
+	// to put in the database.
 	app := &gtsmodel.Application{
-		ID:           appID,
+		ID:           id.NewULID(),
 		Name:         form.ClientName,
 		Website:      form.Website,
-		RedirectURI:  form.RedirectURIs,
+		RedirectURIs: redirectURIs,
 		ClientID:     clientID,
-		ClientSecret: clientSecret,
+		ClientSecret: uuid.NewString(),
 		Scopes:       scopes,
 	}
-
-	// chuck it in the db
 	if err := p.state.DB.PutApplication(ctx, app); err != nil {
 		return nil, gtserror.NewErrorInternalError(err)
 	}
 
-	// now we need to model an oauth client from the application that the oauth library can use
-	oc := &gtsmodel.Client{
-		ID:     clientID,
-		Secret: clientSecret,
-		Domain: form.RedirectURIs,
-		// This client isn't yet associated with a specific user,  it's just an app client right now
-		UserID: "",
-	}
-
-	// chuck it in the db
-	if err := p.state.DB.PutClient(ctx, oc); err != nil {
-		return nil, gtserror.NewErrorInternalError(err)
-	}
-
 	apiApp, err := p.converter.AppToAPIAppSensitive(ctx, app)
 	if err != nil {
 		return nil, gtserror.NewErrorInternalError(err)

internal/processing/conversations/conversations_test.go

@@ -57,7 +57,6 @@ type ConversationsTestSuite struct {
 
 	// standard suite models
 	testTokens       map[string]*gtsmodel.Token
-	testClients      map[string]*gtsmodel.Client
 	testApplications map[string]*gtsmodel.Application
 	testUsers        map[string]*gtsmodel.User
 	testAccounts     map[string]*gtsmodel.Account
@@ -84,7 +83,6 @@ func (suite *ConversationsTestSuite) getClientMsg(timeout time.Duration) (*messa
 
 func (suite *ConversationsTestSuite) SetupSuite() {
 	suite.testTokens = testrig.NewTestTokens()
-	suite.testClients = testrig.NewTestClients()
 	suite.testApplications = testrig.NewTestApplications()
 	suite.testUsers = testrig.NewTestUsers()
 	suite.testAccounts = testrig.NewTestAccounts()

internal/processing/media/media_test.go

@@ -45,7 +45,6 @@ type MediaStandardTestSuite struct {
 
 	// standard suite models
 	testTokens            map[string]*gtsmodel.Token
-	testClients           map[string]*gtsmodel.Client
 	testApplications      map[string]*gtsmodel.Application
 	testUsers             map[string]*gtsmodel.User
 	testAccounts          map[string]*gtsmodel.Account
@@ -59,7 +58,6 @@ type MediaStandardTestSuite struct {
 
 func (suite *MediaStandardTestSuite) SetupSuite() {
 	suite.testTokens = testrig.NewTestTokens()
-	suite.testClients = testrig.NewTestClients()
 	suite.testApplications = testrig.NewTestApplications()
 	suite.testUsers = testrig.NewTestUsers()
 	suite.testAccounts = testrig.NewTestAccounts()

internal/processing/processor_test.go

@@ -58,7 +58,6 @@ type ProcessingStandardTestSuite struct {
 
 	// standard suite models
 	testTokens       map[string]*gtsmodel.Token
-	testClients      map[string]*gtsmodel.Client
 	testApplications map[string]*gtsmodel.Application
 	testUsers        map[string]*gtsmodel.User
 	testAccounts     map[string]*gtsmodel.Account
@@ -77,7 +76,6 @@ type ProcessingStandardTestSuite struct {
 
 func (suite *ProcessingStandardTestSuite) SetupSuite() {
 	suite.testTokens = testrig.NewTestTokens()
-	suite.testClients = testrig.NewTestClients()
 	suite.testApplications = testrig.NewTestApplications()
 	suite.testUsers = testrig.NewTestUsers()
 	suite.testAccounts = testrig.NewTestAccounts()
@@ -124,7 +122,7 @@ func (suite *ProcessingStandardTestSuite) SetupTest() {
 	suite.transportController = testrig.NewTestTransportController(&suite.state, suite.httpClient)
 	suite.mediaManager = testrig.NewTestMediaManager(&suite.state)
 	suite.federator = testrig.NewTestFederator(&suite.state, suite.transportController, suite.mediaManager)
-	suite.oauthServer = testrig.NewTestOauthServer(suite.db)
+	suite.oauthServer = testrig.NewTestOauthServer(&suite.state)
 	suite.emailSender = testrig.NewEmailSender("../../web/template/", nil)
 
 	suite.processor = processing.NewProcessor(

internal/processing/status/status_test.go

@@ -50,7 +50,6 @@ type StatusStandardTestSuite struct {
 
 	// standard suite models
 	testTokens       map[string]*gtsmodel.Token
-	testClients      map[string]*gtsmodel.Client
 	testApplications map[string]*gtsmodel.Application
 	testUsers        map[string]*gtsmodel.User
 	testAccounts     map[string]*gtsmodel.Account
@@ -65,7 +64,6 @@ type StatusStandardTestSuite struct {
 
 func (suite *StatusStandardTestSuite) SetupSuite() {
 	suite.testTokens = testrig.NewTestTokens()
-	suite.testClients = testrig.NewTestClients()
 	suite.testApplications = testrig.NewTestApplications()
 	suite.testUsers = testrig.NewTestUsers()
 	suite.testAccounts = testrig.NewTestAccounts()

internal/processing/stream/stream_test.go

@@ -52,7 +52,7 @@ func (suite *StreamTestSuite) SetupTest() {
 	suite.db = testrig.NewTestDB(&suite.state)
 	suite.state.DB = suite.db
 	suite.state.AdminActions = admin.New(suite.state.DB, &suite.state.Workers)
-	suite.oauthServer = testrig.NewTestOauthServer(suite.db)
+	suite.oauthServer = testrig.NewTestOauthServer(&suite.state)
 	suite.streamProcessor = stream.New(&suite.state, suite.oauthServer)
 
 	testrig.StandardDBSetup(suite.db, suite.testAccounts)

internal/processing/user/user_test.go

@@ -54,7 +54,7 @@ func (suite *UserStandardTestSuite) SetupTest() {
 	suite.db = testrig.NewTestDB(&suite.state)
 	suite.state.DB = suite.db
 	suite.state.AdminActions = admin.New(suite.state.DB, &suite.state.Workers)
-	suite.oauthServer = testrig.NewTestOauthServer(suite.state.DB)
+	suite.oauthServer = testrig.NewTestOauthServer(&suite.state)
 
 	suite.sentEmails = make(map[string]string)
 	suite.emailSender = testrig.NewEmailSender("../../../web/template/", suite.sentEmails)
@@ -62,7 +62,7 @@ func (suite *UserStandardTestSuite) SetupTest() {
 	suite.testTokens = testrig.NewTestTokens()
 	suite.testUsers = testrig.NewTestUsers()
 
-	suite.user = user.New(&suite.state, typeutils.NewConverter(&suite.state), testrig.NewTestOauthServer(suite.db), suite.emailSender)
+	suite.user = user.New(&suite.state, typeutils.NewConverter(&suite.state), testrig.NewTestOauthServer(&suite.state), suite.emailSender)
 
 	testrig.StandardDBSetup(suite.db, nil)
 }

internal/processing/workers/workers_test.go

@@ -39,7 +39,6 @@ type WorkersTestSuite struct {
 
 	// standard suite models
 	testTokens       map[string]*gtsmodel.Token
-	testClients      map[string]*gtsmodel.Client
 	testApplications map[string]*gtsmodel.Application
 	testUsers        map[string]*gtsmodel.User
 	testAccounts     map[string]*gtsmodel.Account
@@ -57,7 +56,6 @@ type WorkersTestSuite struct {
 
 func (suite *WorkersTestSuite) SetupSuite() {
 	suite.testTokens = testrig.NewTestTokens()
-	suite.testClients = testrig.NewTestClients()
 	suite.testApplications = testrig.NewTestApplications()
 	suite.testUsers = testrig.NewTestUsers()
 	suite.testAccounts = testrig.NewTestAccounts()