Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map only specific type of OIDC Claims (#58524) #59043

Merged
merged 1 commit into from
Jul 6, 2020
Merged

Map only specific type of OIDC Claims (#58524) #59043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions x-pack/docs/en/security/authentication/oidc-guide.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,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
<<oidc-user-properties, the listing>> below), to the name of the claims that your
OP will release. In the example above, we have configured the `principal` and
Expand Down
78 changes: 37 additions & 41 deletions ...ecurity/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -211,19 +210,13 @@ private void buildUserFromClaims(JWTClaimsSet claims, ActionListener<Authenticat
return;
}

final Map<String, Object> userMetadata = new HashMap<>();
final Map<String, Object> userMetadata;
if (populateUserMetadata) {
Map<String, Object> claimsMap = claims.getClaims();
/*
* We whitelist the Types that we want to parse as metadata from the Claims, explicitly filtering out {@link Date}s
*/
Set<Map.Entry> allowedEntries = claimsMap.entrySet().stream().filter(entry -> {
Object v = entry.getValue();
return (v instanceof String || v instanceof Boolean || v instanceof Number || v instanceof Collection);
}).collect(Collectors.toSet());
for (Map.Entry entry : allowedEntries) {
userMetadata.put("oidc(" + entry.getKey() + ")", entry.getValue());
}
userMetadata = claims.getClaims().entrySet().stream()
.filter(entry -> isAllowedTypeForClaim(entry.getValue()))
.collect(Collectors.toMap(entry -> "oidc(" + entry.getKey() + ")", Map.Entry::getValue));
} else {
userMetadata = Collections.emptyMap();
}
final List<String> groups = groupsAttribute.getClaimValues(claims);
final String dn = dnAttribute.getClaimValue(claims);
Expand Down Expand Up @@ -390,6 +383,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<JWTClaimsSet, List<String>> parser;
Expand Down Expand Up @@ -417,6 +419,22 @@ public String toString() {
return name;
}

private static Collection<String> parseClaimValues(JWTClaimsSet claimsSet, String claimName, String settingKey) {
Collection<String> values;
final Object claimValueObject = claimsSet.getClaim(claimName);
if (claimValueObject == null) {
values = Collections.emptyList();
} else if (claimValueObject instanceof String) {
values = Collections.singletonList((String) claimValueObject);
} else if (claimValueObject instanceof Collection &&
((Collection) claimValueObject).stream().allMatch(c -> c instanceof String)) {
values = (Collection<String>) 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) {

Expand All @@ -428,19 +446,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<String> values;
if (claimValueObject == null) {
values = Collections.emptyList();
} else if (claimValueObject instanceof String) {
values = Collections.singletonList((String) claimValueObject);
} else if (claimValueObject instanceof List) {
values = (List<String>) 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<String> 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);
Expand All @@ -464,21 +471,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 Collections.emptyList();
} else if (claimValueObject instanceof String) {
return Collections.singletonList((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<String>) claimValueObject).stream()
.filter(Objects::nonNull)
.collect(Collectors.toList());
});
claims -> parseClaimValues(claims, claimName, RealmSettings.getFullSettingKey(realmConfig, setting.getClaim()))
.stream()
.filter(Objects::nonNull)
.collect(Collectors.toList()));
}
} else if (required) {
throw new SettingsException("Setting [" + RealmSettings.getFullSettingKey(realmConfig, setting.getClaim())
Expand Down
89 changes: 80 additions & 9 deletions ...ty/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -84,7 +85,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));
Expand All @@ -103,6 +104,72 @@ public void testAuthentication() throws Exception {
}
}

public void testClaimPropertyMapping() throws Exception {
final UserRoleMapper roleMapper = mock(UserRoleMapper.class);
final String principal = randomAlphaOfLength(12);
AtomicReference<UserRoleMapper.UserData> userData = new AtomicReference<>();
doAnswer(invocation -> {
assert invocation.getArguments().length == 2;
userData.set((UserRoleMapper.UserData) invocation.getArguments()[0]);
ActionListener<Set<String>> listener = (ActionListener<Set<String>>) 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<String, Object> claimsWithObject = org.elasticsearch.common.collect.Map.of(
"groups", org.elasticsearch.common.collect.List.of(
org.elasticsearch.common.collect.Map.of("key1", org.elasticsearch.common.collect.List.of("value1", "value2")),
org.elasticsearch.common.collect.Map.of("key2", org.elasticsearch.common.collect.List.of("value1", "value2")))
);
Map<String, Object> claimsWithNumber = org.elasticsearch.common.collect.Map.of(
"groups", org.elasticsearch.common.collect.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<UserRoleMapper.UserData> userData = new AtomicReference<>();
doAnswer(invocation -> {
assert invocation.getArguments().length == 2;
userData.set((UserRoleMapper.UserData) invocation.getArguments()[0]);
ActionListener<Set<String>> listener = (ActionListener<Set<String>>) 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<String, Object> claims = org.elasticsearch.common.collect.Map.of(
"string", "String",
"number", 232,
"boolean", true,
"string_array", org.elasticsearch.common.collect.List.of("one", "two", "three"),
"number_array", org.elasticsearch.common.collect.List.of(1, 2, 3),
"boolean_array", org.elasticsearch.common.collect.List.of(true, false, true),
"object", org.elasticsearch.common.collect.Map.of("key", org.elasticsearch.common.collect.List.of("value1", "value2")),
"object_array", org.elasticsearch.common.collect.List.of(
org.elasticsearch.common.collect.Map.of("key1", org.elasticsearch.common.collect.List.of("value1", "value2")),
org.elasticsearch.common.collect.Map.of("key2", org.elasticsearch.common.collect.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("[email protected]"));
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);
Expand All @@ -113,7 +180,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));
Expand All @@ -131,7 +198,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));
}
Expand Down Expand Up @@ -333,8 +400,8 @@ public void testBuildingAuthenticationRequestWithLoginHint() {
}

private AuthenticationResult authenticateWithOidc(String principal, UserRoleMapper roleMapper, boolean notPopulateMetadata,
boolean useAuthorizingRealm
,String authenticatingRealm)
boolean useAuthorizingRealm, String authenticatingRealm,
@Nullable Map<String, Object> additionalClaims)
throws Exception {
final MockLookupRealm lookupRealm = new MockLookupRealm(
new RealmConfig(new RealmConfig.RealmIdentifier("mock", "mock_lookup"), globalSettings, env, threadContext));
Expand All @@ -355,7 +422,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)))
Expand All @@ -365,9 +432,13 @@ private AuthenticationResult authenticateWithOidc(String principal, UserRoleMapp
.claim("groups", Arrays.asList("group1", "group2", "groups3"))
.claim("mail", "[email protected]")
.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<String, Object> entry : additionalClaims.entrySet()) {
claimsBuilder.claim(entry.getKey(), entry.getValue());
}
}
final JWTClaimsSet claims = claimsBuilder.build();
doAnswer((i) -> {
ActionListener<JWTClaimsSet> listener = (ActionListener<JWTClaimsSet>) i.getArguments()[1];
listener.onResponse(claims);
Expand Down