Accept and ignore SSO role attributes with null value (#11959)

#10878 - [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)).~
2026-04-21 13:37:30 +00:00 · 2023-05-30 16:57:03 -03:00 · 2023-05-30 16:57:03 -03:00 · 6acb567ade
commit 6acb567ade
parent e361a502a9
7 changed files with 163 additions and 6 deletions
--- a/changes/10878-support-sso-role-attributes-with-null-value
+++ b/changes/10878-support-sso-role-attributes-with-null-value
@ -0,0 +1 @@
+* Support value `null` on `FLEET_JIT_USER_ROLE_GLOBAL` and `FLEET_JIT_USER_ROLE_TEAM_*` SAML attributes. Fleet will accept and ignore such `null` attributes.
--- a/docs/Contributing/Testing-and-local-development.md
+++ b/docs/Contributing/Testing-and-local-development.md
@ -40,6 +40,7 @@
      - [Testing DEP enrollment](#testing-dep-enrollment)
        - [Gating the DEP profile behind SSO](#gating-the-dep-profile-behind-sso)
    - [Nudge](#nudge)
+      - [Debugging tips](#debugging-tips)

 ## License key

@ -323,7 +324,6 @@ Configure SSO on the Organization Settings page with the following:
 ```
 Identity Provider Name: SimpleSAML
 Entity ID: https://localhost:8080
-Issuer URI: http://localhost:8080/simplesaml/saml2/idp/SSOService.php
 Metadata URL: http://localhost:9080/simplesaml/saml2/idp/metadata.php
 ```

--- a/docs/Deploying/Configuration.md
+++ b/docs/Deploying/Configuration.md
@ -3076,7 +3076,8 @@ 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.
 - `FLEET_JIT_USER_ROLE_TEAM_<TEAM_ID>`: Specifies team role for team with ID `<TEAM_ID>` to use when creating the user.

-Currently supported values for the above attributes are: `admin`, `maintainer`, `observer` and `observer_plus`.
+Currently supported values for the above attributes are: `admin`, `maintainer`, `observer`, `observer_plus` and `null`.
+A role attribute with value `null` will be ignored by Fleet. (This is to support limitations on some IdPs which do not allow you to choose what keys are sent to Fleet when creating a new user.)
 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.
--- a/server/fleet/sessions.go
+++ b/server/fleet/sessions.go
@ -114,6 +114,7 @@ func (s SSORolesInfo) isEmpty() bool {
 const (
 	globalUserRoleSSOAttrName     = "FLEET_JIT_USER_ROLE_GLOBAL"
 	teamUserRoleSSOAttrNamePrefix = "FLEET_JIT_USER_ROLE_TEAM_"
+	ssoAttrNullRoleValue          = "null"
 )

 // RolesFromSSOAttributes loads Global and Team roles from SAML custom attributes.
@ -121,7 +122,8 @@ const (
 //   - Custom attributes of the form `FLEET_JIT_USER_ROLE_TEAM_<TEAM_ID>` are used
 //     for setting role for a team with ID <TEAM_ID>.
 //
-// For both attributes currently supported values are `admin`, `maintainer` and `observer`
+// For both attributes currently supported values are `admin`, `maintainer`, `observer`,
+// `observer_plus` and `null`. A `null` value is used to ignore the attribute.
 func RolesFromSSOAttributes(attributes []SAMLAttribute) (SSORolesInfo, error) {
 	ssoRolesInfo := SSORolesInfo{}
 	for _, attribute := range attributes {
@ -131,6 +133,10 @@ func RolesFromSSOAttributes(attributes []SAMLAttribute) (SSORolesInfo, error) {
 			if err != nil {
 				return SSORolesInfo{}, fmt.Errorf("parse global role: %w", err)
 			}
+			if role == ssoAttrNullRoleValue {
+				// If the role is set to the null value then the attribute is ignored.
+				continue
+			}
 			ssoRolesInfo.Global = ptr.String(role)
 		case strings.HasPrefix(attribute.Name, teamUserRoleSSOAttrNamePrefix):
 			teamIDSuffix := strings.TrimPrefix(attribute.Name, teamUserRoleSSOAttrNamePrefix)
@ -142,6 +148,10 @@ func RolesFromSSOAttributes(attributes []SAMLAttribute) (SSORolesInfo, error) {
 			if err != nil {
 				return SSORolesInfo{}, fmt.Errorf("parse team role: %w", err)
 			}
+			if teamRole == ssoAttrNullRoleValue {
+				// If the role is set to the null value then the attribute is ignored.
+				continue
+			}
 			ssoRolesInfo.Teams = append(ssoRolesInfo.Teams, TeamRole{
 				ID:   uint(teamID),
 				Role: teamRole,
@ -170,7 +180,8 @@ func parseRole(values []SAMLAttributeValue) (string, error) {
 	if value != RoleAdmin &&
 		value != RoleMaintainer &&
 		value != RoleObserver &&
-		value != RoleObserverPlus {
+		value != RoleObserverPlus &&
+		value != ssoAttrNullRoleValue {
 		return "", fmt.Errorf("invalid role: %s", value)
 	}
 	return value, nil
--- a/server/fleet/sessions_test.go
+++ b/server/fleet/sessions_test.go
@ -251,6 +251,79 @@ func TestRolesFromSSOAttributes(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "null-value-on-team-attribute-is-ignored-should-use-default",
+			attributes: []SAMLAttribute{
+				{
+					Name: teamUserRoleSSOAttrNamePrefix + "1",
+					Values: []SAMLAttributeValue{
+						{Value: "null"},
+					},
+				},
+			},
+			shouldFail: false,
+			expectedSSORolesInfo: SSORolesInfo{
+				Global: ptr.String("observer"),
+			},
+		},
+		{
+			name: "null-attributes-are-ignored-should-use-default",
+			attributes: []SAMLAttribute{
+				{
+					Name: globalUserRoleSSOAttrName,
+					Values: []SAMLAttributeValue{
+						{Value: "null"},
+					},
+				},
+				{
+					Name: teamUserRoleSSOAttrNamePrefix + "2",
+					Values: []SAMLAttributeValue{
+						{Value: "null"},
+					},
+				},
+			},
+			shouldFail: false,
+			expectedSSORolesInfo: SSORolesInfo{
+				Global: ptr.String("observer"),
+			},
+		},
+		{
+			name: "null-attributes-are-ignored-should-use-the-set-global-attribute",
+			attributes: []SAMLAttribute{
+				{
+					Name: globalUserRoleSSOAttrName,
+					Values: []SAMLAttributeValue{
+						{Value: "admin"},
+					},
+				},
+				{
+					Name: globalUserRoleSSOAttrName,
+					Values: []SAMLAttributeValue{
+						{Value: "null"},
+					},
+				},
+				{
+					Name: teamUserRoleSSOAttrNamePrefix + "1",
+					Values: []SAMLAttributeValue{
+						{Value: "null"},
+					},
+				},
+			},
+			shouldFail: false,
+			expectedSSORolesInfo: SSORolesInfo{
+				Global: ptr.String("admin"),
+			},
+		},
+		{
+			name: "attribute-with-no-values-should-fail",
+			attributes: []SAMLAttribute{
+				{
+					Name:   globalUserRoleSSOAttrName,
+					Values: []SAMLAttributeValue{},
+				},
+			},
+			shouldFail: true,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			ssoRolesInfo, err := RolesFromSSOAttributes(tc.attributes)
--- a/server/service/integration_enterprise_test.go
+++ b/server/service/integration_enterprise_test.go
@ -2014,7 +2014,7 @@ func (s *integrationEnterpriseTestSuite) TestSSOJITProvisioning() {
 	})
 	require.NoError(t, err)

-	// A user with pre-configured roles can be created
+	// A user with pre-configured roles can be created,
 	// see `tools/saml/users.php` for details.
 	auth, body = s.LoginSSOUser("sso_user_4_team_maintainer", "user123#")
 	assert.Equal(t, "sso_user_4_team_maintainer@example.com", auth.UserID())
@ -2026,6 +2026,54 @@ func (s *integrationEnterpriseTestSuite) TestSSOJITProvisioning() {
 		}},
 	})
 	require.Contains(t, body, "Redirecting to Fleet at  ...")
+
+	// A user with pre-configured roles can be created,
+	// see `tools/saml/users.php` for details.
+	auth, body = s.LoginSSOUser("sso_user_5_team_admin", "user123#")
+	assert.Equal(t, "sso_user_5_team_admin@example.com", auth.UserID())
+	assert.Equal(t, "SSO User 5", auth.UserDisplayName())
+	assert.Contains(t, auth.AssertionAttributes(), fleet.SAMLAttribute{
+		Name: "FLEET_JIT_USER_ROLE_TEAM_1",
+		Values: []fleet.SAMLAttributeValue{{
+			Value: "admin",
+		}},
+	})
+	// FLEET_JIT_USER_ROLE_* attributes with value `null` are ignored by Fleet.
+	assert.Contains(t, auth.AssertionAttributes(), fleet.SAMLAttribute{
+		Name: "FLEET_JIT_USER_ROLE_GLOBAL",
+		Values: []fleet.SAMLAttributeValue{{
+			Value: "null",
+		}},
+	})
+	// FLEET_JIT_USER_ROLE_* attributes with value `null` are ignored by Fleet.
+	assert.Contains(t, auth.AssertionAttributes(), fleet.SAMLAttribute{
+		Name: "FLEET_JIT_USER_ROLE_TEAM_2",
+		Values: []fleet.SAMLAttributeValue{{
+			Value: "null",
+		}},
+	})
+	require.Contains(t, body, "Redirecting to Fleet at  ...")
+
+	// A user with pre-configured roles can be created,
+	// see `tools/saml/users.php` for details.
+	auth, body = s.LoginSSOUser("sso_user_6_global_observer", "user123#")
+	assert.Equal(t, "sso_user_6_global_observer@example.com", auth.UserID())
+	assert.Equal(t, "SSO User 6", auth.UserDisplayName())
+	// FLEET_JIT_USER_ROLE_* attributes with value `null` are ignored by Fleet.
+	assert.Contains(t, auth.AssertionAttributes(), fleet.SAMLAttribute{
+		Name: "FLEET_JIT_USER_ROLE_GLOBAL",
+		Values: []fleet.SAMLAttributeValue{{
+			Value: "null",
+		}},
+	})
+	// FLEET_JIT_USER_ROLE_* attributes with value `null` are ignored by Fleet.
+	assert.Contains(t, auth.AssertionAttributes(), fleet.SAMLAttribute{
+		Name: "FLEET_JIT_USER_ROLE_TEAM_1",
+		Values: []fleet.SAMLAttributeValue{{
+			Value: "null",
+		}},
+	})
+	require.Contains(t, body, "Redirecting to Fleet at  ...")
 }

 func (s *integrationEnterpriseTestSuite) TestDistributedReadWithFeatures() {
--- a/tools/saml/users.php
+++ b/tools/saml/users.php
@ -30,7 +30,7 @@ $config = array(
            'email' => 'sso_user_3_global_admin@example.com',
            'FLEET_JIT_USER_ROLE_GLOBAL' => 'admin',
        ),
-        // sso_team_user_3 has FLEET_JIT_USER_ROLE_TEAM_1 attribute to be added as maintainer
+        // sso_user_4_team_maintainer has FLEET_JIT_USER_ROLE_TEAM_1 attribute to be added as maintainer
        // of team with ID 1, its login will fail if team with ID 1 doesn't exist.
        'sso_user_4_team_maintainer:user123#' => array(
            'uid' => array('4'),
@ -39,6 +39,29 @@ $config = array(
            'email' => 'sso_user_4_team_maintainer@example.com',
            'FLEET_JIT_USER_ROLE_TEAM_1' => 'maintainer',
        ),
+        // sso_user_5_team_admin has FLEET_JIT_USER_ROLE_TEAM_1 attribute to be added as admin
+        // of team with ID 1, its login will fail if team with ID 1 doesn't exist.
+        // It also sets FLEET_JIT_USER_ROLE_GLOBAL and FLEET_JIT_USER_ROLE_TEAM_2 to `null` which means
+        // Fleet will ignore such fields.
+        'sso_user_5_team_admin:user123#' => array(
+            'uid' => array('5'),
+            'eduPersonAffiliation' => array('group1'),
+            'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name' => array('SSO User 5'),
+            'email' => 'sso_user_5_team_admin@example.com',
+            'FLEET_JIT_USER_ROLE_TEAM_1' => 'admin',
+            'FLEET_JIT_USER_ROLE_GLOBAL' => 'null',
+            'FLEET_JIT_USER_ROLE_TEAM_2' => 'null',
+        ),
+        // sso_user_6_global_observer has all FLEET_JIT_USER_ROLE_* attributes set to null, so it
+        // will be added as global observer (default).
+        'sso_user_6_global_observer:user123#' => array(
+            'uid' => array('6'),
+            'eduPersonAffiliation' => array('group1'),
+            'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name' => array('SSO User 6'),
+            'email' => 'sso_user_6_global_observer@example.com',
+            'FLEET_JIT_USER_ROLE_GLOBAL' => 'null',
+            'FLEET_JIT_USER_ROLE_TEAM_1' => 'null',
+        ),
    ),

 );
				`@ -0,0 +1 @@`
				* Support value `null` on `FLEET_JIT_USER_ROLE_GLOBAL` and `FLEET_JIT_USER_ROLE_TEAM_*` SAML attributes. Fleet will accept and ignore such `null` attributes.