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.

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

fix: restrict colon in group #205

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ public class CustomAWSSAMLGroupMapper extends AbstractSAMLProtocolMapper impleme
property.setName(AttributeStatementHelper.SAML_ATTRIBUTE_NAME);
property.setLabel("Group attribute name");
property.setDefaultValue("member");
property.setHelpText("Name of the SAML attribute you want to put your groups into. i.e. 'member', 'memberOf'.");
property.setHelpText(
"Name of the SAML attribute you want to put your groups into. i.e. 'member', 'memberOf'.");
configProperties.add(property);
property = new ProviderConfigProperty();
property.setName(AttributeStatementHelper.FRIENDLY_NAME);
Expand Down Expand Up @@ -80,7 +81,7 @@ public List<ProviderConfigProperty> getConfigProperties() {

@Override
public void transformAttributeStatement(AttributeStatementType attributeStatement, ProtocolMapperModel mappingModel,
KeycloakSession session, UserSessionModel userSession, AuthenticatedClientSessionModel clientSession) {
KeycloakSession session, UserSessionModel userSession, AuthenticatedClientSessionModel clientSession) {
UserModel user = userSession.getUser();

// Retrieve the user's groups
Expand All @@ -92,11 +93,20 @@ public void transformAttributeStatement(AttributeStatementType attributeStatemen
.map(ModelToRepresentation::buildGroupPath)
.collect(Collectors.toList());

// Check for any group paths containing a colon
for (String groupPath : groupPaths) {
if (groupPath.contains(":")) {
throw new IllegalArgumentException(
"Group name contains invalid character ':'. Group name: " + groupPath);
}
}

// Concatenate the group paths into a single string separated by colons
String groupsString = String.join(":", groupPaths);

// Create a new SAML attribute
AttributeType attribute = new AttributeType(mappingModel.getConfig().get(AttributeStatementHelper.SAML_ATTRIBUTE_NAME));
AttributeType attribute = new AttributeType(
mappingModel.getConfig().get(AttributeStatementHelper.SAML_ATTRIBUTE_NAME));
attribute.setNameFormat(JBossSAMLURIConstants.ATTRIBUTE_FORMAT_BASIC.get());
attribute.addAttributeValue(groupsString);
attributeStatement.addAttribute(new ASTChoiceType(attribute));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
import java.util.Map;
import java.util.stream.Stream;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.Mockito.*;

public class CustomAWSSAMLGroupMapperTest {
Expand Down Expand Up @@ -91,6 +94,22 @@ public void testTransformAttributeStatement_singleGroupParent() {
}));
}

@Test
public void testTransformAttributeStatement_groupNameWithColonThrowsException() {
GroupModel group = mock(GroupModel.class);
when(group.getName()).thenReturn("Admin:IT");
when(group.getParent()).thenReturn(null);

when(mockUser.getGroupsStream()).thenReturn(Stream.of(group));

try {
mapper.transformAttributeStatement(mockAttributeStatement, mockMappingModel, mockSession, mockUserSession, mockClientSession);
fail("Expected IllegalArgumentException to be thrown");
} catch (IllegalArgumentException e) {
assertEquals("Group name contains invalid character ':'. Group name: /Admin:IT", e.getMessage());
}
}

@Test
public void testTransformAttributeStatement_multipleGroupsNoParent() {
GroupModel group1 = mock(GroupModel.class);
Expand Down