diff --git a/x-pack/docs/en/security/authentication/oidc-guide.asciidoc b/x-pack/docs/en/security/authentication/oidc-guide.asciidoc index 2d063b6385deb..2683388d10725 100644 --- a/x-pack/docs/en/security/authentication/oidc-guide.asciidoc +++ b/x-pack/docs/en/security/authentication/oidc-guide.asciidoc @@ -258,6 +258,9 @@ The recommended steps for configuring OpenID Claims mapping are as follows: the errors, you should only request scopes that the OP supports, and which you intend to map to {es} user properties. + NOTE: You can only map claims with values that are strings, numbers, boolean values or an array + of the aforementioned. + . Configure the OpenID Connect realm in {es} to associate the {es} user properties (see <> below), to the name of the claims that your OP will release. In the example above, we have configured the `principal` and diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java index 2189c629a5059..af347f5dfb801 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java @@ -210,13 +210,9 @@ private void buildUserFromClaims(JWTClaimsSet claims, ActionListener userMetadata; if (populateUserMetadata) { - userMetadata = claims.getClaims().entrySet().stream().filter(entry -> { - /* - * We whitelist the Types that we want to parse as metadata from the Claims, explicitly filtering out {@link Date}s - */ - Object v = entry.getValue(); - return (v instanceof String || v instanceof Boolean || v instanceof Number || v instanceof Collection); - }).collect(Collectors.toUnmodifiableMap(entry -> "oidc(" + entry.getKey() + ")", Map.Entry::getValue)); + userMetadata = claims.getClaims().entrySet().stream() + .filter(entry -> isAllowedTypeForClaim(entry.getValue())) + .collect(Collectors.toUnmodifiableMap(entry -> "oidc(" + entry.getKey() + ")", Map.Entry::getValue)); } else { userMetadata = Map.of(); } @@ -385,6 +381,15 @@ public void close() { openIdConnectAuthenticator.close(); } + /* + * We only map claims that are of Type String, Boolean, or Number, or arrays that contain only these types + */ + private static boolean isAllowedTypeForClaim(Object o) { + return (o instanceof String || o instanceof Boolean || o instanceof Number + || (o instanceof Collection && ((Collection) o).stream() + .allMatch(c -> c instanceof String || c instanceof Boolean || c instanceof Number))); + } + static final class ClaimParser { private final String name; private final Function> parser; @@ -412,6 +417,22 @@ public String toString() { return name; } + private static Collection parseClaimValues(JWTClaimsSet claimsSet, String claimName, String settingKey) { + Collection values; + final Object claimValueObject = claimsSet.getClaim(claimName); + if (claimValueObject == null) { + values = List.of(); + } else if (claimValueObject instanceof String) { + values = List.of((String) claimValueObject); + } else if (claimValueObject instanceof Collection && + ((Collection) claimValueObject).stream().allMatch(c -> c instanceof String)) { + values = (Collection) claimValueObject; + } else { + throw new SettingsException("Setting [ " + settingKey + " expects a claim with String or a String Array value"); + } + return values; + } + static ClaimParser forSetting(Logger logger, OpenIdConnectRealmSettings.ClaimSetting setting, RealmConfig realmConfig, boolean required) { @@ -423,19 +444,8 @@ static ClaimParser forSetting(Logger logger, OpenIdConnectRealmSettings.ClaimSet "OpenID Connect Claim [" + claimName + "] with pattern [" + regex.pattern() + "] for [" + setting.name(realmConfig) + "]", claims -> { - Object claimValueObject = claims.getClaim(claimName); - List values; - if (claimValueObject == null) { - values = List.of(); - } else if (claimValueObject instanceof String) { - values = List.of((String) claimValueObject); - } else if (claimValueObject instanceof List) { - values = (List) claimValueObject; - } else { - throw new SettingsException("Setting [" + RealmSettings.getFullSettingKey(realmConfig, setting.getClaim()) - + " expects a claim with String or a String Array value but found a " - + claimValueObject.getClass().getName()); - } + Collection values = + parseClaimValues(claims, claimName, RealmSettings.getFullSettingKey(realmConfig, setting.getClaim())); return values.stream().map(s -> { if (s == null) { logger.debug("OpenID Connect Claim [{}] is null", claimName); @@ -459,21 +469,10 @@ static ClaimParser forSetting(Logger logger, OpenIdConnectRealmSettings.ClaimSet } else { return new ClaimParser( "OpenID Connect Claim [" + claimName + "] for [" + setting.name(realmConfig) + "]", - claims -> { - Object claimValueObject = claims.getClaim(claimName); - if (claimValueObject == null) { - return List.of(); - } else if (claimValueObject instanceof String) { - return List.of((String) claimValueObject); - } else if (claimValueObject instanceof List == false) { - throw new SettingsException("Setting [" + RealmSettings.getFullSettingKey(realmConfig, setting.getClaim()) - + " expects a claim with String or a String Array value but found a " - + claimValueObject.getClass().getName()); - } - return ((List) claimValueObject).stream() - .filter(Objects::nonNull) - .collect(Collectors.toUnmodifiableList()); - }); + claims -> parseClaimValues(claims, claimName, RealmSettings.getFullSettingKey(realmConfig, setting.getClaim())) + .stream() + .filter(Objects::nonNull) + .collect(Collectors.toUnmodifiableList())); } } else if (required) { throw new SettingsException("Setting [" + RealmSettings.getFullSettingKey(realmConfig, setting.getClaim()) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java index 257ab6c226ae7..1f8c291e20005 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java @@ -11,6 +11,7 @@ import com.nimbusds.openid.connect.sdk.Nonce; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.PlainActionFuture; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.env.Environment; @@ -85,7 +86,7 @@ public void testAuthentication() throws Exception { final boolean notPopulateMetadata = randomBoolean(); final String authenticatingRealm = randomBoolean() ? REALM_NAME : null; - AuthenticationResult result = authenticateWithOidc(principal, roleMapper, notPopulateMetadata, false, authenticatingRealm); + AuthenticationResult result = authenticateWithOidc(principal, roleMapper, notPopulateMetadata, false, authenticatingRealm, null); assertThat(result, notNullValue()); assertThat(result.getStatus(), equalTo(AuthenticationResult.Status.SUCCESS)); assertThat(result.getUser().principal(), equalTo(principal)); @@ -104,6 +105,68 @@ public void testAuthentication() throws Exception { } } + public void testClaimPropertyMapping() throws Exception { + final UserRoleMapper roleMapper = mock(UserRoleMapper.class); + final String principal = randomAlphaOfLength(12); + AtomicReference userData = new AtomicReference<>(); + doAnswer(invocation -> { + assert invocation.getArguments().length == 2; + userData.set((UserRoleMapper.UserData) invocation.getArguments()[0]); + ActionListener> listener = (ActionListener>) invocation.getArguments()[1]; + listener.onResponse(new HashSet<>(Arrays.asList("kibana_user", "role1"))); + return null; + }).when(roleMapper).resolveRoles(any(UserRoleMapper.UserData.class), any(ActionListener.class)); + Map claimsWithObject = Map.of( + "groups", List.of(Map.of("key1", List.of("value1", "value2")), Map.of("key2", List.of("value1", "value2"))) + ); + Map claimsWithNumber = Map.of( + "groups", List.of(2, "value2")); + Exception e = expectThrows(Exception.class, () -> authenticateWithOidc(principal, roleMapper, false, false, + REALM_NAME, claimsWithObject)); + Exception e2 = expectThrows(Exception.class, () -> authenticateWithOidc(principal, roleMapper, false, false, + REALM_NAME, claimsWithNumber)); + assertThat(e.getCause().getMessage(), containsString("expects a claim with String or a String Array value")); + assertThat(e2.getCause().getMessage(), containsString("expects a claim with String or a String Array value")); + } + + public void testClaimMetadataMapping() throws Exception { + final UserRoleMapper roleMapper = mock(UserRoleMapper.class); + final String principal = randomAlphaOfLength(12); + AtomicReference userData = new AtomicReference<>(); + doAnswer(invocation -> { + assert invocation.getArguments().length == 2; + userData.set((UserRoleMapper.UserData) invocation.getArguments()[0]); + ActionListener> listener = (ActionListener>) invocation.getArguments()[1]; + listener.onResponse(new HashSet<>(Arrays.asList("kibana_user", "role1"))); + return null; + }).when(roleMapper).resolveRoles(any(UserRoleMapper.UserData.class), any(ActionListener.class)); + Map claims = Map.of( + "string", "String", + "number", 232, + "boolean", true, + "string_array", List.of("one", "two", "three"), + "number_array", List.of(1, 2, 3), + "boolean_array", List.of(true, false, true), + "object", Map.of("key", List.of("value1", "value2")), + "object_array", List.of(Map.of("key1", List.of("value1", "value2")), Map.of("key2", List.of("value1", "value2"))) + ); + AuthenticationResult result = authenticateWithOidc(principal, roleMapper, false, false, REALM_NAME, claims); + assertThat(result, notNullValue()); + assertThat(result.getStatus(), equalTo(AuthenticationResult.Status.SUCCESS)); + assertThat(result.getUser().principal(), equalTo(principal)); + assertThat(result.getUser().email(), equalTo("cbarton@shield.gov")); + assertThat(result.getUser().fullName(), equalTo("Clinton Barton")); + assertThat(result.getUser().roles(), arrayContainingInAnyOrder("kibana_user", "role1")); + assertTrue(result.getUser().metadata().containsKey("oidc(string)")); + assertTrue(result.getUser().metadata().containsKey("oidc(number)")); + assertTrue(result.getUser().metadata().containsKey("oidc(boolean)")); + assertTrue(result.getUser().metadata().containsKey("oidc(string_array)")); + assertTrue(result.getUser().metadata().containsKey("oidc(boolean_array)")); + assertTrue(result.getUser().metadata().containsKey("oidc(number_array)")); + assertFalse(result.getUser().metadata().containsKey("oidc(object_array)")); + assertFalse(result.getUser().metadata().containsKey("oidc(object)")); + } + public void testWithAuthorizingRealm() throws Exception { final UserRoleMapper roleMapper = mock(UserRoleMapper.class); final String principal = randomAlphaOfLength(12); @@ -114,7 +177,7 @@ public void testWithAuthorizingRealm() throws Exception { return null; }).when(roleMapper).resolveRoles(any(UserRoleMapper.UserData.class), any(ActionListener.class)); final String authenticatingRealm = randomBoolean() ? REALM_NAME : null; - AuthenticationResult result = authenticateWithOidc(principal, roleMapper, randomBoolean(), true, authenticatingRealm); + AuthenticationResult result = authenticateWithOidc(principal, roleMapper, randomBoolean(), true, authenticatingRealm, null); assertThat(result, notNullValue()); assertThat(result.getStatus(), equalTo(AuthenticationResult.Status.SUCCESS)); assertThat(result.getUser().principal(), equalTo(principal)); @@ -132,7 +195,7 @@ public void testWithAuthorizingRealm() throws Exception { public void testAuthenticationWithWrongRealm() throws Exception{ final String principal = randomAlphaOfLength(12); AuthenticationResult result = authenticateWithOidc(principal, mock(UserRoleMapper.class), randomBoolean(), true, - REALM_NAME+randomAlphaOfLength(8)); + REALM_NAME + randomAlphaOfLength(8), null); assertThat(result, notNullValue()); assertThat(result.getStatus(), equalTo(AuthenticationResult.Status.CONTINUE)); } @@ -334,8 +397,8 @@ public void testBuildingAuthenticationRequestWithLoginHint() { } private AuthenticationResult authenticateWithOidc(String principal, UserRoleMapper roleMapper, boolean notPopulateMetadata, - boolean useAuthorizingRealm - ,String authenticatingRealm) + boolean useAuthorizingRealm, String authenticatingRealm, + @Nullable Map additionalClaims) throws Exception { RealmConfig.RealmIdentifier realmIdentifier = new RealmConfig.RealmIdentifier("mock", "mock_lookup"); final MockLookupRealm lookupRealm = new MockLookupRealm( @@ -360,7 +423,7 @@ private AuthenticationResult authenticateWithOidc(String principal, UserRoleMapp final OpenIdConnectRealm realm = new OpenIdConnectRealm(config, authenticator, roleMapper); initializeRealms(realm, lookupRealm); final OpenIdConnectToken token = new OpenIdConnectToken("", new State(), new Nonce(), authenticatingRealm); - final JWTClaimsSet claims = new JWTClaimsSet.Builder() + final JWTClaimsSet.Builder claimsBuilder = new JWTClaimsSet.Builder() .subject(principal) .audience("https://rp.elastic.co/cb") .expirationTime(Date.from(now().plusSeconds(3600))) @@ -370,9 +433,13 @@ private AuthenticationResult authenticateWithOidc(String principal, UserRoleMapp .claim("groups", Arrays.asList("group1", "group2", "groups3")) .claim("mail", "cbarton@shield.gov") .claim("name", "Clinton Barton") - .claim("id_token_hint", "thisis.aserialized.jwt") - .build(); - + .claim("id_token_hint", "thisis.aserialized.jwt"); + if (additionalClaims != null) { + for (Map.Entry entry : additionalClaims.entrySet()) { + claimsBuilder.claim(entry.getKey(), entry.getValue()); + } + } + final JWTClaimsSet claims = claimsBuilder.build(); doAnswer((i) -> { ActionListener listener = (ActionListener) i.getArguments()[1]; listener.onResponse(claims);