From 869da7a78bcc30106410cf0d5cf40aa788a437c0 Mon Sep 17 00:00:00 2001 From: Max Erenberg Date: Mon, 12 Apr 2021 18:46:30 -0400 Subject: [PATCH] [MM-32044] Reset SAML auth data (#17161) Automatic Merge --- api4/api.go | 3 ++ api4/saml.go | 32 ++++++++++++++ api4/saml_test.go | 21 +++++++++ app/app_iface.go | 1 + app/opentracing/opentracing_layer.go | 22 ++++++++++ app/saml.go | 13 ++++++ build/docker/keycloak/README.md | 9 ++-- i18n/en.json | 4 ++ model/client4.go | 22 ++++++++++ store/opentracinglayer/opentracinglayer.go | 18 ++++++++ store/retrylayer/retrylayer.go | 20 +++++++++ store/sqlstore/user_store.go | 42 ++++++++++++++++++ store/store.go | 1 + store/storetest/mocks/UserStore.go | 21 +++++++++ store/storetest/user_store.go | 50 ++++++++++++++++++++++ store/timerlayer/timerlayer.go | 16 +++++++ 16 files changed, 292 insertions(+), 3 deletions(-) diff --git a/api4/api.go b/api4/api.go index 46b57ae703..d5a9c3aa5f 100644 --- a/api4/api.go +++ b/api4/api.go @@ -363,6 +363,8 @@ func InitLocal(configservice configservice.ConfigService, globalOptionsFunc app. api.BaseRoutes.Jobs = api.BaseRoutes.ApiRoot.PathPrefix("/jobs").Subrouter() + api.BaseRoutes.SAML = api.BaseRoutes.ApiRoot.PathPrefix("/saml").Subrouter() + api.InitUserLocal() api.InitTeamLocal() api.InitChannelLocal() @@ -381,6 +383,7 @@ func InitLocal(configservice configservice.ConfigService, globalOptionsFunc app. api.InitImportLocal() api.InitExportLocal() api.InitJobLocal() + api.InitSamlLocal() root.Handle("/api/v4/{anything:.*}", http.HandlerFunc(api.Handle404)) diff --git a/api4/saml.go b/api4/saml.go index eef2f46941..a9ff6dfe18 100644 --- a/api4/saml.go +++ b/api4/saml.go @@ -4,6 +4,7 @@ package api4 import ( + "encoding/json" "io/ioutil" "mime" "mime/multipart" @@ -27,6 +28,12 @@ func (api *API) InitSaml() { api.BaseRoutes.SAML.Handle("/certificate/status", api.ApiSessionRequired(getSamlCertificateStatus)).Methods("GET") api.BaseRoutes.SAML.Handle("/metadatafromidp", api.ApiHandler(getSamlMetadataFromIdp)).Methods("POST") + + api.BaseRoutes.SAML.Handle("/reset_auth_data", api.ApiSessionRequired(resetAuthDataToEmail)).Methods("POST") +} + +func (api *API) InitSamlLocal() { + api.BaseRoutes.SAML.Handle("/reset_auth_data", api.ApiLocal(resetAuthDataToEmail)).Methods("POST") } func getSamlMetadata(c *Context, w http.ResponseWriter, r *http.Request) { @@ -247,3 +254,28 @@ func getSamlMetadataFromIdp(c *Context, w http.ResponseWriter, r *http.Request) w.Write([]byte(metadata.ToJson())) } + +func resetAuthDataToEmail(c *Context, w http.ResponseWriter, r *http.Request) { + if !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) { + c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM) + return + } + type ResetAuthDataParams struct { + IncludeDeleted bool `json:"include_deleted"` + DryRun bool `json:"dry_run"` + SpecifiedUserIDs []string `json:"user_ids"` + } + var params *ResetAuthDataParams + jsonErr := json.NewDecoder(r.Body).Decode(¶ms) + if jsonErr != nil { + c.Err = model.NewAppError("resetAuthDataToEmail", "model.utils.decode_json.app_error", nil, jsonErr.Error(), http.StatusBadRequest) + return + } + numAffected, appErr := c.App.ResetSamlAuthDataToEmail(params.IncludeDeleted, params.DryRun, params.SpecifiedUserIDs) + if appErr != nil { + c.Err = appErr + return + } + b, _ := json.Marshal(map[string]interface{}{"num_affected": numAffected}) + w.Write(b) +} diff --git a/api4/saml_test.go b/api4/saml_test.go index b0c1131b64..67224c0b9a 100644 --- a/api4/saml_test.go +++ b/api4/saml_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/v5/einterfaces/mocks" "github.com/mattermost/mattermost-server/v5/model" ) @@ -50,3 +51,23 @@ func TestSamlCompleteCSRFPass(t *testing.T) { require.NotEqual(t, http.StatusUnauthorized, resp.StatusCode) defer resp.Body.Close() } + +func TestSamlResetId(t *testing.T) { + th := SetupEnterprise(t).InitBasic() + defer th.TearDown() + th.App.Srv().Saml = &mocks.SamlInterface{} + + user := th.BasicUser + _, appErr := th.App.UpdateUserAuth(user.Id, &model.UserAuth{ + AuthData: model.NewString(model.NewId()), + AuthService: model.USER_AUTH_SERVICE_SAML, + }) + require.Nil(t, appErr) + + _, resp := th.Client.ResetSamlAuthDataToEmail(false, false, nil) + CheckForbiddenStatus(t, resp) + + numAffected, resp := th.SystemAdminClient.ResetSamlAuthDataToEmail(false, false, nil) + CheckOKStatus(t, resp) + require.Equal(t, int64(1), numAffected) +} diff --git a/app/app_iface.go b/app/app_iface.go index 28840b315b..0f1d9efcb8 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -893,6 +893,7 @@ type AppIface interface { RequestLicenseAndAckWarnMetric(warnMetricId string, isBot bool) *model.AppError ResetPasswordFromToken(userSuppliedTokenString, newPassword string) *model.AppError ResetPermissionsSystem() *model.AppError + ResetSamlAuthDataToEmail(includeDeleted bool, dryRun bool, userIDs []string) (numAffected int, appErr *model.AppError) RestoreChannel(channel *model.Channel, userID string) (*model.Channel, *model.AppError) RestoreTeam(teamID string) *model.AppError RestrictUsersGetByPermissions(userID string, options *model.UserGetOptions) (*model.UserGetOptions, *model.AppError) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 6a1fe19ddb..970417d99d 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -12785,6 +12785,28 @@ func (a *OpenTracingAppLayer) ResetPermissionsSystem() *model.AppError { return resultVar0 } +func (a *OpenTracingAppLayer) ResetSamlAuthDataToEmail(includeDeleted bool, dryRun bool, userIDs []string) (numAffected int, appErr *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.ResetSamlAuthDataToEmail") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.ResetSamlAuthDataToEmail(includeDeleted, dryRun, userIDs) + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) RestoreChannel(channel *model.Channel, userID string) (*model.Channel, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RestoreChannel") diff --git a/app/saml.go b/app/saml.go index 53d93357f5..9cf2a2f626 100644 --- a/app/saml.go +++ b/app/saml.go @@ -282,3 +282,16 @@ func (a *App) SetSamlIdpCertificateFromMetadata(data []byte) *model.AppError { return nil } + +func (a *App) ResetSamlAuthDataToEmail(includeDeleted bool, dryRun bool, userIDs []string) (numAffected int, appErr *model.AppError) { + if a.Saml() == nil { + appErr = model.NewAppError("ResetAuthDataToEmail", "api.admin.saml.not_available.app_error", nil, "", http.StatusNotImplemented) + return + } + numAffected, err := a.srv.Store.User().ResetAuthDataToEmailForUsers(model.USER_AUTH_SERVICE_SAML, userIDs, includeDeleted, dryRun) + if err != nil { + appErr = model.NewAppError("ResetAuthDataToEmail", "api.admin.saml.failure_reset_authdata_to_email.app_error", nil, err.Error(), http.StatusInternalServerError) + return + } + return +} diff --git a/build/docker/keycloak/README.md b/build/docker/keycloak/README.md index e77839c5cc..828f72d329 100644 --- a/build/docker/keycloak/README.md +++ b/build/docker/keycloak/README.md @@ -5,10 +5,11 @@ To use this keycloak image, we suggest you to use this configuration settings: - Override SAML bind data with AD/LDAP information: `false` - Identity Provider Metadata URL: empty string - SAML SSO URL: `http://localhost:8484/auth/realms/mattermost/protocol/saml` -- Identity Provider Issuer URL: h`ttp://localhost:8065/login/sso/SAML` -- Identity Provider Public Certificate: The file `keycloak_cert.pem` in this same directory +- Identity Provider Issuer URL: `http://localhost:8484/auth/realms/mattermost` +- Identity Provider Public Certificate: The file `keycloak.crt` in this same directory - Verify Signature: `true` - Service Provider Login URL: `http://localhost:8065/login/sso/saml` +- Service Provider Identifier: `http://localhost:8065/login/sso/saml` - Enable Encryption: `false` - Sign Request: `false` - Email Attribute: `email` @@ -25,12 +26,14 @@ database configuration) and restart the server: "Enable": true, "EnableSyncWithLdap": true, "EnableSyncWithLdapIncludeAuth": false, + "IgnoreGuestsLdapSync": false, "Verify": true, "Encrypt": false, "SignRequest": false, "IdpUrl": "http://localhost:8484/auth/realms/mattermost/protocol/saml", - "IdpDescriptorUrl": "http://localhost:8065/login/sso/saml", + "IdpDescriptorUrl": "http://localhost:8484/auth/realms/mattermost", "IdpMetadataUrl": "", + "ServiceProviderIdentifier": "http://localhost:8065/login/sso/saml", "AssertionConsumerServiceURL": "http://localhost:8065/login/sso/saml", "SignatureAlgorithm": "RSAwithSHA1", "CanonicalAlgorithm": "Canonical1.0", diff --git a/i18n/en.json b/i18n/en.json index 0324812670..2b3deed2fb 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -95,6 +95,10 @@ "id": "api.admin.saml.failure_parse_idp_certificate.app_error", "translation": "Failure encountered while parsing the metadata information received from the Identity Provider to a certificate." }, + { + "id": "api.admin.saml.failure_reset_authdata_to_email.app_error", + "translation": "Failed to reset AuthData field to Email." + }, { "id": "api.admin.saml.failure_save_idp_certificate_file.app_error", "translation": "Could not save certificate file." diff --git a/model/client4.go b/model/client4.go index 5c4e513eb4..0d2f5d4f97 100644 --- a/model/client4.go +++ b/model/client4.go @@ -3899,6 +3899,28 @@ func (c *Client4) GetSamlMetadataFromIdp(samlMetadataURL string) (*SamlMetadataR return SamlMetadataResponseFromJson(r.Body), BuildResponse(r) } +// ResetSamlAuthDataToEmail resets the AuthData field of SAML users to their Email. +func (c *Client4) ResetSamlAuthDataToEmail(includeDeleted bool, dryRun bool, userIDs []string) (int64, *Response) { + params := map[string]interface{}{ + "include_deleted": includeDeleted, + "dry_run": dryRun, + "user_ids": userIDs, + } + b, _ := json.Marshal(params) + r, err := c.doApiPostBytes(c.GetSamlRoute()+"/reset_auth_data", b) + if err != nil { + return 0, BuildErrorResponse(r, err) + } + defer closeBody(r) + respBody := map[string]int64{} + jsonErr := json.NewDecoder(r.Body).Decode(&respBody) + if jsonErr != nil { + appErr := NewAppError("Api4.ResetSamlAuthDataToEmail", "api.marshal_error", nil, err.Error(), http.StatusInternalServerError) + return 0, BuildErrorResponse(r, appErr) + } + return respBody["num_affected"], BuildResponse(r) +} + // Compliance Section // CreateComplianceReport creates an incoming webhook for a channel. diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 2ac7ee7ca7..59794ab75a 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -9788,6 +9788,24 @@ func (s *OpenTracingLayerUserStore) PromoteGuestToUser(userID string) error { return err } +func (s *OpenTracingLayerUserStore) ResetAuthDataToEmailForUsers(service string, userIDs []string, includeDeleted bool, dryRun bool) (int, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.ResetAuthDataToEmailForUsers") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.UserStore.ResetAuthDataToEmailForUsers(service, userIDs, includeDeleted, dryRun) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerUserStore) ResetLastPictureUpdate(userID string) error { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.ResetLastPictureUpdate") diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index 6ab4c139af..49402206af 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -10606,6 +10606,26 @@ func (s *RetryLayerUserStore) PromoteGuestToUser(userID string) error { } +func (s *RetryLayerUserStore) ResetAuthDataToEmailForUsers(service string, userIDs []string, includeDeleted bool, dryRun bool) (int, error) { + + tries := 0 + for { + result, err := s.UserStore.ResetAuthDataToEmailForUsers(service, userIDs, includeDeleted, dryRun) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } + +} + func (s *RetryLayerUserStore) ResetLastPictureUpdate(userID string) error { tries := 0 diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index c5d5724bd2..515538a908 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -308,6 +308,48 @@ func (us SqlUserStore) UpdateAuthData(userId string, service string, authData *s return userId, nil } +// ResetAuthDataToEmailForUsers resets the AuthData of users whose AuthService +// is |service| to their Email. If userIDs is non-empty, only the users whose +// IDs are in userIDs will be affected. If dryRun is true, only the number +// of users who *would* be affected is returned; otherwise, the number of +// users who actually were affected is returned. +func (us SqlUserStore) ResetAuthDataToEmailForUsers(service string, userIDs []string, includeDeleted bool, dryRun bool) (int, error) { + whereEquals := sq.Eq{"AuthService": service} + if len(userIDs) > 0 { + whereEquals["Id"] = userIDs + } + if !includeDeleted { + whereEquals["DeleteAt"] = 0 + } + + if dryRun { + builder := us.getQueryBuilder(). + Select("COUNT(*)"). + From("Users"). + Where(whereEquals) + query, args, err := builder.ToSql() + if err != nil { + return 0, errors.Wrap(err, "select_count_users_tosql") + } + numAffected, err := us.GetReplica().SelectInt(query, args...) + return int(numAffected), err + } + builder := us.getQueryBuilder(). + Update("Users"). + Set("AuthData", sq.Expr("Email")). + Where(whereEquals) + query, args, err := builder.ToSql() + if err != nil { + return 0, errors.Wrap(err, "update_users_tosql") + } + result, err := us.GetMaster().Exec(query, args...) + if err != nil { + return 0, errors.Wrap(err, "failed to update users' AuthData") + } + numAffected, err := result.RowsAffected() + return int(numAffected), err +} + func (us SqlUserStore) UpdateMfaSecret(userId, secret string) error { updateAt := model.GetMillis() diff --git a/store/store.go b/store/store.go index 9f8adf4586..85f411f1c1 100644 --- a/store/store.go +++ b/store/store.go @@ -328,6 +328,7 @@ type UserStore interface { UpdatePassword(userID, newPassword string) error UpdateUpdateAt(userID string) (int64, error) UpdateAuthData(userID string, service string, authData *string, email string, resetMfa bool) (string, error) + ResetAuthDataToEmailForUsers(service string, userIDs []string, includeDeleted bool, dryRun bool) (int, error) UpdateMfaSecret(userID, secret string) error UpdateMfaActive(userID string, active bool) error Get(ctx context.Context, id string) (*model.User, error) diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index e1f3dce672..7155381b2a 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -1066,6 +1066,27 @@ func (_m *UserStore) PromoteGuestToUser(userID string) error { return r0 } +// ResetAuthDataToEmailForUsers provides a mock function with given fields: service, userIDs, includeDeleted, dryRun +func (_m *UserStore) ResetAuthDataToEmailForUsers(service string, userIDs []string, includeDeleted bool, dryRun bool) (int, error) { + ret := _m.Called(service, userIDs, includeDeleted, dryRun) + + var r0 int + if rf, ok := ret.Get(0).(func(string, []string, bool, bool) int); ok { + r0 = rf(service, userIDs, includeDeleted, dryRun) + } else { + r0 = ret.Get(0).(int) + } + + var r1 error + if rf, ok := ret.Get(1).(func(string, []string, bool, bool) error); ok { + r1 = rf(service, userIDs, includeDeleted, dryRun) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // ResetLastPictureUpdate provides a mock function with given fields: userID func (_m *UserStore) ResetLastPictureUpdate(userID string) error { ret := _m.Called(userID) diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 94c547e0b3..5e91d50224 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -67,6 +67,7 @@ func TestUserStore(t *testing.T, ss store.Store, s SqlStore) { t.Run("UpdatePassword", func(t *testing.T) { testUserStoreUpdatePassword(t, ss) }) t.Run("Delete", func(t *testing.T) { testUserStoreDelete(t, ss) }) t.Run("UpdateAuthData", func(t *testing.T) { testUserStoreUpdateAuthData(t, ss) }) + t.Run("ResetAuthDataToEmailForUsers", func(t *testing.T) { testUserStoreResetAuthDataToEmailForUsers(t, ss) }) t.Run("UserUnreadCount", func(t *testing.T) { testUserUnreadCount(t, ss) }) t.Run("UpdateMfaSecret", func(t *testing.T) { testUserStoreUpdateMfaSecret(t, ss) }) t.Run("UpdateMfaActive", func(t *testing.T) { testUserStoreUpdateMfaActive(t, ss) }) @@ -2183,6 +2184,55 @@ func testUserStoreUpdateAuthData(t *testing.T, ss store.Store) { require.Equal(t, "", user.Password, "Password was not cleared properly") } +func testUserStoreResetAuthDataToEmailForUsers(t *testing.T, ss store.Store) { + user := &model.User{} + user.Username = "user1" + model.NewId() + user.Email = MakeEmail() + _, err := ss.User().Save(user) + require.NoError(t, err) + defer func() { require.NoError(t, ss.User().PermanentDelete(user.Id)) }() + + resetAuthDataToID := func() { + _, err = ss.User().UpdateAuthData( + user.Id, model.USER_AUTH_SERVICE_SAML, model.NewString("some-id"), "", false) + require.NoError(t, err) + } + resetAuthDataToID() + + // dry run + numAffected, err := ss.User().ResetAuthDataToEmailForUsers(model.USER_AUTH_SERVICE_SAML, nil, false, true) + require.NoError(t, err) + require.Equal(t, 1, numAffected) + // real run + numAffected, err = ss.User().ResetAuthDataToEmailForUsers(model.USER_AUTH_SERVICE_SAML, nil, false, false) + require.NoError(t, err) + require.Equal(t, 1, numAffected) + user, appErr := ss.User().Get(context.Background(), user.Id) + require.NoError(t, appErr) + require.Equal(t, *user.AuthData, user.Email) + + resetAuthDataToID() + // with specific user IDs + numAffected, err = ss.User().ResetAuthDataToEmailForUsers(model.USER_AUTH_SERVICE_SAML, []string{model.NewId()}, false, true) + require.NoError(t, err) + require.Equal(t, 0, numAffected) + numAffected, err = ss.User().ResetAuthDataToEmailForUsers(model.USER_AUTH_SERVICE_SAML, []string{user.Id}, false, true) + require.NoError(t, err) + require.Equal(t, 1, numAffected) + + // delete user + user.DeleteAt = model.GetMillisForTime(time.Now()) + ss.User().Update(user, true) + // without deleted user + numAffected, err = ss.User().ResetAuthDataToEmailForUsers(model.USER_AUTH_SERVICE_SAML, nil, false, true) + require.NoError(t, err) + require.Equal(t, 0, numAffected) + // with deleted user + numAffected, err = ss.User().ResetAuthDataToEmailForUsers(model.USER_AUTH_SERVICE_SAML, nil, true, true) + require.NoError(t, err) + require.Equal(t, 1, numAffected) +} + func testUserUnreadCount(t *testing.T, ss store.Store) { teamId := model.NewId() diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index d9bef90a2e..946e3bd350 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -8839,6 +8839,22 @@ func (s *TimerLayerUserStore) PromoteGuestToUser(userID string) error { return err } +func (s *TimerLayerUserStore) ResetAuthDataToEmailForUsers(service string, userIDs []string, includeDeleted bool, dryRun bool) (int, error) { + start := timemodule.Now() + + result, err := s.UserStore.ResetAuthDataToEmailForUsers(service, userIDs, includeDeleted, dryRun) + + elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("UserStore.ResetAuthDataToEmailForUsers", success, elapsed) + } + return result, err +} + func (s *TimerLayerUserStore) ResetLastPictureUpdate(userID string) error { start := timemodule.Now()