Skip to content

Commit

Permalink
Merge pull request #193 from yahoo/proxy
Browse files Browse the repository at this point in the history 
#192 strip out roles for proxy user requests instead of rejecting
  • Loading branch information
@havetisyan
havetisyan authored Jul 28, 2017
2 parents 506b43b + bfab0ce commit ade9452
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 9 deletions.
14 changes: 9 additions & 5 deletions servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1162,16 +1162,20 @@ public RoleToken getRoleToken(ResourceContext ctx, String domainName, String rol
}

// if this is proxy for operation then we want to make sure that
// both principals have access to the same set of roles otherwise
// we're not going to issue a roletoken with an updated principal
// field
// both principals have access to the same set of roles so we'll
// remove any roles that are authorized by only one of the principals

String proxyUser = null;
if (proxyForPrincipal != null) {
Set<String> rolesForProxy = new HashSet<>();
dataStore.getAccessibleRoles(data, domainName, proxyForPrincipal, roleName, rolesForProxy, false);
if (!compareRoleSets(roles, rolesForProxy)) {
throw forbiddenError("getRoleToken: Principal does not have access to the same set of roles as proxy principal",
roles.retainAll(rolesForProxy);

// check again in case we removed all the roles and ended up
// with an empty set

if (roles.isEmpty()) {
throw forbiddenError("getRoleToken: No access to any roles by User and Proxy Principals",
caller, domainName);
}

Expand Down
50 changes: 46 additions & 4 deletions servers/zts/src/test/java/com/yahoo/athenz/zts/ZTSImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.security.PublicKey;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -1536,6 +1535,7 @@ public void testGetRoleTokenUnauthorizedProxy() {
try {
zts.getRoleToken(context, "coretech-proxy1", null, Integer.valueOf(600),
Integer.valueOf(1200), "user_domain.unknown-proxy-user");
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 403);
assertTrue(ex.getMessage().contains("not authorized for proxy role token request"));
Expand Down Expand Up @@ -1589,7 +1589,7 @@ public void testGetRoleTokenProxyUser() {
}

@Test
public void testGetRoleTokenProxyUserMismatchRoles() {
public void testGetRoleTokenProxyUserMismatchRolesIntersection() {

List<RoleMember> writers = new ArrayList<>();
writers.add(new RoleMember().setMemberName("user_domain.proxy-user1"));
Expand All @@ -1608,12 +1608,39 @@ public void testGetRoleTokenProxyUserMismatchRoles() {
"v=U1;d=user_domain;n=proxy-user1;s=sig", 0, null);
ResourceContext context = createResourceContext(principal);

RoleToken roleToken = zts.getRoleToken(context, "coretech-proxy3", null,
Integer.valueOf(600), Integer.valueOf(1200), "user_domain.joe");
com.yahoo.athenz.auth.token.RoleToken token = new com.yahoo.athenz.auth.token.RoleToken(roleToken.getToken());

assertEquals(token.getRoles().size(), 1);
assertTrue(token.getRoles().contains("writers"));
}

@Test
public void testGetRoleTokenProxyUserMismatchRolesEmptySet() {

List<RoleMember> writers = new ArrayList<>();
writers.add(new RoleMember().setMemberName("user_domain.joe"));

List<RoleMember> readers = new ArrayList<>();
readers.add(new RoleMember().setMemberName("user_domain.proxy-user2"));
readers.add(new RoleMember().setMemberName("user_domain.jane"));
readers.add(new RoleMember().setMemberName("user_domain.proxy-user1"));

SignedDomain signedDomain = createSignedDomain("coretech-proxy4", "weather-proxy4", "storage",
writers, readers, true);
store.processDomain(signedDomain, false);

Principal principal = SimplePrincipal.create("user_domain", "proxy-user1",
"v=U1;d=user_domain;n=proxy-user1;s=sig", 0, null);
ResourceContext context = createResourceContext(principal);

try {
zts.getRoleToken(context, "coretech-proxy3", null, Integer.valueOf(600),
zts.getRoleToken(context, "coretech-proxy4", null, Integer.valueOf(600),
Integer.valueOf(1200), "user_domain.joe");
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 403);
assertTrue(ex.getMessage().contains("does not have access to the same set of roles as proxy"));
}
}

Expand Down Expand Up @@ -2490,6 +2517,7 @@ public void testGetTenantDomainsInvalidDomain() {

try {
zts.getTenantDomains(context, "athenz.non_product", "user100", null, null);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 404);
}
Expand Down Expand Up @@ -3155,6 +3183,7 @@ public void testPostAWSCertificateRequestInvalidCSR() throws IOException {

try {
zts.postAWSCertificateRequest(context, "athenz", "syncer", req);
fail();
} catch (ResourceException ex) {
assertEquals(400, ex.getCode());
}
Expand Down Expand Up @@ -3187,6 +3216,7 @@ public void testPostAWSCertificateRequestMismatchPrincipal() throws IOException

try {
zts.postAWSCertificateRequest(context, "athenz", "zts", req);
fail();
} catch (ResourceException ex) {
assertEquals(400, ex.getCode());
}
Expand Down Expand Up @@ -3736,6 +3766,7 @@ public void testPostOSTKInstanceRefreshRequestPrincipalMismatch() throws IOExcep

try {
zts.postOSTKInstanceRefreshRequest(context, "athenz", "production", req);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 400);
}
Expand Down Expand Up @@ -4151,6 +4182,7 @@ public void testPostDomainMetrics() {
String errMsg = "No such domain";
try {
ztsImpl.postDomainMetrics(context, testDomain, req);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 404);
assertTrue(ex.getMessage().contains(errMsg));
Expand Down Expand Up @@ -4185,6 +4217,7 @@ public void testPostDomainMetrics() {
metrixMap.clear();
try {
ztsImpl.postDomainMetrics(context, testDomain, req);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 400);
assertTrue(ex.getMessage().contains(errMsg), ex.getMessage());
Expand Down Expand Up @@ -4220,6 +4253,7 @@ public void testPostDomainMetrics() {
metrixMap.clear();
try {
ztsImpl.postDomainMetrics(context, testDomain, req);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 400);
assertTrue(ex.getMessage().contains(errMsg), ex.getMessage());
Expand Down Expand Up @@ -4253,6 +4287,7 @@ public void testPostDomainMetrics() {
metrixMap.clear();
try {
ztsImpl.postDomainMetrics(context, testDomain, req);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 400);
assertTrue(ex.getMessage().contains(errMsg), ex.getMessage());
Expand Down Expand Up @@ -5270,6 +5305,7 @@ public void testPostInstanceRefreshInformationCSRValidateFailure() throws IOExce
try {
ztsImpl.postInstanceRefreshInformation(context, "athenz.provider",
"athenz", "production", "1001", info);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 400);
assertTrue(ex.getMessage().contains("CSR validation failed"));
Expand Down Expand Up @@ -5328,6 +5364,7 @@ public void testPostInstanceRefreshInformationCertDNSMismatch() throws IOExcepti
try {
ztsImpl.postInstanceRefreshInformation(context, "athenz.provider",
"athenz", "production", "1001", info);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 400);
assertTrue(ex.getMessage().contains("dnsName attribute mismatch in CSR"));
Expand Down Expand Up @@ -5388,6 +5425,7 @@ public void testPostInstanceRefreshInformationGetCertDBFailure() throws IOExcept
try {
ztsImpl.postInstanceRefreshInformation(context, "athenz.provider",
"athenz", "production", "1001", info);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 403);
assertTrue(ex.getMessage().contains("Unable to find certificate record"));
Expand Down Expand Up @@ -5455,6 +5493,7 @@ public void testPostInstanceRefreshInformationCertRecordCNMismatch() throws IOEx
try {
ztsImpl.postInstanceRefreshInformation(context, "athenz.provider",
"athenz", "production", "1001", info);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 400);
assertTrue(ex.getMessage().contains("service name mismatch"));
Expand Down Expand Up @@ -5522,6 +5561,7 @@ public void testPostInstanceRefreshInformationSerialMismatch() throws IOExceptio
try {
ztsImpl.postInstanceRefreshInformation(context, "athenz.provider",
"athenz", "production", "1001", info);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 403);
assertTrue(ex.getMessage().contains("Certificate revoked"));
Expand Down Expand Up @@ -5587,6 +5627,7 @@ public void testPostInstanceRefreshInformationIdentityFailure() throws IOExcepti
try {
ztsImpl.postInstanceRefreshInformation(context, "athenz.provider",
"athenz", "production", "1001", info);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 500);
assertTrue(ex.getMessage().contains("unable to generate identity"));
Expand Down Expand Up @@ -5654,6 +5695,7 @@ public void testPostInstanceRefreshInformationCertRecordFailure() throws IOExcep
try {
ztsImpl.postInstanceRefreshInformation(context, "athenz.provider",
"athenz", "production", "1001", info);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), 500);
assertTrue(ex.getMessage().contains("unable to update cert db"));
Expand Down

0 comments on commit ade9452

Please sign in to comment.