Relese5.0 merge master 20180608 (#8933)

* Add missing diagnostics (#8911) * Update diagnostics.go * Update diagnostics.go * Fix push notification styling backwards compatibility (#8913) * MM-10803: remove premature user sanitization on deactivation (#8926) * remove unused UpdateNonSSOUserActive * MM-10803: stop prematurely sanitizing users on deactivate This change was preceded by the removal of UpdateNonSSOUserActive to ensure there are no APIs relying on the sanitized return value. * MM-10803: test websocket events after UpdateUserActive * MM-10264: Adds system scheme to permissions import/export. (#8924) * MM-10264: Adds system scheme to permissions import/export. * MM-10264: Switches to more likely unique name. * MM-10264: Changed collision prevention string. * MM-10264: Rolls back created schemes in all error cases. * MM-10264: Test fix for more rollback cases.
2025-02-25 18:55:24 -06:00 · 2018-06-08 17:04:17 +02:00
parent e09b3c566b
commit 3bae67489f
6 changed files with 195 additions and 89 deletions
--- a/api4/apitestlib.go
+++ b/api4/apitestlib.go
@@ -271,6 +271,10 @@ func (me *TestHelper) CreateWebSocketClient() (*model.WebSocketClient, *model.Ap
 	return model.NewWebSocketClient4(fmt.Sprintf("ws://localhost:%v", me.App.Srv.ListenAddr.Port), me.Client.AuthToken)
 }

+func (me *TestHelper) CreateWebSocketSystemAdminClient() (*model.WebSocketClient, *model.AppError) {
+	return model.NewWebSocketClient4(fmt.Sprintf("ws://localhost:%v", me.App.Srv.ListenAddr.Port), me.SystemAdminClient.AuthToken)
+}
+
 func (me *TestHelper) CreateUser() *model.User {
 	return me.CreateUserWithClient(me.Client)
 }
--- a/api4/user_test.go
+++ b/api4/user_test.go
@@ -513,7 +513,7 @@ func TestSearchUsers(t *testing.T) {
 		t.Fatal("should have found user")
 	}

-	_, err := th.App.UpdateNonSSOUserActive(th.BasicUser2.Id, false)
+	_, err := th.App.UpdateActive(th.BasicUser2, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -630,7 +630,7 @@ func TestSearchUsers(t *testing.T) {
 	th.App.UpdateConfig(func(cfg *model.Config) { cfg.PrivacySettings.ShowEmailAddress = false })
 	th.App.UpdateConfig(func(cfg *model.Config) { cfg.PrivacySettings.ShowFullName = false })

-	_, err = th.App.UpdateNonSSOUserActive(th.BasicUser2.Id, true)
+	_, err = th.App.UpdateActive(th.BasicUser2, true)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1205,71 +1205,159 @@ func TestUpdateUserRoles(t *testing.T) {
 	CheckBadRequestStatus(t, resp)
 }

+func assertExpectedWebsocketEvent(t *testing.T, client *model.WebSocketClient, event string, test func(*model.WebSocketEvent)) {
+	for {
+		select {
+		case resp, ok := <-client.EventChannel:
+			if !ok {
+				t.Fatalf("channel closed before receiving expected event %s", model.WEBSOCKET_EVENT_USER_UPDATED)
+			} else if resp.Event == model.WEBSOCKET_EVENT_USER_UPDATED {
+				test(resp)
+				return
+			}
+		case <-time.After(5 * time.Second):
+			t.Fatalf("failed to receive expected event %s", model.WEBSOCKET_EVENT_USER_UPDATED)
+		}
+	}
+}
+
+func assertWebsocketEventUserUpdatedWithEmail(t *testing.T, client *model.WebSocketClient, email string) {
+	assertExpectedWebsocketEvent(t, client, model.WEBSOCKET_EVENT_USER_UPDATED, func(event *model.WebSocketEvent) {
+		if eventUser, ok := event.Data["user"].(map[string]interface{}); !ok {
+			t.Fatalf("expected user")
+		} else if userEmail, ok := eventUser["email"].(string); !ok {
+			t.Fatalf("expected email %s, but got nil", email)
+		} else {
+			assert.Equal(t, email, userEmail)
+		}
+	})
+}
+
 func TestUpdateUserActive(t *testing.T) {
-	th := Setup().InitBasic().InitSystemAdmin()
-	defer th.TearDown()
+	t.Run("basic tests", func(t *testing.T) {
+		th := Setup().InitBasic().InitSystemAdmin()
+		defer th.TearDown()

-	Client := th.Client
-	SystemAdminClient := th.SystemAdminClient
-	user := th.BasicUser
+		Client := th.Client
+		SystemAdminClient := th.SystemAdminClient
+		user := th.BasicUser

-	EnableUserDeactivation := th.App.Config().TeamSettings.EnableUserDeactivation
-	defer func() {
-		th.App.UpdateConfig(func(cfg *model.Config) { cfg.TeamSettings.EnableUserDeactivation = EnableUserDeactivation })
-	}()
+		EnableUserDeactivation := th.App.Config().TeamSettings.EnableUserDeactivation
+		defer func() {
+			th.App.UpdateConfig(func(cfg *model.Config) { cfg.TeamSettings.EnableUserDeactivation = EnableUserDeactivation })
+		}()

-	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableUserDeactivation = true })
-	pass, resp := Client.UpdateUserActive(user.Id, false)
-	CheckNoError(t, resp)
+		th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableUserDeactivation = true })
+		pass, resp := Client.UpdateUserActive(user.Id, false)
+		CheckNoError(t, resp)

-	if !pass {
-		t.Fatal("should have returned true")
-	}
+		if !pass {
+			t.Fatal("should have returned true")
+		}

-	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableUserDeactivation = false })
-	pass, resp = Client.UpdateUserActive(user.Id, false)
-	CheckUnauthorizedStatus(t, resp)
+		th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableUserDeactivation = false })
+		pass, resp = Client.UpdateUserActive(user.Id, false)
+		CheckUnauthorizedStatus(t, resp)

