Skip to content

Commit

Permalink
Add test for failure condition
Browse files Browse the repository at this point in the history
Signed-off-by: Craig Perkins <[email protected]>
  • Loading branch information
cwperks committed Feb 12, 2024
1 parent 778cb8c commit c34c821
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ private AuthTokenProcessorAction.Response handleImpl(
String samlRequestId,
String acsEndpoint,
Saml2Settings saml2Settings
) {
) throws Exception {
if (token_log.isDebugEnabled()) {
try {
token_log.debug(
Expand Down Expand Up @@ -172,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 @@ -236,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
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
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,7 +444,7 @@ public void testMetadataBody() throws Exception {
mockSamlIdpServer.loadSigningKeys("saml/kirk-keystore.jks", "kirk");
mockSamlIdpServer.setAuthenticateUser("horst");
mockSamlIdpServer.setEndpointQueryString(null);
mockSamlIdpServer.setAuthenticateUserRoles(Arrays.asList("Admin", "Developer"));
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 @@ -457,37 +459,48 @@ public void testMetadataBody() throws Exception {
.put("path.home", ".")
.build();

HTTPSamlAuthenticator samlAuthenticator = new HTTPSamlAuthenticator(settings, null);
SignedJWT jwt = getSignedJWT(settings);

AuthenticateHeaders authenticateHeaders = getAutenticateHeaders(samlAuthenticator);
Assert.assertEquals(List.of("HR", "IT"), jwt.getJWTClaimsSet().getClaim("roles"));
Assert.assertEquals("horst", jwt.getJWTClaimsSet().getClaim("sub"));
}

String encodedSamlResponse = mockSamlIdpServer.handleSsoGetRequestURI(authenticateHeaders.location);
@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"));

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");
// Note: We need to replace endpoint with mockSamlIdpServer endpoint
final String metadataBody = FileHelper.loadFile("saml/metadata.xml")
.replaceAll("http://localhost:33667/", mockSamlIdpServer.getMetadataUri());

Assert.assertNotNull("Expected authorization attribute in JSON: " + responseJson, authorization);
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 = SignedJWT.parse(authorization.replaceAll("\\s*bearer\\s*", ""));
SignedJWT jwt = getSignedJWT(settings);

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

@Test
public void testMetadataBodyAllowRepeatAttributeNames() throws Exception {
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("Admin", "Developer"));
mockSamlIdpServer.setAllowRepeatAttributeNames(true);
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")
Expand All @@ -498,11 +511,67 @@ public void testMetadataBodyAllowRepeatAttributeNames() throws Exception {
.put("kibana_url", "http://wherever")
.put("idp.entity_id", mockSamlIdpServer.getIdpEntityId())
.put("exchange_key", "abc")
.put("roles_key", "role")
.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 @@ -511,19 +580,7 @@ public void testMetadataBodyAllowRepeatAttributeNames() 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(List.of("Admin", "Developer"), jwt.getJWTClaimsSet().getClaim("roles"));
Assert.assertEquals("horst", jwt.getJWTClaimsSet().getClaim("sub"));
assertThat(responseJson, containsString("Error while validating SAML response"));
}

@Test(expected = RuntimeException.class)
Expand Down Expand Up @@ -900,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 @@ -931,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
46 changes: 25 additions & 21 deletions src/test/java/com/amazon/dlic/auth/http/saml/MockSamlIdpServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class MockSamlIdpServer implements Closeable {
private Credential signingCredential;
private String authenticateUser;
private List<String> authenticateUserRoles;
private boolean allowRepeatAttributes = false;
private List<String> authenticateUserRolesAsRepeatedAttributeName;
private int baseId = 1;
private boolean signResponses = true;
private X509Certificate spSignatureCertificate;
Expand Down Expand Up @@ -466,31 +466,31 @@ private String createSamlAuthResponse(AuthnRequest authnRequest) {
conditions.setNotOnOrAfter(Instant.now().plus(1, ChronoUnit.MINUTES));

if (authenticateUserRoles != null) {
if (!allowRepeatAttributes) {
AttributeStatement attributeStatement = createSamlElement(AttributeStatement.class);
assertion.getAttributeStatements().add(attributeStatement);

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

attribute.setName("roles");
attribute.setNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:basic");

for (String role : authenticateUserRoles) {
attribute.getAttributeValues().add(createXSAny(AttributeValue.DEFAULT_ELEMENT_NAME, role));
}
}

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("roles");
attribute.setName("role");
attribute.setNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:basic");

for (String role : authenticateUserRoles) {
attribute.getAttributeValues().add(createXSAny(AttributeValue.DEFAULT_ELEMENT_NAME, role));
}
} else {
for (String role : authenticateUserRoles) {
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));
}
attribute.getAttributeValues().add(createXSAny(AttributeValue.DEFAULT_ELEMENT_NAME, role));
}
}

Expand Down Expand Up @@ -1127,8 +1127,12 @@ public void setAuthenticateUserRoles(List<String> authenticateUserRoles) {
this.authenticateUserRoles = authenticateUserRoles;
}

public void setAllowRepeatAttributeNames(boolean allowRepeatAttributes) {
this.allowRepeatAttributes = allowRepeatAttributes;
public List<String> getAuthenticateUserRolesAsRepeatedAttributeName() {
return authenticateUserRolesAsRepeatedAttributeName;
}

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

public boolean isSignResponses() {
Expand Down

0 comments on commit c34c821

Please sign in to comment.