diff --git a/changes/10878-support-sso-role-attributes-with-null-value b/changes/10878-support-sso-role-attributes-with-null-value new file mode 100644 index 0000000000..e949aa315d --- /dev/null +++ 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. diff --git a/docs/Contributing/Testing-and-local-development.md b/docs/Contributing/Testing-and-local-development.md index b7f4121d8e..6b59bb4413 100644 --- 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 ``` diff --git a/docs/Deploying/Configuration.md b/docs/Deploying/Configuration.md index 890fd3f327..f4f143ef8d 100644 --- 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_`: Specifies team role for team with 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_` will cause an error during login as Fleet users cannot be Global users and belong to teams. diff --git a/server/fleet/sessions.go b/server/fleet/sessions.go index 04693aed81..de07b1da76 100644 --- 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_` are used // for setting role for a team with 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 diff --git a/server/fleet/sessions_test.go b/server/fleet/sessions_test.go index 33867bb137..5bc909897c 100644 --- 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) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index e32d868523..baa5a18f9f 100644 --- 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() { diff --git a/tools/saml/users.php b/tools/saml/users.php index f9052b9417..29dec878c1 100644 --- 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', + ), ), );