-	if pass {
-		t.Fatal("should have returned false")
-	}
+		if pass {
+			t.Fatal("should have returned false")
+		}

-	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableUserDeactivation = true })
-	pass, resp = Client.UpdateUserActive(user.Id, false)
-	CheckUnauthorizedStatus(t, resp)
+		th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableUserDeactivation = true })
+		pass, resp = Client.UpdateUserActive(user.Id, false)
+		CheckUnauthorizedStatus(t, resp)

-	if pass {
-		t.Fatal("should have returned false")
-	}
+		if pass {
+			t.Fatal("should have returned false")
+		}

-	th.LoginBasic2()
+		th.LoginBasic2()

-	_, resp = Client.UpdateUserActive(user.Id, true)
-	CheckForbiddenStatus(t, resp)
+		_, resp = Client.UpdateUserActive(user.Id, true)
+		CheckForbiddenStatus(t, resp)

-	_, resp = Client.UpdateUserActive(GenerateTestId(), true)
-	CheckForbiddenStatus(t, resp)
+		_, resp = Client.UpdateUserActive(GenerateTestId(), true)
+		CheckForbiddenStatus(t, resp)

-	_, resp = Client.UpdateUserActive("junk", true)
-	CheckBadRequestStatus(t, resp)
+		_, resp = Client.UpdateUserActive("junk", true)
+		CheckBadRequestStatus(t, resp)

-	Client.Logout()
+		Client.Logout()

-	_, resp = Client.UpdateUserActive(user.Id, true)
-	CheckUnauthorizedStatus(t, resp)
+		_, resp = Client.UpdateUserActive(user.Id, true)
+		CheckUnauthorizedStatus(t, resp)

-	_, resp = SystemAdminClient.UpdateUserActive(user.Id, true)
-	CheckNoError(t, resp)
+		_, resp = SystemAdminClient.UpdateUserActive(user.Id, true)
+		CheckNoError(t, resp)

-	_, resp = SystemAdminClient.UpdateUserActive(user.Id, false)
-	CheckNoError(t, resp)
+		_, resp = SystemAdminClient.UpdateUserActive(user.Id, false)
+		CheckNoError(t, resp)

-	authData := model.NewId()
-	result := <-th.App.Srv.Store.User().UpdateAuthData(user.Id, "random", &authData, "", true)
-	require.Nil(t, result.Err)
+		authData := model.NewId()
+		result := <-th.App.Srv.Store.User().UpdateAuthData(user.Id, "random", &authData, "", true)
+		require.Nil(t, result.Err)

