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

Jump to bottom

Add SAML authenticator setting to allow repeat attribute names #21

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add SAML authenticator setting to allow repeat attribute names #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
778cb8c
Add SAML authenticator setting to allow repeat attribute names
cwperks Feb 9, 2024
c34c821
Add test for failure condition
cwperks Feb 12, 2024
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
18 changes: 15 additions & 3 deletions src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class AuthTokenProcessorHandler {
private String jwtRolesKey;
private String samlSubjectKey;
private String samlRolesKey;
private boolean allowRepeatAttributeName;
private String kibanaRootUrl;

private long expiryOffset = 0;
Expand All @@ -88,6 +89,7 @@ class AuthTokenProcessorHandler {

this.jwtRolesKey = jwtSettings.get("roles_key", "roles");
this.jwtSubjectKey = jwtSettings.get("subject_key", "sub");
this.allowRepeatAttributeName = settings.getAsBoolean("allow_repeat_attribute_names", false);

this.samlRolesKey = settings.get("roles_key");
this.samlSubjectKey = settings.get("subject_key");
Expand Down Expand Up @@ -138,7 +140,7 @@ private AuthTokenProcessorAction.Response handleImpl(
String samlRequestId,
String acsEndpoint,
Saml2Settings saml2Settings
) {
) throws Exception {
if (token_log.isDebugEnabled()) {
try {
token_log.debug(
Expand All @@ -151,6 +153,10 @@ private AuthTokenProcessorAction.Response handleImpl(
}
}

if (allowRepeatAttributeName) {
saml2Settings.setAllowRepeatAttributeName(allowRepeatAttributeName);
}

try {

final SamlResponse samlResponse = new SamlResponse(saml2Settings, acsEndpoint, samlResponseBase64);
Expand All @@ -166,10 +172,10 @@ private AuthTokenProcessorAction.Response handleImpl(
return responseBody;
} catch (ValidationError e) {
log.warn("Error while validating SAML response", e);
return null;
throw e;
} catch (Exception e) {
log.error("Error while converting SAML to JWT", e);
return null;
throw e;
}
}

Expand Down Expand Up @@ -230,6 +236,12 @@ private Optional<SecurityResponse> handleLowLevel(RestRequest restRequest) throw
} catch (JsonProcessingException e) {
log.warn("Error while parsing JSON for /_opendistro/_security/api/authtoken", e);
return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, "JSON could not be parsed"));
} catch (ValidationError e) {
log.warn("Error while validating SAML response", e);
return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, "Error while validating SAML response"));
} catch (Exception e) {
log.warn("Error while converting SAML to JWT", e);
return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, "Error while converting SAML to JWT"));
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest
final String suffix = matcher.matches() ? matcher.group(2) : null;

if (API_AUTHTOKEN_SUFFIX.equals(suffix)) {
// Verficiation of SAML ASC endpoint only works with RestRequests
// Verification of SAML ASC endpoint only works with RestRequests
if (!(request instanceof OpenSearchRequest)) {
throw new SecurityRequestChannelUnsupported(
API_AUTHTOKEN_SUFFIX + " not supported for request of type " + request.getClass().getName()
Expand All @@ -208,8 +208,7 @@ public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest
if (e instanceof SecurityRequestChannelUnsupported) {
throw (SecurityRequestChannelUnsupported) e;
}
log.error("Error in reRequestAuthentication()", e);
return Optional.empty();
return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, e.getMessage()));
}
}

Expand Down
157 changes: 144 additions & 13 deletions src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@
import org.opensearch.security.util.FakeRestRequest;

import com.nimbusds.jwt.SignedJWT;
import com.onelogin.saml2.exception.ValidationError;
import org.opensaml.saml.saml2.core.NameIDType;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator.IDP_METADATA_CONTENT;
import static com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator.IDP_METADATA_URL;

