diff --git a/server/channels/app/oauth.go b/server/channels/app/oauth.go index 300fe046b4..b8f0ec4c52 100644 --- a/server/channels/app/oauth.go +++ b/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) diff --git a/server/channels/app/oauth_test.go b/server/channels/app/oauth_test.go index 7443683941..2167973391 100644 --- a/server/channels/app/oauth_test.go +++ b/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) +} diff --git a/server/channels/app/user.go b/server/channels/app/user.go index 6b85b28f7e..f0b6b7c826 100644 --- a/server/channels/app/user.go +++ b/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 } diff --git a/server/channels/store/opentracinglayer/opentracinglayer.go b/server/channels/store/opentracinglayer/opentracinglayer.go index 723b8a4775..5a193bcb76 100644 --- a/server/channels/store/opentracinglayer/opentracinglayer.go +++ b/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") diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 3603bd7f7e..f7ec2bfd27 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/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 diff --git a/server/channels/store/sqlstore/oauth_store.go b/server/channels/store/sqlstore/oauth_store.go index 10b9d696ed..ba738b7aa8 100644 --- a/server/channels/store/sqlstore/oauth_store.go +++ b/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 { diff --git a/server/channels/store/store.go b/server/channels/store/store.go index e52c2037ab..52774d2e71 100644 --- a/server/channels/store/store.go +++ b/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) diff --git a/server/channels/store/storetest/mocks/OAuthStore.go b/server/channels/store/storetest/mocks/OAuthStore.go index 77d8beeb6e..75cd533163 100644 --- a/server/channels/store/storetest/mocks/OAuthStore.go +++ b/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) diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 2c156ccbe5..7c8c64a033 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/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()