-	_, resp = SystemAdminClient.UpdateUserActive(user.Id, false)
-	CheckNoError(t, resp)
+		_, resp = SystemAdminClient.UpdateUserActive(user.Id, false)
+		CheckNoError(t, resp)
+	})
+
+	t.Run("websocket events", func(t *testing.T) {
+		th := Setup().InitBasic().InitSystemAdmin()
+		defer th.TearDown()
+
+		SystemAdminClient := th.SystemAdminClient
+		user := th.BasicUser2
+
+		EnableUserDeactivation := th.App.Config().TeamSettings.EnableUserDeactivation
+		defer func() {
+			th.App.UpdateConfig(func(cfg *model.Config) { cfg.TeamSettings.EnableUserDeactivation = EnableUserDeactivation })
+		}()
+
+		th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableUserDeactivation = true })
+
+		webSocketClient, err := th.CreateWebSocketClient()
+		assert.Nil(t, err)
+		defer webSocketClient.Close()
+
+		webSocketClient.Listen()
+
+		time.Sleep(300 * time.Millisecond)
+		if resp := <-webSocketClient.ResponseChannel; resp.Status != model.STATUS_OK {
+			t.Fatal("should have responded OK to authentication challenge")
+		}
+
+		adminWebSocketClient, err := th.CreateWebSocketSystemAdminClient()
+		assert.Nil(t, err)
+		defer adminWebSocketClient.Close()
+
+		adminWebSocketClient.Listen()
+
+		time.Sleep(300 * time.Millisecond)
+		if resp := <-adminWebSocketClient.ResponseChannel; resp.Status != model.STATUS_OK {
+			t.Fatal("should have responded OK to authentication challenge")
+		}
+
+		ShowEmailAddress := th.App.Config().PrivacySettings.ShowEmailAddress
+		defer func() {
+			th.App.UpdateConfig(func(cfg *model.Config) { cfg.PrivacySettings.ShowEmailAddress = ShowEmailAddress })
+		}()
+
+		// Verify that both admins and regular users see the email when privacy settings allow same.
+		th.App.UpdateConfig(func(cfg *model.Config) { cfg.PrivacySettings.ShowEmailAddress = true })
+		_, resp := SystemAdminClient.UpdateUserActive(user.Id, false)
+		CheckNoError(t, resp)
+
+		assertWebsocketEventUserUpdatedWithEmail(t, webSocketClient, user.Email)
+		assertWebsocketEventUserUpdatedWithEmail(t, adminWebSocketClient, user.Email)
+
+		// Verify that only admins see the email when privacy settings hide emails.
+		th.App.UpdateConfig(func(cfg *model.Config) { cfg.PrivacySettings.ShowEmailAddress = false })
+		_, resp = SystemAdminClient.UpdateUserActive(user.Id, true)
+		CheckNoError(t, resp)
+
+		assertWebsocketEventUserUpdatedWithEmail(t, webSocketClient, "")
+		assertWebsocketEventUserUpdatedWithEmail(t, adminWebSocketClient, user.Email)
+	})
 }

 func TestGetUsers(t *testing.T) {
--- a/app/permissions.go
+++ b/app/permissions.go
@@ -14,6 +14,7 @@ import (
 )

 const permissionsExportBatchSize = 100
+const systemSchemeName = "00000000-0000-0000-0000-000000000000" // Prevents collisions with user-created schemes.

 func (a *App) ResetPermissionsSystem() *model.AppError {
 	// Reset all Teams to not have a scheme.
@@ -101,6 +102,31 @@ func (a *App) ExportPermissions(w io.Writer) error {

 	}

+	defaultRoleNames := []string{}
+	for _, dr := range model.MakeDefaultRoles() {
+		defaultRoleNames = append(defaultRoleNames, dr.Name)
+	}
+
+	roles, appErr := a.GetRolesByNames(defaultRoleNames)
+	if appErr != nil {
+		return errors.New(appErr.Message)
+	}
+
+	schemeExport, err := json.Marshal(&model.SchemeConveyor{
+		Name:  systemSchemeName,
+		Roles: roles,
+	})
+	if err != nil {
+		return err
+	}
+
+	schemeExport = append(schemeExport, []byte("\n")...)
+
+	_, err = w.Write(schemeExport)
+	if err != nil {
+		return err
+	}
+
 	return nil
 }

@@ -113,13 +139,33 @@ func (a *App) ImportPermissions(jsonl io.Reader) error {
 		var schemeConveyor *model.SchemeConveyor
 		err := json.Unmarshal(scanner.Bytes(), &schemeConveyor)
 		if err != nil {
+			rollback(a, createdSchemeIDs)
 			return err
 		}

+		if schemeConveyor.Name == systemSchemeName {
+			for _, roleIn := range schemeConveyor.Roles {
+				dbRole, err := a.GetRoleByName(roleIn.Name)
+				if err != nil {
+					rollback(a, createdSchemeIDs)
+					return errors.New(err.Message)
+				}
+				_, err = a.PatchRole(dbRole, &model.RolePatch{
+					Permissions: &roleIn.Permissions,
+				})
+				if err != nil {
+					rollback(a, createdSchemeIDs)
+					return err
+				}
+			}
+			continue
+		}
+
 		// Create the new Scheme. The new Roles are created automatically.
 		var appErr *model.AppError
 		schemeCreated, appErr := a.CreateScheme(schemeConveyor.Scheme())
 		if appErr != nil {
+			rollback(a, createdSchemeIDs)
 			return errors.New(appErr.Message)
 		}
 		createdSchemeIDs = append(createdSchemeIDs, schemeCreated.Id)
@@ -139,21 +185,26 @@ func (a *App) ImportPermissions(jsonl io.Reader) error {
 			err = updateRole(a, schemeConveyor, roleNameTuple[0], roleNameTuple[1])
 			if err != nil {
 				// Delete the new Schemes. The new Roles are deleted automatically.
-				for _, schemeID := range createdSchemeIDs {
-					a.DeleteScheme(schemeID)
-				}
+				rollback(a, createdSchemeIDs)
 				return err
 			}
 		}
 	}

 	if err := scanner.Err(); err != nil {
+		rollback(a, createdSchemeIDs)
 		return err
 	}

 	return nil
 }