Expand Down Expand Up @@ -442,6 +444,7 @@ public void testMetadataBody() throws Exception {
mockSamlIdpServer.loadSigningKeys("saml/kirk-keystore.jks", "kirk");
mockSamlIdpServer.setAuthenticateUser("horst");
mockSamlIdpServer.setEndpointQueryString(null);
mockSamlIdpServer.setAuthenticateUserRoles(Arrays.asList("HR", "IT"));

// Note: We need to replace endpoint with mockSamlIdpServer endpoint
final String metadataBody = FileHelper.loadFile("saml/metadata.xml")
Expand All @@ -456,6 +459,119 @@ public void testMetadataBody() throws Exception {
.put("path.home", ".")
.build();

SignedJWT jwt = getSignedJWT(settings);

Assert.assertEquals(List.of("HR", "IT"), jwt.getJWTClaimsSet().getClaim("roles"));
Assert.assertEquals("horst", jwt.getJWTClaimsSet().getClaim("sub"));
}

@Test
public void testRolesWithRepeatAttributeNames() throws Exception {
mockSamlIdpServer.setSignResponses(true);
mockSamlIdpServer.loadSigningKeys("saml/kirk-keystore.jks", "kirk");
mockSamlIdpServer.setAuthenticateUser("horst");
mockSamlIdpServer.setEndpointQueryString(null);
mockSamlIdpServer.setAuthenticateUserRolesAsRepeatedAttributeName(Arrays.asList("Admin", "Developer"));

// Note: We need to replace endpoint with mockSamlIdpServer endpoint
final String metadataBody = FileHelper.loadFile("saml/metadata.xml")
.replaceAll("http://localhost:33667/", mockSamlIdpServer.getMetadataUri());

Settings settings = Settings.builder()
.put(IDP_METADATA_CONTENT, metadataBody)
.put("kibana_url", "http://wherever")
.put("idp.entity_id", mockSamlIdpServer.getIdpEntityId())
.put("exchange_key", "abc")
.put("roles_key", "role")
.put("allow_repeat_attribute_names", true)
.put("path.home", ".")
.build();

SignedJWT jwt = getSignedJWT(settings);

Assert.assertEquals(List.of("Admin", "Developer"), jwt.getJWTClaimsSet().getClaim("roles"));
Assert.assertEquals("horst", jwt.getJWTClaimsSet().getClaim("sub"));
}

@Test
public void testRolesWithMixOfRepeatAttributeNamesAndListOfAttributesWithSingleName_rolesKeyRepeated() throws Exception {
mockSamlIdpServer.setSignResponses(true);
mockSamlIdpServer.loadSigningKeys("saml/kirk-keystore.jks", "kirk");
mockSamlIdpServer.setAuthenticateUser("horst");
mockSamlIdpServer.setEndpointQueryString(null);
mockSamlIdpServer.setAuthenticateUserRoles(Arrays.asList("HR", "IT"));
mockSamlIdpServer.setAuthenticateUserRolesAsRepeatedAttributeName(Arrays.asList("Admin", "Developer"));

// Note: We need to replace endpoint with mockSamlIdpServer endpoint
final String metadataBody = FileHelper.loadFile("saml/metadata.xml")
.replaceAll("http://localhost:33667/", mockSamlIdpServer.getMetadataUri());

Settings settings = Settings.builder()
.put(IDP_METADATA_CONTENT, metadataBody)
.put("kibana_url", "http://wherever")
.put("idp.entity_id", mockSamlIdpServer.getIdpEntityId())
.put("exchange_key", "abc")
.put("roles_key", "role") // "role" will extract roles using the repeated attribute names
.put("allow_repeat_attribute_names", true)
.put("path.home", ".")
.build();

SignedJWT jwt = getSignedJWT(settings);

Assert.assertEquals(List.of("Admin", "Developer"), jwt.getJWTClaimsSet().getClaim("roles"));
Assert.assertEquals("horst", jwt.getJWTClaimsSet().getClaim("sub"));
}

@Test
public void testRolesWithMixOfRepeatAttributeNamesAndListOfAttributesWithSingleName__rolesKeySingular() throws Exception {
mockSamlIdpServer.setSignResponses(true);
mockSamlIdpServer.loadSigningKeys("saml/kirk-keystore.jks", "kirk");
mockSamlIdpServer.setAuthenticateUser("horst");
mockSamlIdpServer.setEndpointQueryString(null);
mockSamlIdpServer.setAuthenticateUserRoles(Arrays.asList("HR", "IT"));
mockSamlIdpServer.setAuthenticateUserRolesAsRepeatedAttributeName(Arrays.asList("Admin", "Developer"));

// Note: We need to replace endpoint with mockSamlIdpServer endpoint
final String metadataBody = FileHelper.loadFile("saml/metadata.xml")
.replaceAll("http://localhost:33667/", mockSamlIdpServer.getMetadataUri());

Settings settings = Settings.builder()
.put(IDP_METADATA_CONTENT, metadataBody)
.put("kibana_url", "http://wherever")
.put("idp.entity_id", mockSamlIdpServer.getIdpEntityId())
.put("exchange_key", "abc")
.put("roles_key", "roles") // "role" will extract roles using the repeated attribute names
.put("allow_repeat_attribute_names", true)
.put("path.home", ".")
.build();

SignedJWT jwt = getSignedJWT(settings);

Assert.assertEquals(List.of("HR", "IT"), jwt.getJWTClaimsSet().getClaim("roles"));
Assert.assertEquals("horst", jwt.getJWTClaimsSet().getClaim("sub"));
}

@Test
public void testThrowsExceptionWhenRepeatAttributeNamesForbidden() throws Exception {
mockSamlIdpServer.setSignResponses(true);
mockSamlIdpServer.loadSigningKeys("saml/kirk-keystore.jks", "kirk");
mockSamlIdpServer.setAuthenticateUser("horst");
mockSamlIdpServer.setEndpointQueryString(null);
mockSamlIdpServer.setAuthenticateUserRolesAsRepeatedAttributeName(Arrays.asList("Admin", "Developer"));

// Note: We need to replace endpoint with mockSamlIdpServer endpoint
final String metadataBody = FileHelper.loadFile("saml/metadata.xml")
.replaceAll("http://localhost:33667/", mockSamlIdpServer.getMetadataUri());

Settings settings = Settings.builder()
.put(IDP_METADATA_CONTENT, metadataBody)
.put("kibana_url", "http://wherever")
.put("idp.entity_id", mockSamlIdpServer.getIdpEntityId())
.put("exchange_key", "abc")
.put("roles_key", "role")
.put("path.home", ".")
.build();

HTTPSamlAuthenticator samlAuthenticator = new HTTPSamlAuthenticator(settings, null);

AuthenticateHeaders authenticateHeaders = getAutenticateHeaders(samlAuthenticator);
Expand All @@ -464,18 +580,7 @@ public void testMetadataBody() throws Exception {

RestRequest tokenRestRequest = buildTokenExchangeRestRequest(encodedSamlResponse, authenticateHeaders);
String responseJson = getResponse(samlAuthenticator, tokenRestRequest);
HashMap<String, Object> response = DefaultObjectMapper.objectMapper.readValue(
responseJson,
new TypeReference<HashMap<String, Object>>() {
}
);
String authorization = (String) response.get("authorization");

Assert.assertNotNull("Expected authorization attribute in JSON: " + responseJson, authorization);

SignedJWT jwt = SignedJWT.parse(authorization.replaceAll("\\s*bearer\\s*", ""));

Assert.assertEquals("horst", jwt.getJWTClaimsSet().getClaim("sub"));
assertThat(responseJson, containsString("Error while validating SAML response"));
}

@Test(expected = RuntimeException.class)
Expand Down Expand Up @@ -852,7 +957,8 @@ public void initialConnectionFailureTest() throws Exception {
RestRequest restRequest = new FakeRestRequest(ImmutableMap.of(), new HashMap<String, String>());
Optional<SecurityResponse> maybeResponse = sendToAuthenticator(samlAuthenticator, restRequest);

assertThat(maybeResponse.isPresent(), Matchers.equalTo(false));
assertThat(maybeResponse.isPresent(), Matchers.equalTo(true));
assertThat(maybeResponse.get().getBody(), containsString("Could not find entity descriptor for http://test.entity"));

mockSamlIdpServer.start();

Expand Down Expand Up @@ -883,6 +989,31 @@ public void initialConnectionFailureTest() throws Exception {
}
}

private SignedJWT getSignedJWT(Settings samlAuthenticatorSettings) throws Exception {
HTTPSamlAuthenticator samlAuthenticator = new HTTPSamlAuthenticator(samlAuthenticatorSettings, null);

AuthenticateHeaders authenticateHeaders = getAutenticateHeaders(samlAuthenticator);

String encodedSamlResponse = mockSamlIdpServer.handleSsoGetRequestURI(authenticateHeaders.location);

RestRequest tokenRestRequest = buildTokenExchangeRestRequest(encodedSamlResponse, authenticateHeaders);
String responseJson = getResponse(samlAuthenticator, tokenRestRequest);

if (responseJson.contains("Error while validating SAML response")) {
throw new ValidationError("Found an Attribute element with duplicated Name", 0);
}
HashMap<String, Object> response = DefaultObjectMapper.objectMapper.readValue(
responseJson,
new TypeReference<HashMap<String, Object>>() {
}
);
String authorization = (String) response.get("authorization");

Assert.assertNotNull("Expected authorization attribute in JSON: " + responseJson, authorization);

return SignedJWT.parse(authorization.replaceAll("\\s*bearer\\s*", ""));
}

private AuthenticateHeaders getAutenticateHeaders(HTTPSamlAuthenticator samlAuthenticator) {
RestRequest restRequest = new FakeRestRequest(ImmutableMap.of(), new HashMap<String, String>());
SecurityResponse response = sendToAuthenticator(samlAuthenticator, restRequest).orElseThrow();
Expand Down
23 changes: 23 additions & 0 deletions src/test/java/com/amazon/dlic/auth/http/saml/MockSamlIdpServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ class MockSamlIdpServer implements Closeable {
private Credential signingCredential;
private String authenticateUser;
private List<String> authenticateUserRoles;
private List<String> authenticateUserRolesAsRepeatedAttributeName;
private int baseId = 1;
private boolean signResponses = true;
private X509Certificate spSignatureCertificate;
Expand Down Expand Up @@ -479,6 +480,20 @@ private String createSamlAuthResponse(AuthnRequest authnRequest) {
}
}

if (authenticateUserRolesAsRepeatedAttributeName != null) {
for (String role : authenticateUserRolesAsRepeatedAttributeName) {
AttributeStatement attributeStatement = createSamlElement(AttributeStatement.class);
assertion.getAttributeStatements().add(attributeStatement);

Attribute attribute = createSamlElement(Attribute.class);
attributeStatement.getAttributes().add(attribute);

attribute.setName("role");
attribute.setNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:basic");
attribute.getAttributeValues().add(createXSAny(AttributeValue.DEFAULT_ELEMENT_NAME, role));
}
}

if (signResponses) {
Signature signature = createSamlElement(Signature.class);
assertion.setSignature(signature);
Expand Down Expand Up @@ -1112,6 +1127,14 @@ public void setAuthenticateUserRoles(List<String> authenticateUserRoles) {
this.authenticateUserRoles = authenticateUserRoles;
}

public List<String> getAuthenticateUserRolesAsRepeatedAttributeName() {
return authenticateUserRolesAsRepeatedAttributeName;
}

public void setAuthenticateUserRolesAsRepeatedAttributeName(List<String> authenticateUserRolesAsRepeatedAttributeName) {
this.authenticateUserRolesAsRepeatedAttributeName = authenticateUserRolesAsRepeatedAttributeName;
}

public boolean isSignResponses() {
return signResponses;
}
Expand Down
Loading