[MM-50219] OAuth 2.0 (#22310)

* tools updates

* Revert "tools updates"

This reverts commit 6293297b55803c5a263e200ebd80192899666ae9.

* oauth fix

* rename migration

* migrations-extract

* translations

* unit test

* lint

---------

Co-authored-by: Benjamin Cooke <benjamincooke@Benjamins-MacBook-Pro.local>
Co-authored-by: Benjamin Cooke <benjamincooke@Benjamins-MBP.ht.home>
Co-authored-by: Benjamin Cooke <benjamincooke@Benjamins-MBP.fritz.box>
Co-authored-by: Benjamin Cooke <benjamincooke@Benjamins-MacBook-Pro.fritz.box>

app/oauth.go

@@ -501,6 +501,10 @@ func (a *App) DeauthorizeOAuthAppForUser(userID, appID string) *model.AppError {
 		}
 	}
 
+	if err := a.Srv().Store().OAuth().RemoveAuthDataByClientId(appID, userID); err != nil {
+		return model.NewAppError("DeauthorizeOAuthAppForUser", "app.oauth.remove_auth_data_by_client_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
+	}
+
 	// Deauthorize the app
 	if err := a.Srv().Store().Preference().Delete(userID, model.PreferenceCategoryAuthorizedOAuthApp, appID); err != nil {
 		return model.NewAppError("DeauthorizeOAuthAppForUser", "app.preference.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err)

app/oauth_test.go

@@ -7,9 +7,11 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -19,6 +21,7 @@ import (
 	"github.com/mattermost/mattermost-server/v6/einterfaces/mocks"
 	"github.com/mattermost/mattermost-server/v6/model"
 	"github.com/mattermost/mattermost-server/v6/plugin/plugintest/mock"
+	"github.com/mattermost/mattermost-server/v6/store"
 )
 
 func TestGetOAuthAccessTokenForImplicitFlow(t *testing.T) {
@@ -588,3 +591,45 @@ func TestGetAuthorizationCode(t *testing.T) {
 		}
 	})
 }
+
+func TestDeauthorizeOAuthApp(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
+
+	oapp := &model.OAuthApp{
+		Name:         "fakeoauthapp" + model.NewRandomString(10),
+		CreatorId:    th.BasicUser2.Id,
+		Homepage:     "https://nowhere.com",
+		Description:  "test",
+		CallbackUrls: []string{"https://nowhere.com"},
+	}
+
+	oapp, err := th.App.CreateOAuthApp(oapp)
+	require.Nil(t, err)
+
+	authRequest := &model.AuthorizeRequest{
+		ResponseType: model.ImplicitResponseType,
+		ClientId:     oapp.Id,
+		RedirectURI:  oapp.CallbackUrls[0],
+		Scope:        "",
+		State:        "123",
+	}
+
+	redirectUrl, err := th.App.GetOAuthCodeRedirect(th.BasicUser.Id, authRequest)
+	assert.Nil(t, err)
+
+	dErr := th.App.DeauthorizeOAuthAppForUser(th.BasicUser.Id, oapp.Id)
+	assert.Nil(t, dErr)
+
+	uri, uErr := url.Parse(redirectUrl)
+	require.NoError(t, uErr)
+
+	queryParams := uri.Query()
+	code := queryParams.Get("code")
+
+	data, nErr := th.App.Srv().Store().OAuth().GetAuthData(code)
+	require.Equal(t, store.NewErrNotFound("AuthData", fmt.Sprintf("code=%s", code)), nErr)
+	assert.Nil(t, data)
+}

db/migrations/migrations.list

@@ -208,6 +208,8 @@ db/migrations/mysql/000103_add_sentat_to_notifyadmin.down.sql
 db/migrations/mysql/000103_add_sentat_to_notifyadmin.up.sql
 db/migrations/mysql/000104_upgrade_notifyadmin.down.sql
 db/migrations/mysql/000104_upgrade_notifyadmin.up.sql
+db/migrations/mysql/000105_remove_tokens.down.sql
+db/migrations/mysql/000105_remove_tokens.up.sql
 db/migrations/postgres/000001_create_teams.down.sql
 db/migrations/postgres/000001_create_teams.up.sql
 db/migrations/postgres/000002_create_team_members.down.sql
@@ -416,3 +418,5 @@ db/migrations/postgres/000103_add_sentat_to_notifyadmin.down.sql
 db/migrations/postgres/000103_add_sentat_to_notifyadmin.up.sql
 db/migrations/postgres/000104_upgrade_notifyadmin.down.sql
 db/migrations/postgres/000104_upgrade_notifyadmin.up.sql
+db/migrations/postgres/000105_remove_tokens.down.sql
+db/migrations/postgres/000105_remove_tokens.up.sql

db/migrations/mysql/000105_remove_tokens.down.sql

@@ -0,0 +1 @@
+-- Skipping it because the forward migrations are destructive

db/migrations/mysql/000105_remove_tokens.up.sql

@@ -0,0 +1,4 @@
+DELETE o, s from OAuthAccessData o 
+LEFT JOIN Preferences p ON o.clientid = p.name AND o.userid = p.userid AND p.category = 'oauth_app'
+INNER JOIN Sessions s ON o.token = s.token
+WHERE p.name IS NULL;

db/migrations/postgres/000105_remove_tokens.down.sql

@@ -0,0 +1 @@
+-- Skipping it because the forward migrations are destructive

db/migrations/postgres/000105_remove_tokens.up.sql

@@ -0,0 +1,13 @@
+DO $$
+BEGIN
+WITH oauthDelete AS (
+	DELETE FROM oauthaccessdata o
+	WHERE NOT EXISTS (
+		SELECT p.* FROM preferences p
+		WHERE o.clientid = p.name AND o.userid = p.userid AND p.category = 'oauth_app' 
+		and p.name IS NULL
+	)
+	RETURNING o.token
+)
+DELETE FROM sessions s WHERE s.token in (select oauthDelete.token from oauthDelete);
+END $$;

i18n/en.json

@@ -5951,6 +5951,10 @@
     "id": "app.oauth.remove_access_data.app_error",
     "translation": "Unable to remove the access token."
   },
