Change role of existing users only if SSO attributes are present in the SAMLResponse (#11966)

#10784

The removal of the now deprecated `sso_settings.enable_jit_role_sync`
config will be tackled in: #10688.

- [X] Changes file added for user-visible changes in `changes/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- ~[ ] Documented any API changes (docs/Using-Fleet/REST-API.md or
docs/Contributing/API-for-contributors.md)~
- ~[ ] Documented any permissions changes~
- ~[ ] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)~
- ~[ ] Added support on fleet's osquery simulator `cmd/osquery-perf` for
new osquery data ingestion features.~
- [X] Added/updated tests
- [X] Manual QA for all new/changed functionality
  - ~For Orbit and Fleet Desktop changes:~
- ~[ ] Manual QA must be performed in the three main OSs, macOS, Windows
and Linux.~
- ~[ ] Auto-update manual QA, from released version of component to new
version (see [tools/tuf/test](../tools/tuf/test/README.md)).~

changes/10784-change-role-of-existing-users-only-if-sso-attrs-are-present

@@ -0,0 +1 @@
+* Deprecate `enable_jit_role_sync` setting and only change role for existing users if role attributes are set in the `SAMLResponse`.

docs/Deploying/Configuration.md

@@ -3057,7 +3057,7 @@ The new account's email and full name are copied from the user data in the SSO r
 By default, accounts created via JIT provisioning are assigned the [Global Observer role](https://fleetdm.com/docs/using-fleet/permissions).
 To assign different roles for accounts created via JIT provisioning see [Customization of user roles](#customization-of-user-roles) below.
 
-To enable this option, go to **Settings > Organization settings > single sign-on options** and check "_Automatically create Observer user on login_" or [adjust your config](#sso-settings-enable-jit-provisioning).
+To enable this option, go to **Settings > Organization settings > single sign-on options** and check "_Create user and sync permissions on login_" or [adjust your config](#sso-settings-enable-jit-provisioning).
 
 For this to work correctly make sure that:
 
@@ -3071,6 +3071,8 @@ For this to work correctly make sure that:
 
 #### Customization of user roles
 
+> This feature requires setting `sso_settings.enable_jit_provisioning` to `true`.
+
 Users created via JIT provisioning can be assigned Fleet roles using SAML custom attributes that are sent by the IdP in `SAMLResponse`s during login.
 Fleet will attempt to parse SAML custom attributes with the following format:
 - `FLEET_JIT_USER_ROLE_GLOBAL`: Specifies the global role to use when creating the user.
@@ -3082,10 +3084,13 @@ SAML supports multi-valued attributes, Fleet will always use the last value.
 
 NOTE: Setting both `FLEET_JIT_USER_ROLE_GLOBAL` and `FLEET_JIT_USER_ROLE_TEAM_<TEAM_ID>` will cause an error during login as Fleet users cannot be Global users and belong to teams.
 
-During every SSO login, if `sso_settings.enable_jit_role_sync` is set to `true` (default is `false`) and if the account already exists, the roles of the Fleet account will be updated to match those set in the SAML custom attributes.
-> IMPORTANT: Beware that if `sso_settings.enable_jit_role_sync` is set to `true` but no SAML role attributes are configured for accounts then all Fleet users are changed to Global observers on every SSO login (overriding any previous role change).
-
-If none of the attributes above are set, then Fleet will default to use the `Global Observer` role.
+Following is the behavior that will take place on every SSO login:
+A. If the account does not exist then:
+    - If the `SAMLResponse` has any role attributes then those will be used to set the account roles.
+    - If the `SAMLResponse` does not have any role attributes set, then Fleet will default to use the `Global Observer` role.
+B. If the account already exists:
+    - If the `SAMLResponse` has any role attributes then those will be used to update the account roles.
+    - If the `SAMLResponse` does not have any role attributes set, no role change is attempted.
 
 Here's a `SAMLResponse` sample to set the role of SSO users to Global `admin`:
 ```xml

docs/Using-Fleet/configuration-files/README.md

@@ -704,18 +704,8 @@ For additional information on SSO configuration, including just-in-time (JIT) us
 
 ##### sso_settings.enable_jit_role_sync
 
-**Available in Fleet Premium**.
-
-If set to `true` Fleet account roles will be updated to match those set in the SAML custom attributes at every login. See [customization of user roles](../../Deploying/Configuration.md#customization-of-user-roles).
-This flag only has effect if `sso_settings.enable_jit_provisioning` is set to `true`.
-
-- Optional setting (boolean)
-- Default value: `false`
-- Config file format:
-  ```yaml
-  sso_settings:
-    enable_jit_role_sync: true
-  ```
+> This setting is now deprecated and will be removed soon.
+> For more information on how SSO login and role syncing works see [customization of user roles](../../Deploying/Configuration.md#customization-of-user-roles) 
 
 ##### sso_settings.enable_sso
 

ee/server/service/users.go

@@ -32,13 +32,23 @@ func (svc *Service) GetSSOUser(ctx context.Context, auth fleet.Auth) (*fleet.Use
 		// If the user exists, we want to update the user roles from the attributes received
 		// in the SAMLResponse.
 
-		// If JIT provisioning or role sync are disabled, then we don't attempt to change the
-		// role of the existing user.
-		if !config.SSOSettings.EnableJITProvisioning || !config.SSOSettings.EnableJITRoleSync {
+		// If JIT provisioning is disabled, then Fleet does not attempt to change
+		// the role of the existing user.
+		if !config.SSOSettings.EnableJITProvisioning {
 			return user, nil
 		}
 
-		newGlobalRole, newTeamsRoles, err := svc.userRolesFromSSOAttributes(ctx, auth)
+		// Load custom roles from SSO attributes.
+		ssoRolesInfo, err := fleet.RolesFromSSOAttributes(auth.AssertionAttributes())
+		if err != nil {
+			return nil, ctxerr.Wrap(ctx, err, "invalid SSO attributes")
+		}
+		if !ssoRolesInfo.IsSet() {
+			// If role attributes were not set, then there's nothing to do here.
+			return user, nil
+		}
+
+		newGlobalRole, newTeamsRoles, err := svc.userRolesFromSSOAttributes(ctx, ssoRolesInfo)
 		if err != nil {
 			return nil, ctxerr.Wrap(ctx, err, "user roles from SSO attributes")
 		}
@@ -77,10 +87,23 @@ func (svc *Service) GetSSOUser(ctx context.Context, auth fleet.Auth) (*fleet.Use
 		displayName = auth.UserID()
 	}
 
-	// Retrieve (if set) user roles from SAML custom attributes.
-	globalRole, teamRoles, err := svc.userRolesFromSSOAttributes(ctx, auth)
+	var (
+		globalRole *string
+		teamRoles  []fleet.UserTeam
+	)
+	// Attempt to retrieve user roles from SAML custom attributes.
+	ssoRolesInfo, err := fleet.RolesFromSSOAttributes(auth.AssertionAttributes())
 	if err != nil {
-		return nil, ctxerr.Wrap(ctx, err, "user roles from SSO attributes")
+		return nil, ctxerr.Wrap(ctx, err, "invalid SSO attributes")
+	}
+	if ssoRolesInfo.IsSet() {
+		globalRole, teamRoles, err = svc.userRolesFromSSOAttributes(ctx, ssoRolesInfo)
+		if err != nil {
+			return nil, ctxerr.Wrap(ctx, err, "user roles from SSO attributes")
+		}
+	} else {
+		// If no roles are set in the SSO attributes, default to setting user as a global observer.
+		globalRole = ptr.String(fleet.RoleObserver)
 	}
 
 	user, err = svc.Service.NewUser(ctx, fleet.UserPayload{
@@ -129,18 +152,9 @@ func rolesChanged(oldGlobal *string, oldTeams []fleet.UserTeam, newGlobal *strin
 	return false
 }
 
-// userRolesFromSSOAttributes loads the global or team roles from custom SSO attributes.
-//
-// The returned `globalRole` and `teamRoles` are ready to be assigned to
-// `fleet.User` struct fields `GlobalRole` and `Teams` respectively.
-//
-// If the custom attributes are not found, then the default global observer ("observer", nil, nil) is returned.
-func (svc *Service) userRolesFromSSOAttributes(ctx context.Context, auth fleet.Auth) (globalRole *string, teamsRoles []fleet.UserTeam, err error) {
-	ssoRolesInfo, err := fleet.RolesFromSSOAttributes(auth.AssertionAttributes())
-	if err != nil {
-		return nil, nil, ctxerr.Wrap(ctx, err, "invalid SSO attributes")
-	}
-
+// userRolesFromSSOAttributes returns `globalRole` and `teamRoles` ready to be assigned
+// to a `fleet.User` struct fields `GlobalRole` and `Teams` respectively.
+func (svc *Service) userRolesFromSSOAttributes(ctx context.Context, ssoRolesInfo fleet.SSORolesInfo) (globalRole *string, teamsRoles []fleet.UserTeam, err error) {
 	for _, teamRole := range ssoRolesInfo.Teams {
 		team, err := svc.ds.Team(ctx, teamRole.ID)
 		if err != nil {

frontend/pages/admin/OrgSettingsPage/cards/Sso/Sso.tsx

@@ -224,7 +224,7 @@ const Sso = ({
               parseTarget
             >
               <>
-                Automatically create Observer user on login{" "}
+                Create user and sync permissions on login{" "}
                 <CustomLink
                   url="https://fleetdm.com/docs/deploying/configuration?utm_medium=fleetui&utm_source=sso-settings#just-in-time-jit-user-provisioning"
                   text="Learn more"

server/fleet/app.go

@@ -67,6 +67,8 @@ type SSOSettings struct {
 	// EnableJITProvisioning allows user accounts to be created the first time
 	// users try to log in
 	EnableJITProvisioning bool `json:"enable_jit_provisioning"`
+	// EnableJITRoleSync is deprecated.
+	//
 	// EnableJITRoleSync sets whether the roles of existing accounts will be updated
 	// every time SSO users log in (does not have effect if EnableJITProvisioning is false).
 	EnableJITRoleSync bool `json:"enable_jit_role_sync"`

server/fleet/sessions.go

@@ -107,8 +107,9 @@ func (s SSORolesInfo) verify() error {
 	return nil
 }
 
-func (s SSORolesInfo) isEmpty() bool {
-	return s.Global == nil && len(s.Teams) == 0
+// IsSet returns whether any role attributes were set.
+func (s SSORolesInfo) IsSet() bool {
+	return s.Global != nil || len(s.Teams) != 0
 }
 
 const (
@@ -163,11 +164,6 @@ func RolesFromSSOAttributes(attributes []SAMLAttribute) (SSORolesInfo, error) {
 	if err := ssoRolesInfo.verify(); err != nil {
 		return SSORolesInfo{}, err
 	}
-	if ssoRolesInfo.isEmpty() {
-		// When the configuration is not set, the default is to
-		// make the user a global observer.
-		return SSORolesInfo{Global: ptr.String(RoleObserver)}, nil
-	}
 	return ssoRolesInfo, nil
 }
 

server/fleet/sessions_test.go

@@ -15,20 +15,16 @@ func TestRolesFromSSOAttributes(t *testing.T) {
 		expectedSSORolesInfo SSORolesInfo
 	}{
 		{
-			name:       "nil-should-use-default",
-			attributes: nil,
-			shouldFail: false,
-			expectedSSORolesInfo: SSORolesInfo{
-				Global: ptr.String("observer"),
-			},
+			name:                 "nil",
+			attributes:           nil,
+			shouldFail:           false,
+			expectedSSORolesInfo: SSORolesInfo{},
 		},
 		{
-			name:       "empty-should-use-default",
-			attributes: []SAMLAttribute{},
-			shouldFail: false,
-			expectedSSORolesInfo: SSORolesInfo{
-				Global: ptr.String("observer"),
-			},
+			name:                 "no-role-attributes",
+			attributes:           []SAMLAttribute{},
+			shouldFail:           false,
+			expectedSSORolesInfo: SSORolesInfo{},
 		},
 		{
 			name: "unknown-key-should-use-default",
@@ -40,10 +36,8 @@ func TestRolesFromSSOAttributes(t *testing.T) {
 					},
 				},
 			},
-			shouldFail: false,
-			expectedSSORolesInfo: SSORolesInfo{
-				Global: ptr.String("observer"),
-			},
+			shouldFail:           false,
+			expectedSSORolesInfo: SSORolesInfo{},
 		},
 		{
 			name: "global-only",
@@ -252,7 +246,7 @@ func TestRolesFromSSOAttributes(t *testing.T) {
 			},
 		},
 		{
-			name: "null-value-on-team-attribute-is-ignored-should-use-default",
+			name: "null-value-on-team-attribute-is-ignored",
 			attributes: []SAMLAttribute{
 				{
 					Name: teamUserRoleSSOAttrNamePrefix + "1",
@@ -261,13 +255,11 @@ func TestRolesFromSSOAttributes(t *testing.T) {
 					},
 				},
 			},
-			shouldFail: false,
-			expectedSSORolesInfo: SSORolesInfo{
-				Global: ptr.String("observer"),
-			},
+			shouldFail:           false,
+			expectedSSORolesInfo: SSORolesInfo{},
 		},
 		{
-			name: "null-attributes-are-ignored-should-use-default",
+			name: "null-attributes-on-global-and-team-are-ignored",
 			attributes: []SAMLAttribute{
 				{
 					Name: globalUserRoleSSOAttrName,
@@ -282,10 +274,8 @@ func TestRolesFromSSOAttributes(t *testing.T) {
 					},
 				},
 			},
-			shouldFail: false,
-			expectedSSORolesInfo: SSORolesInfo{
-				Global: ptr.String("observer"),
-			},
+			shouldFail:           false,
+			expectedSSORolesInfo: SSORolesInfo{},
 		},
 		{
 			name: "null-attributes-are-ignored-should-use-the-set-global-attribute",

server/service/appconfig.go

@@ -655,9 +655,6 @@ func validateSSOSettings(p fleet.AppConfig, existing *fleet.AppConfig, invalid *
 			if p.SSOSettings.EnableJITProvisioning {
 				invalid.Append("enable_jit_provisioning", ErrMissingLicense.Error())
 			}
-			if p.SSOSettings.EnableJITRoleSync {
-				invalid.Append("enable_jit_role_sync", ErrMissingLicense.Error())
-			}
 		}
 	}
 }

server/service/appconfig_test.go

@@ -421,27 +421,6 @@ func TestJITProvisioning(t *testing.T) {
 		assert.Contains(t, invalid.Error(), "missing or invalid license")
 	})
 
-	config = fleet.AppConfig{
-		SSOSettings: fleet.SSOSettings{
-			EnableSSO:         true,
-			EnableJITRoleSync: true,
-			SSOProviderSettings: fleet.SSOProviderSettings{
-				EntityID:    "fleet",
-				IssuerURI:   "http://issuer.idp.com",
-				IDPName:     "onelogin",
-				MetadataURL: "http://isser.metadata.com",
-			},
-		},
-	}
-
-	t.Run("doesn't allow to enable JIT role sync without a premium license", func(t *testing.T) {
-		invalid := &fleet.InvalidArgumentError{}
-		validateSSOSettings(config, &fleet.AppConfig{}, invalid, &fleet.LicenseInfo{})
-		require.True(t, invalid.HasErrors())
-		assert.Contains(t, invalid.Error(), "enable_jit_role_sync")
-		assert.Contains(t, invalid.Error(), "missing or invalid license")
-	})
-
 	t.Run("allows JIT provisioning to be enabled with a premium license", func(t *testing.T) {
 		invalid := &fleet.InvalidArgumentError{}
 		validateSSOSettings(config, &fleet.AppConfig{}, invalid, &fleet.LicenseInfo{Tier: fleet.TierPremium})
@@ -454,7 +433,6 @@ func TestJITProvisioning(t *testing.T) {
 
 		oldConfig.SSOSettings.EnableJITProvisioning = true
 		config.SSOSettings.EnableJITProvisioning = false
-		config.SSOSettings.EnableJITRoleSync = false
 		validateSSOSettings(config, oldConfig, invalid, &fleet.LicenseInfo{})
 		require.False(t, invalid.HasErrors())
 	})

server/service/integration_enterprise_test.go

@@ -1879,7 +1879,6 @@ func (s *integrationEnterpriseTestSuite) TestSSOJITProvisioning() {
 	s.DoJSON("GET", "/api/latest/fleet/config", nil, http.StatusOK, &acResp)
 	require.NotNil(t, acResp)
 	require.False(t, acResp.SSOSettings.EnableJITProvisioning)
-	require.False(t, acResp.SSOSettings.EnableJITRoleSync)
 
 	acResp = appConfigResponse{}
 	s.DoJSON("PATCH", "/api/latest/fleet/config", json.RawMessage(`{
@@ -1894,7 +1893,6 @@ func (s *integrationEnterpriseTestSuite) TestSSOJITProvisioning() {
 	}`), http.StatusOK, &acResp)
 	require.NotNil(t, acResp)
 	require.False(t, acResp.SSOSettings.EnableJITProvisioning)
-	require.False(t, acResp.SSOSettings.EnableJITRoleSync)
 
 	// users can't be created if SSO is disabled
 	auth, body := s.LoginSSOUser("sso_user", "user123#")
@@ -1904,6 +1902,8 @@ func (s *integrationEnterpriseTestSuite) TestSSOJITProvisioning() {
 	var nfe fleet.NotFoundError
 	require.ErrorAs(t, err, &nfe)
 
+	// If enable_jit_provisioning is enabled Roles won't be updated for existing SSO users.
+
 	// enable JIT provisioning
 	acResp = appConfigResponse{}
 	s.DoJSON("PATCH", "/api/latest/fleet/config", json.RawMessage(`{
@@ -1913,13 +1913,11 @@ func (s *integrationEnterpriseTestSuite) TestSSOJITProvisioning() {
 			"issuer_uri": "http://localhost:8080/simplesaml/saml2/idp/SSOService.php",
 			"idp_name": "SimpleSAML",
 			"metadata_url": "http://localhost:9080/simplesaml/saml2/idp/metadata.php",
-			"enable_jit_provisioning": true,
-			"enable_jit_role_sync": false
+			"enable_jit_provisioning": true
 		}
 	}`), http.StatusOK, &acResp)
 	require.NotNil(t, acResp)
 	require.True(t, acResp.SSOSettings.EnableJITProvisioning)
-	require.False(t, acResp.SSOSettings.EnableJITRoleSync)
 
 	// a new user is created and redirected accordingly
 	auth, body = s.LoginSSOUser("sso_user", "user123#")
@@ -1943,13 +1941,14 @@ func (s *integrationEnterpriseTestSuite) TestSSOJITProvisioning() {
 		return false
 	})
 
-	// Test that roles are not updated for an existing user because enable_jit_role_sync is false.
+	// Test that roles are not updated for an existing user when SSO attributes are not set.
 
 	// Change role to global admin first.
 	user.GlobalRole = ptr.String("admin")
 	err = s.ds.SaveUser(context.Background(), user)
 	require.NoError(t, err)
-	// Login should NOT change the role to the default (global observer).
+	// Login should NOT change the role to the default (global observer) because SSO attributes
+	// are not set for this user (see ../../tools/saml/users.php).
 	auth, body = s.LoginSSOUser("sso_user", "user123#")
 	assert.Equal(t, "sso_user@example.com", auth.UserID())
 	assert.Equal(t, "SSO User 1", auth.UserDisplayName())
@@ -1959,32 +1958,6 @@ func (s *integrationEnterpriseTestSuite) TestSSOJITProvisioning() {
 	require.NotNil(t, user.GlobalRole)
 	require.Equal(t, *user.GlobalRole, "admin")
 
-	// Test that roles are updated for an existing user because enable_jit_role_sync is true.
-	acResp = appConfigResponse{}
-	s.DoJSON("PATCH", "/api/latest/fleet/config", json.RawMessage(`{
-		"sso_settings": {
-			"enable_sso": true,
-			"entity_id": "https://localhost:8080",
-			"issuer_uri": "http://localhost:8080/simplesaml/saml2/idp/SSOService.php",
-			"idp_name": "SimpleSAML",
-			"metadata_url": "http://localhost:9080/simplesaml/saml2/idp/metadata.php",
-			"enable_jit_provisioning": true,
-			"enable_jit_role_sync": true
-		}
-	}`), http.StatusOK, &acResp)
-	require.NotNil(t, acResp)
-	require.True(t, acResp.SSOSettings.EnableJITProvisioning)
-	require.True(t, acResp.SSOSettings.EnableJITRoleSync)
-	// Login should change the role to the default role (global observer).
-	auth, body = s.LoginSSOUser("sso_user", "user123#")
-	assert.Equal(t, "sso_user@example.com", auth.UserID())
-	assert.Equal(t, "SSO User 1", auth.UserDisplayName())
-	require.Contains(t, body, "Redirecting to Fleet at  ...")
-	user, err = s.ds.UserByEmail(context.Background(), "sso_user@example.com")
-	require.NoError(t, err)
-	require.NotNil(t, user.GlobalRole)
-	require.Equal(t, *user.GlobalRole, "observer")
-
 	// A user with pre-configured roles can be created
 	// see `tools/saml/users.php` for details.
 	auth, body = s.LoginSSOUser("sso_user_3_global_admin", "user123#")
@@ -1998,6 +1971,26 @@ func (s *integrationEnterpriseTestSuite) TestSSOJITProvisioning() {
 	})
 	require.Contains(t, body, "Redirecting to Fleet at  ...")
 
+	// Test that roles are updated for an existing user when SSO attributes are set.
+
+	// Change role to global maintainer first.
+	user3, err := s.ds.UserByEmail(context.Background(), auth.UserID())
+	require.NoError(t, err)
+	require.Equal(t, auth.UserID(), user3.Email)
+	user3.GlobalRole = ptr.String("maintainer")
+	err = s.ds.SaveUser(context.Background(), user3)
+	require.NoError(t, err)
+
+	// Login should change the role to the configured role in the SSO attributes (global admin).
+	auth, body = s.LoginSSOUser("sso_user_3_global_admin", "user123#")
+	assert.Equal(t, "sso_user_3_global_admin@example.com", auth.UserID())
+	assert.Equal(t, "SSO User 3", auth.UserDisplayName())
+	require.Contains(t, body, "Redirecting to Fleet at  ...")
+	user3, err = s.ds.UserByEmail(context.Background(), "sso_user_3_global_admin@example.com")
+	require.NoError(t, err)
+	require.NotNil(t, user3.GlobalRole)
+	require.Equal(t, *user3.GlobalRole, "admin")
+
 	// We cannot use NewTeam and must use adhoc SQL because the teams.id is
 	// auto-incremented and other tests cause it to be different than what we need (ID=1).
 	var execErr error

server/service/sessions_test.go

@@ -229,7 +229,6 @@ func TestGetSSOUser(t *testing.T) {
 				EnableSSO:             true,
 				EnableSSOIdPLogin:     true,
 				EnableJITProvisioning: true,
-				EnableJITRoleSync:     true,
 			},
 		}, nil
 	}
@@ -316,7 +315,7 @@ func TestGetSSOUser(t *testing.T) {
 
 	require.True(t, ds.SaveUserFuncInvoked)
 
-	// (3) Test existing user's role is not changed after a new login if EnableJITRoleSync is false.
+	// (3) Test existing user's role is not changed after a new login if EnableJITProvisioning is false.
 
 	ds.SaveUserFuncInvoked = false
 
@@ -325,13 +324,11 @@ func TestGetSSOUser(t *testing.T) {
 			SSOSettings: fleet.SSOSettings{
 				EnableSSO:             true,
 				EnableSSOIdPLogin:     true,
-				EnableJITProvisioning: true,
-				EnableJITRoleSync:     false,
+				EnableJITProvisioning: false,
 			},
 		}, nil
 	}
 
-	// No configuration set for this user.
 	auth.assertionAttributes = []fleet.SAMLAttribute{
 		{
 			Name: "FLEET_JIT_USER_ROLE_TEAM_2",
@@ -354,7 +351,6 @@ func TestGetSSOUser(t *testing.T) {
 				EnableSSO:             true,
 				EnableSSOIdPLogin:     true,
 				EnableJITProvisioning: true,
-				EnableJITRoleSync:     true,
 			},
 		}, nil
 	}