[MM-40735] Add MattermostAppID to OAuth app (#19214)

app/oauth.go

@@ -244,7 +244,7 @@ func (a *App) GetOAuthAccessTokenForImplicitFlow(userID string, authRequest *mod
 		return nil, err
 	}
 
-	session, err := a.newSession(oauthApp.Name, user)
+	session, err := a.newSession(oauthApp, user)
 	if err != nil {
 		return nil, err
 	}
@@ -306,7 +306,7 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectURI, c
 		if accessData != nil {
 			if accessData.IsExpired() {
 				var access *model.AccessResponse
-				access, err := a.newSessionUpdateToken(oauthApp.Name, accessData, user)
+				access, err := a.newSessionUpdateToken(oauthApp, accessData, user)
 				if err != nil {
 					return nil, err
 				}
@@ -323,7 +323,7 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectURI, c
 		} else {
 			var session *model.Session
 			// Create a new session and return new access token
-			session, err := a.newSession(oauthApp.Name, user)
+			session, err := a.newSession(oauthApp, user)
 			if err != nil {
 				return nil, err
 			}
@@ -357,7 +357,7 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectURI, c
 			return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal_user.app_error", nil, "", http.StatusNotFound)
 		}
 
-		access, err := a.newSessionUpdateToken(oauthApp.Name, accessData, user)
+		access, err := a.newSessionUpdateToken(oauthApp, accessData, user)
 		if err != nil {
 			return nil, err
 		}
@@ -367,12 +367,14 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectURI, c
 	return accessRsp, nil
 }
 
-func (a *App) newSession(appName string, user *model.User) (*model.Session, *model.AppError) {
+func (a *App) newSession(app *model.OAuthApp, user *model.User) (*model.Session, *model.AppError) {
 	// Set new token an session
 	session := &model.Session{UserId: user.Id, Roles: user.Roles, IsOAuth: true}
 	session.GenerateCSRF()
 	a.ch.srv.userService.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthSSOInDays)
-	session.AddProp(model.SessionPropPlatform, appName)
+	session.AddProp(model.SessionPropPlatform, app.Name)
+	session.AddProp(model.SessionPropOAuthAppID, app.Id)
+	session.AddProp(model.SessionPropMattermostAppID, app.MattermostAppID)
 	session.AddProp(model.SessionPropOs, "OAuth2")
 	session.AddProp(model.SessionPropBrowser, "OAuth2")
 
@@ -386,13 +388,13 @@ func (a *App) newSession(appName string, user *model.User) (*model.Session, *mod
 	return session, nil
 }
 
-func (a *App) newSessionUpdateToken(appName string, accessData *model.AccessData, user *model.User) (*model.AccessResponse, *model.AppError) {
+func (a *App) newSessionUpdateToken(app *model.OAuthApp, accessData *model.AccessData, user *model.User) (*model.AccessResponse, *model.AppError) {
 	// Remove the previous session
 	if err := a.Srv().Store.Session().Remove(accessData.Token); err != nil {
 		mlog.Warn("error removing access data token from session", mlog.Err(err))
 	}
 
-	session, err := a.newSession(appName, user)
+	session, err := a.newSession(app, user)
 	if err != nil {
 		return nil, err
 	}

db/migrations/mysql/000078_create_oauth_mattermost_app_id.down.sql

@@ -0,0 +1,14 @@
+SET @preparedStatement = (SELECT IF(
+    (
+        SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS
+        WHERE table_name = 'OAuthApps'
+        AND table_schema = DATABASE()
+        AND column_name = 'MattermostAppID'
+    ) > 0,
+    'ALTER TABLE OAuthApps DROP COLUMN MattermostAppID;',
+    'SELECT 1'
+));
+
+PREPARE alterIfExists FROM @preparedStatement;
+EXECUTE alterIfExists;
+DEALLOCATE PREPARE alterIfExists;

db/migrations/mysql/000078_create_oauth_mattermost_app_id.up.sql

@@ -0,0 +1,14 @@
+SET @preparedStatement = (SELECT IF(
+    (
+        SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS
+        WHERE table_name = 'OAuthApps'
+        AND table_schema = DATABASE()
+        AND column_name = 'MattermostAppID'
+    ) > 0,
+	'SELECT 1',
+    'ALTER TABLE OAuthApps ADD COLUMN MattermostAppID varchar(32);'
+));
+
+PREPARE alterIfExists FROM @preparedStatement;
+EXECUTE alterIfExists;
+DEALLOCATE PREPARE alterIfExists;

db/migrations/postgres/000078_create_oauth_mattermost_app_id.down.sql

@@ -0,0 +1 @@
+ALTER TABLE OAuthApps DROP COLUMN IF EXISTS MattermostAppID;

db/migrations/postgres/000078_create_oauth_mattermost_app_id.up.sql

@@ -0,0 +1 @@
+ALTER TABLE OAuthApps ADD COLUMN IF NOT EXISTS MattermostAppID varchar(32);

i18n/en.json

@@ -8671,6 +8671,10 @@
     "id": "model.oauth.is_valid.icon_url.app_error",
     "translation": "Icon URL must be a valid URL and start with http:// or https://."
   },
+  {
+    "id": "model.oauth.is_valid.mattermost_app_id.app_error",
+    "translation": "The maximum length of MattermostAppID is 32 characters."
+  },
   {
     "id": "model.oauth.is_valid.name.app_error",
     "translation": "Invalid name."

model/oauth.go

@@ -18,17 +18,18 @@ const (
 )
 
 type OAuthApp struct {
-	Id           string      `json:"id"`
+	Id              string      `json:"id"`
-	CreatorId    string      `json:"creator_id"`
+	CreatorId       string      `json:"creator_id"`
-	CreateAt     int64       `json:"create_at"`
+	CreateAt        int64       `json:"create_at"`
-	UpdateAt     int64       `json:"update_at"`
+	UpdateAt        int64       `json:"update_at"`
-	ClientSecret string      `json:"client_secret"`
+	ClientSecret    string      `json:"client_secret"`
-	Name         string      `json:"name"`
+	Name            string      `json:"name"`
-	Description  string      `json:"description"`
+	Description     string      `json:"description"`
-	IconURL      string      `json:"icon_url"`
+	IconURL         string      `json:"icon_url"`
-	CallbackUrls StringArray `json:"callback_urls"`
+	CallbackUrls    StringArray `json:"callback_urls"`
-	Homepage     string      `json:"homepage"`
+	Homepage        string      `json:"homepage"`
-	IsTrusted    bool        `json:"is_trusted"`
+	IsTrusted       bool        `json:"is_trusted"`
+	MattermostAppID string      `json:"mattermost_app_id"`
 }
 
 // IsValid validates the app and returns an error if it isn't configured
@@ -83,6 +84,10 @@ func (a *OAuthApp) IsValid() *AppError {
 		}
 	}
 
+	if len(a.MattermostAppID) > 32 {
+		return NewAppError("OAuthApp.IsValid", "model.oauth.is_valid.mattermost_app_id.app_error", nil, "app_id="+a.Id, http.StatusBadRequest)
+	}
+
 	return nil
 }
 

model/oauth_test.go

@@ -59,6 +59,9 @@ func TestOAuthAppIsValid(t *testing.T) {
 	app.CallbackUrls = []string{"https://nowhere.com"}
 	require.NotNil(t, app.IsValid())
 
+	app.MattermostAppID = "Some app ID"
+	require.NotNil(t, app.IsValid())
+
 	app.Homepage = "https://nowhere.com"
 	require.Nil(t, app.IsValid())
 

model/session.go

@@ -24,6 +24,8 @@ const (
 	SessionPropUserAccessTokenId  = "user_access_token_id"
 	SessionPropIsBot              = "is_bot"
 	SessionPropIsBotValue         = "true"
+	SessionPropOAuthAppID         = "oauth_app_id"
+	SessionPropMattermostAppID    = "mattermost_app_id"
 	SessionTypeUserAccessToken    = "UserAccessToken"
 	SessionTypeCloudKey           = "CloudKey"
 	SessionTypeRemoteclusterToken = "RemoteClusterToken"

store/sqlstore/oauth_store.go

@@ -32,9 +32,9 @@ func (as SqlOAuthStore) SaveApp(app *model.OAuthApp) (*model.OAuthApp, error) {
 	}
 
 	if _, err := as.GetMasterX().NamedExec(`INSERT INTO OAuthApps
-		(Id, CreatorId, CreateAt, UpdateAt, ClientSecret, Name, Description, IconURL, CallbackUrls, Homepage, IsTrusted)
+		(Id, CreatorId, CreateAt, UpdateAt, ClientSecret, Name, Description, IconURL, CallbackUrls, Homepage, IsTrusted, MattermostAppID)
 		VALUES
-		(:Id, :CreatorId, :CreateAt, :UpdateAt, :ClientSecret, :Name, :Description, :IconURL, :CallbackUrls, :Homepage, :IsTrusted)`, app); err != nil {
+		(:Id, :CreatorId, :CreateAt, :UpdateAt, :ClientSecret, :Name, :Description, :IconURL, :CallbackUrls, :Homepage, :IsTrusted, :MattermostAppID)`, app); err != nil {
 		return nil, errors.Wrap(err, "failed to save OAuthApp")
 	}
 	return app, nil
@@ -63,7 +63,7 @@ func (as SqlOAuthStore) UpdateApp(app *model.OAuthApp) (*model.OAuthApp, error)
 	res, err := as.GetMasterX().NamedExec(`UPDATE OAuthApps
 		SET UpdateAt=:UpdateAt, ClientSecret=:ClientSecret, Name=:Name,
 			Description=:Description, IconURL=:IconURL, CallbackUrls=:CallbackUrls,
-			Homepage=:Homepage, IsTrusted=:IsTrusted
+			Homepage=:Homepage, IsTrusted=:IsTrusted, MattermostAppID=:MattermostAppID
 		WHERE Id=:Id`, app)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to update OAuthApp with id=%s", app.Id)

store/storetest/oauth_store.go

@@ -48,9 +48,14 @@ func testOAuthStoreSaveApp(t *testing.T, ss store.Store) {
 	_, err = ss.OAuth().SaveApp(&a1)
 	require.Error(t, err, "Should have failed, app should be invalid cause it doesn' have a name set")
 
+	a1.Name = "TestApp" + model.NewId() // Valid name
+	a1.MattermostAppID = "a very, very, very, very, very, very, very long id"
+	_, err = ss.OAuth().SaveApp(&a1)
+	require.Error(t, err, "Should have failed, app should be invalid cause the MattermostAppID is to long")
+
 	// Save the app
 	a1.Id = ""
-	a1.Name = "TestApp" + model.NewId()
+	a1.MattermostAppID = "some small id" // Valid id
 	_, err = ss.OAuth().SaveApp(&a1)
 	require.NoError(t, err)
 }