+func rollback(a *App, createdSchemeIDs []string) {
+	for _, schemeID := range createdSchemeIDs {
+		a.DeleteScheme(schemeID)
+	}
+}
+
 func updateRole(a *App, sc *model.SchemeConveyor, roleCreatedName, defaultRoleName string) error {
 	var err *model.AppError

--- a/app/permissions_test.go
+++ b/app/permissions_test.go
@@ -179,7 +179,7 @@ func TestImportPermissions_idempotentScheme(t *testing.T) {
 		if appErr != nil {
 			panic(appErr)
 		}
-		expected = len(results) + 1
+		expected = len(results)

 		err := th.App.ImportPermissions(r)
 		if err == nil {
--- a/app/user.go
+++ b/app/user.go
@@ -862,22 +862,6 @@ func (a *App) UpdatePasswordAsUser(userId, currentPassword, newPassword string)
 	return a.UpdatePasswordSendEmail(user, newPassword, T("api.user.update_password.menu"))
 }

-func (a *App) UpdateNonSSOUserActive(userId string, active bool) (*model.User, *model.AppError) {
-	var user *model.User
-	var err *model.AppError
-	if user, err = a.GetUser(userId); err != nil {
-		return nil, err
-	}
-
-	if user.IsSSOUser() {
-		err := model.NewAppError("UpdateActive", "api.user.update_active.no_deactivate_sso.app_error", nil, "userId="+user.Id, http.StatusBadRequest)
-		err.StatusCode = http.StatusBadRequest
-		return nil, err
-	}
-
-	return a.UpdateActive(user, active)
-}
-
 func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.AppError) {
 	if active {
 		user.DeleteAt = 0
@@ -895,9 +879,6 @@ func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.A
 		}

 		ruser := result.Data.([2]*model.User)[0]
-		options := a.Config().GetSanitizeOptions()
-		options["passwordupdate"] = false
-		ruser.Sanitize(options)

 		if !active {
 			a.SetStatusOffline(ruser.Id, false)
--- a/app/user_test.go
+++ b/app/user_test.go
@@ -96,24 +96,6 @@ func TestCreateOAuthUser(t *testing.T) {
 	}
 }

-func TestDeactivateSSOUser(t *testing.T) {
-	th := Setup().InitBasic()
-	defer th.TearDown()
-
-	r := rand.New(rand.NewSource(time.Now().UnixNano()))
-	glUser := oauthgitlab.GitLabUser{Id: int64(r.Intn(1000)) + 1, Username: "o" + model.NewId(), Email: model.NewId() + "@simulator.amazonses.com", Name: "Joram Wilander"}
-
-	json := glUser.ToJson()
-	user, err := th.App.CreateOAuthUser(model.USER_AUTH_SERVICE_GITLAB, strings.NewReader(json), th.BasicTeam.Id)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer th.App.PermanentDeleteUser(user)
-
-	_, err = th.App.UpdateNonSSOUserActive(user.Id, false)
-	assert.Equal(t, "api.user.update_active.no_deactivate_sso.app_error", err.Id)
-}
-
 func TestCreateProfileImage(t *testing.T) {
 	b, err := CreateProfileImage("Corey Hulen", "eo1zkdr96pdj98pjmq8zy35wba", "luximbi.ttf")
 	if err != nil {