+  {
+    "id": "app.oauth.remove_auth_data_by_client_id.app_error",
+    "translation": "Unable to remove oauth data."
+  },
   {
     "id": "app.oauth.save_app.existing.app_error",
     "translation": "Must call update for existing app."

store/opentracinglayer/opentracinglayer.go

@@ -5555,6 +5555,24 @@ func (s *OpenTracingLayerOAuthStore) RemoveAuthData(code string) error {
 	return err
 }
 
+func (s *OpenTracingLayerOAuthStore) RemoveAuthDataByClientId(clientId string, userId string) error {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "OAuthStore.RemoveAuthDataByClientId")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	err := s.OAuthStore.RemoveAuthDataByClientId(clientId, userId)
+	if err != nil {
+		span.LogFields(spanlog.Error(err))
+		ext.Error.Set(span, true)
+	}
+
+	return err
+}
+
 func (s *OpenTracingLayerOAuthStore) SaveAccessData(accessData *model.AccessData) (*model.AccessData, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "OAuthStore.SaveAccessData")

store/retrylayer/retrylayer.go

@@ -6298,6 +6298,27 @@ func (s *RetryLayerOAuthStore) RemoveAuthData(code string) error {
 
 }
 
+func (s *RetryLayerOAuthStore) RemoveAuthDataByClientId(clientId string, userId string) error {
+
+	tries := 0
+	for {
+		err := s.OAuthStore.RemoveAuthDataByClientId(clientId, userId)
+		if err == nil {
+			return nil
+		}
+		if !isRepeatableError(err) {
+			return err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return err
+		}
+		timepkg.Sleep(100 * timepkg.Millisecond)
+	}
+
+}
+
 func (s *RetryLayerOAuthStore) SaveAccessData(accessData *model.AccessData) (*model.AccessData, error) {
 
 	tries := 0

store/sqlstore/oauth_store.go

@@ -261,6 +261,14 @@ func (as SqlOAuthStore) RemoveAuthData(code string) error {
 	return nil
 }
 
+func (as SqlOAuthStore) RemoveAuthDataByClientId(clientId string, userId string) error {
+	_, err := as.GetMasterX().Exec("DELETE FROM OAuthAuthData WHERE ClientId = ? and UserId = ?", clientId, userId)
+	if err != nil {
+		return errors.Wrapf(err, "failed to delete AuthData with clientId=%s and userId=%s", clientId, userId)
+	}
+	return nil
+}
+
 func (as SqlOAuthStore) PermanentDeleteAuthDataByUser(userId string) error {
 	_, err := as.GetMasterX().Exec("DELETE FROM OAuthAccessData WHERE UserId = ?", userId)
 	if err != nil {

store/store.go

@@ -565,6 +565,7 @@ type OAuthStore interface {
 	SaveAuthData(authData *model.AuthData) (*model.AuthData, error)
 	GetAuthData(code string) (*model.AuthData, error)
 	RemoveAuthData(code string) error
+	RemoveAuthDataByClientId(clientId string, userId string) error
 	PermanentDeleteAuthDataByUser(userID string) error
 	SaveAccessData(accessData *model.AccessData) (*model.AccessData, error)
 	UpdateAccessData(accessData *model.AccessData) (*model.AccessData, error)

store/storetest/mocks/OAuthStore.go

@@ -291,6 +291,20 @@ func (_m *OAuthStore) RemoveAuthData(code string) error {
 	return r0
 }
 
+// RemoveAuthDataByClientId provides a mock function with given fields: clientId, userId
+func (_m *OAuthStore) RemoveAuthDataByClientId(clientId string, userId string) error {
+	ret := _m.Called(clientId, userId)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string, string) error); ok {
+		r0 = rf(clientId, userId)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // SaveAccessData provides a mock function with given fields: accessData
 func (_m *OAuthStore) SaveAccessData(accessData *model.AccessData) (*model.AccessData, error) {
 	ret := _m.Called(accessData)

store/timerlayer/timerlayer.go

@@ -5036,6 +5036,22 @@ func (s *TimerLayerOAuthStore) RemoveAuthData(code string) error {
 	return err
 }
 
+func (s *TimerLayerOAuthStore) RemoveAuthDataByClientId(clientId string, userId string) error {
+	start := time.Now()
+
+	err := s.OAuthStore.RemoveAuthDataByClientId(clientId, userId)
+
+	elapsed := float64(time.Since(start)) / float64(time.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("OAuthStore.RemoveAuthDataByClientId", success, elapsed)
+	}
+	return err
+}
+
 func (s *TimerLayerOAuthStore) SaveAccessData(accessData *model.AccessData) (*model.AccessData, error) {
 	start := time.Now()