diff --git a/libs/java/auth_core/src/main/java/com/yahoo/athenz/auth/impl/ExternalEmailMemberValidator.java b/libs/java/auth_core/src/main/java/com/yahoo/athenz/auth/impl/ExternalEmailMemberValidator.java index a37e18c5f35..cd2d609dc6d 100644 --- a/libs/java/auth_core/src/main/java/com/yahoo/athenz/auth/impl/ExternalEmailMemberValidator.java +++ b/libs/java/auth_core/src/main/java/com/yahoo/athenz/auth/impl/ExternalEmailMemberValidator.java @@ -23,6 +23,9 @@ import java.util.Set; import java.util.regex.Pattern; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * A sample external member validator that validates email addresses. * It doesn't verify the strict rules for email addresses (for example, @@ -45,6 +48,8 @@ */ public class ExternalEmailMemberValidator implements ExternalMemberValidator { + private static final Logger LOG = LoggerFactory.getLogger(ExternalEmailMemberValidator.class); + public static final String PROP_VALID_EMAIL_DOMAINS = "athenz.external_member.valid_email_domains"; private static final Pattern EMAIL_PATTERN = Pattern.compile( @@ -52,6 +57,7 @@ public class ExternalEmailMemberValidator implements ExternalMemberValidator { private final Set validEmailDomains; + public ExternalEmailMemberValidator() { validEmailDomains = parseEmailDomains(System.getProperty(PROP_VALID_EMAIL_DOMAINS)); } @@ -74,10 +80,12 @@ static Set parseEmailDomains(final String domainList) { public boolean validateMember(final String domainName, final String memberName) { if (memberName == null || memberName.isEmpty()) { + LOG.error("Member name is null or empty"); return false; } if (!EMAIL_PATTERN.matcher(memberName).matches()) { + LOG.error("Member name is not a valid email address: {}", memberName); return false; } diff --git a/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java b/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java index ec659184748..b3389b79759 100644 --- a/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java +++ b/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java @@ -4896,13 +4896,27 @@ Authority.UserType validateRoleMemberPrincipal(final String domainName, final St case EXTERNAL: - externalMemberValidatorManager.validateMember(domainName, memberName, caller); + validateExternalMember(domainName, memberName, caller); break; } return userType; } + void validateExternalMember(final String domainName, final String memberName, final String caller) { + + // we need to separate the external member prefix from the member name + // and only pass the member name to the external resource validator + // we'll check for the missing separator though we should never get there + // since the server has already validated the format of the external member name + + int sepIdx = memberName.indexOf(AuthorityConsts.EXT_SEP); + if (sepIdx == -1) { + throw ZMSUtils.requestError("Principal " + memberName + " is not valid", caller); + } + externalMemberValidatorManager.validateMember(domainName, memberName.substring(sepIdx + AuthorityConsts.EXT_SEP.length()), caller); + } + Authority.UserType validateGroupMemberPrincipal(final String domainName, final String groupName, final String memberName, int principalType, final Set userAuthorityFilterSet, PrincipalDomainFilter principalDomainFilter, final String caller) { @@ -4949,8 +4963,8 @@ Authority.UserType validateGroupMemberPrincipal(final String domainName, final S case EXTERNAL: - externalMemberValidatorManager.validateMember(domainName, memberName, caller); - break; + validateExternalMember(domainName, memberName, caller); + break; } return userType; diff --git a/servers/zms/src/test/java/com/yahoo/athenz/zms/ZMSExternalMemberTest.java b/servers/zms/src/test/java/com/yahoo/athenz/zms/ZMSExternalMemberTest.java index 413d2fb1eeb..3b24a0d9f55 100644 --- a/servers/zms/src/test/java/com/yahoo/athenz/zms/ZMSExternalMemberTest.java +++ b/servers/zms/src/test/java/com/yahoo/athenz/zms/ZMSExternalMemberTest.java @@ -285,7 +285,7 @@ public void testPutMembershipExternalMemberValidator() { fail(); } catch (ResourceException ex) { assertEquals(ex.getCode(), ResourceException.BAD_REQUEST); - assertTrue(ex.getMessage().contains("Member ext-mbr-role-validator:ext.invalid-user is not valid according to the external member validator for domain ext-mbr-role-validator")); + assertTrue(ex.getMessage().contains("Member invalid-user is not valid according to the external member validator for domain ext-mbr-role-validator")); } zmsImpl.deleteTopLevelDomain(ctx, domainName, auditRef, null); @@ -571,7 +571,7 @@ public void testPutGroupMembershipExternalMemberValidator() { fail(); } catch (ResourceException ex) { assertEquals(ex.getCode(), ResourceException.BAD_REQUEST); - assertTrue(ex.getMessage().contains("Member ext-mbr-group-validator:ext.invalid-user is not valid according to the external member validator for domain ext-mbr-group-validator")); + assertTrue(ex.getMessage().contains("Member invalid-user is not valid according to the external member validator for domain ext-mbr-group-validator")); } zmsImpl.deleteTopLevelDomain(ctx, domainName, auditRef, null); @@ -912,4 +912,100 @@ public void testGetPrincipalGroupsWithExternalMember() { zmsImpl.deleteTopLevelDomain(ctx, domainName1, auditRef, null); zmsImpl.deleteTopLevelDomain(ctx, domainName2, auditRef, null); } + + @Test + public void testValidateExternalMemberNoSeparator() { + + ZMSImpl zmsImpl = zmsTestInitializer.getZms(); + + try { + zmsImpl.validateExternalMember("domain1", "member-without-separator", "testCaller"); + fail(); + } catch (ResourceException ex) { + assertEquals(ex.getCode(), ResourceException.BAD_REQUEST); + assertTrue(ex.getMessage().contains("Principal member-without-separator is not valid")); + } + } + + @Test + public void testValidateExternalMemberEmptyName() { + + ZMSImpl zmsImpl = zmsTestInitializer.getZms(); + + try { + zmsImpl.validateExternalMember("domain1", "", "testCaller"); + fail(); + } catch (ResourceException ex) { + assertEquals(ex.getCode(), ResourceException.BAD_REQUEST); + assertTrue(ex.getMessage().contains("Principal is not valid")); + } + } + + @Test + public void testValidateExternalMemberPlainUser() { + + ZMSImpl zmsImpl = zmsTestInitializer.getZms(); + + try { + zmsImpl.validateExternalMember("domain1", "user.joe", "testCaller"); + fail(); + } catch (ResourceException ex) { + assertEquals(ex.getCode(), ResourceException.BAD_REQUEST); + assertTrue(ex.getMessage().contains("Principal user.joe is not valid")); + } + } + + @Test + public void testValidateExternalMemberPartialSeparator() { + + ZMSImpl zmsImpl = zmsTestInitializer.getZms(); + + // "ext." without the leading colon is not a valid separator + try { + zmsImpl.validateExternalMember("domain1", "ext.partner-user", "testCaller"); + fail(); + } catch (ResourceException ex) { + assertEquals(ex.getCode(), ResourceException.BAD_REQUEST); + assertTrue(ex.getMessage().contains("Principal ext.partner-user is not valid")); + } + + // colon without "ext." suffix is not a valid separator + try { + zmsImpl.validateExternalMember("domain1", "domain1:partner-user", "testCaller"); + fail(); + } catch (ResourceException ex) { + assertEquals(ex.getCode(), ResourceException.BAD_REQUEST); + assertTrue(ex.getMessage().contains("Principal domain1:partner-user is not valid")); + } + } + + @Test + public void testValidateExternalMemberWithValidSeparator() { + + ZMSImpl zmsImpl = zmsTestInitializer.getZms(); + RsrcCtxWrapper ctx = zmsTestInitializer.getMockDomRsrcCtx(); + final String auditRef = zmsTestInitializer.getAuditRef(); + + final String domainName = "ext-validate-sep"; + + TopLevelDomain dom = zmsTestInitializer.createTopLevelDomainObject(domainName, + "Test Domain", "testOrg", zmsTestInitializer.getAdminUser(), + ctx.principal().getFullName()); + zmsImpl.postTopLevelDomain(ctx, auditRef, null, dom); + + ZMSTestUtils.setupSystemMetaAuthorization(ctx, zmsImpl, + ctx.principal().getFullName(), auditRef); + + DomainMeta dm = new DomainMeta().setExternalMemberValidator( + "com.yahoo.athenz.zms.TestExternalMemberValidator"); + zmsImpl.putDomainSystemMeta(ctx, domainName, "externalmembervalidator", auditRef, dm); + zmsImpl.externalMemberValidatorManager.refreshValidators(); + + // valid separator present - should not throw since the member name + // does not contain "invalid" + + zmsImpl.validateExternalMember(domainName, domainName + ":ext.partner-user", "testCaller"); + + zmsImpl.deleteTopLevelDomain(ctx, domainName, auditRef, null); + } }