Fixes MM-50733 (#22784)

server/channels/app/oauth.go

@@ -309,6 +309,10 @@ 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)
 		}
 
+		if user.DeleteAt != 0 {
+			return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.expired_code.app_error", nil, "", http.StatusForbidden)
+		}
+
 		accessData, nErr = a.Srv().Store().OAuth().GetPreviousAccessData(user.Id, clientId)
 		if nErr != nil {
 			return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.internal.app_error", nil, "", http.StatusBadRequest)

server/channels/app/oauth_test.go

@@ -633,3 +633,47 @@ func TestDeauthorizeOAuthApp(t *testing.T) {
 	require.Equal(t, store.NewErrNotFound("AuthData", fmt.Sprintf("code=%s", code)), nErr)
 	assert.Nil(t, data)
 }
+
+func TestDeactivatedUserOAuthApp(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)
+
+	uri, uErr := url.Parse(redirectUrl)
+	require.NoError(t, uErr)
+
+	queryParams := uri.Query()
+	code := queryParams.Get("code")
+
+	_, appErr := th.App.UpdateActive(th.Context, th.BasicUser, false)
+	require.Nil(t, appErr)
+
+	resp, accErr := th.App.GetOAuthAccessTokenForCodeFlow(oapp.Id, model.AccessTokenGrantType, oapp.CallbackUrls[0], code, oapp.ClientSecret, "")
+	assert.Nil(t, resp)
+	require.NotNil(t, accErr, "Should not get access token")
+	require.Equal(t, http.StatusBadRequest, accErr.StatusCode)
+	assert.Equal(t, "api.oauth.get_access_token.expired_code.app_error", accErr.Id)
+}

server/channels/app/user.go

@@ -933,6 +933,10 @@ func (a *App) userDeactivated(c request.CTX, userID string) *model.AppError {
 		a.disableUserBots(c, userID)
 	}
 
+	if nErr := a.Srv().Store().OAuth().RemoveAuthDataByUserId(userID); nErr != nil {
+		mlog.Warn("unable to remove auth data by user id", mlog.Err(nErr))
+	}
+
 	return nil
 }
 

server/channels/store/opentracinglayer/opentracinglayer.go

@@ -5555,6 +5555,24 @@ func (s *OpenTracingLayerOAuthStore) RemoveAuthDataByClientId(clientId string, u
 	return err
 }
 
+func (s *OpenTracingLayerOAuthStore) RemoveAuthDataByUserId(userId string) error {
+	origCtx := s.Root.Store.Context()
+	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "OAuthStore.RemoveAuthDataByUserId")
+	s.Root.Store.SetContext(newCtx)
+	defer func() {
+		s.Root.Store.SetContext(origCtx)
+	}()
+
+	defer span.Finish()
+	err := s.OAuthStore.RemoveAuthDataByUserId(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")

server/channels/store/retrylayer/retrylayer.go

@@ -6298,6 +6298,27 @@ func (s *RetryLayerOAuthStore) RemoveAuthDataByClientId(clientId string, userId
 
 }
 
+func (s *RetryLayerOAuthStore) RemoveAuthDataByUserId(userId string) error {
+
+	tries := 0
+	for {
+		err := s.OAuthStore.RemoveAuthDataByUserId(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

server/channels/store/sqlstore/oauth_store.go

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

server/channels/store/store.go

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

server/channels/store/storetest/mocks/OAuthStore.go

@@ -305,6 +305,20 @@ func (_m *OAuthStore) RemoveAuthDataByClientId(clientId string, userId string) e
 	return r0
 }
 
+// RemoveAuthDataByUserId provides a mock function with given fields: userId
+func (_m *OAuthStore) RemoveAuthDataByUserId(userId string) error {
+	ret := _m.Called(userId)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string) error); ok {
+		r0 = rf(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)

server/channels/store/timerlayer/timerlayer.go

@@ -5036,6 +5036,22 @@ func (s *TimerLayerOAuthStore) RemoveAuthDataByClientId(clientId string, userId
 	return err
 }
 
+func (s *TimerLayerOAuthStore) RemoveAuthDataByUserId(userId string) error {
+	start := time.Now()
+
+	err := s.OAuthStore.RemoveAuthDataByUserId(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.RemoveAuthDataByUserId", success, elapsed)
+	}
+	return err
+}
+
 func (s *TimerLayerOAuthStore) SaveAccessData(accessData *model.AccessData) (*model.AccessData, error) {
 	start := time.Now()