Skip to content

Commit

Permalink
support system allowed roles in id tokens by skipping limit check (#2626
Browse files Browse the repository at this point in the history 
)

Signed-off-by: Henry Avetisyan <hga@yahooinc.com>
Co-authored-by: Henry Avetisyan <hga@yahooinc.com>
  • Loading branch information
@havetisyan
havetisyan and havetisyan authored May 24, 2024
1 parent e5c248e commit 3e4b8e9
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 24 deletions.
8 changes: 8 additions & 0 deletions servers/zts/conf/zts.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -753,3 +753,11 @@ athenz.zts.k8s_provider_distribution_validator_factory_class=com.yahoo.athenz.in
# This property configures the factory providing internal implementation of the AttributeValidator interface
# as a plugin to validate the identity attributes in the request
# athenz.zts.k8s_provider_aws_attr_validator_factory_class=

# This property configures a comma separated list of system allowed role names
# that can be requested in ID tokens without affecting the maximum number of
# domains limitation set by the athenz.zts.id_token_max_domains property. The
# caller can only include a single system allowed role in the request.
# The most common case for this setting would be to avoid increasing the limit
# of maximum domains in a request from 1 to 2.
#athenz.zts.id_token_system_allowed_roles=
1 change: 1 addition & 0 deletions servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSConsts.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public final class ZTSConsts {
public static final String ZTS_PROP_ID_TOKEN_MAX_TIMEOUT = "athenz.zts.id_token_max_timeout";
public static final String ZTS_PROP_ID_TOKEN_DEFAULT_TIMEOUT = "athenz.zts.id_token_default_timeout";
public static final String ZTS_PROP_ID_TOKEN_MAX_DOMAINS = "athenz.zts.id_token_max_domains";
public static final String ZTS_PROD_ID_TOKEN_ALLOWED_ROLES = "athenz.zts.id_token_allowed_roles";
public static final String ZTS_PROP_SIGNED_POLICY_TIMEOUT = "athenz.zts.signed_policy_timeout";
public static final String ZTS_PROP_AUTHORIZED_PROXY_USERS = "athenz.zts.authorized_proxy_users";
public static final String ZTS_PROP_SECURE_REQUESTS_ONLY = "athenz.zts.secure_requests_only";
Expand Down
2 changes: 1 addition & 1 deletion servers/zts/src/main/java/com/yahoo/athenz/zts/token/AccessTokenRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public AccessTokenRequest(final String scope) {
// <domainName>:role.<roleName>
// openid <domainName>:service.<serviceName>

super(scope, 1);
super(scope, 1, null);

// if we don't have a domain then it's invalid scope

Expand Down
8 changes: 7 additions & 1 deletion servers/zts/src/main/java/com/yahoo/athenz/zts/token/IdTokenRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,19 @@
*/
package com.yahoo.athenz.zts.token;

import com.yahoo.athenz.common.server.util.config.dynamic.DynamicConfigCsv;
import com.yahoo.athenz.zts.ZTSConsts;

import static com.yahoo.athenz.common.server.util.config.ConfigManagerSingleton.CONFIG_MANAGER;

public class IdTokenRequest extends OAuthTokenRequest {

private static int maxDomains = Integer.parseInt(
System.getProperty(ZTSConsts.ZTS_PROP_ID_TOKEN_MAX_DOMAINS, "10"));

private static DynamicConfigCsv systemAllowedRoles = new DynamicConfigCsv(CONFIG_MANAGER,
ZTSConsts.ZTS_PROD_ID_TOKEN_ALLOWED_ROLES, null);

public static void setMaxDomains(int numDomains) {
maxDomains = numDomains;
}
Expand All @@ -34,7 +40,7 @@ public IdTokenRequest(final String scope) {
// openid <domainName>:role.<roleName>
// openid <domainName>:group.<groupName>

super(scope, maxDomains);
super(scope, maxDomains, systemAllowedRoles);

// make sure openid scope is requested

Expand Down
45 changes: 34 additions & 11 deletions servers/zts/src/main/java/com/yahoo/athenz/zts/token/OAuthTokenRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.yahoo.athenz.zts.token;

import com.yahoo.athenz.common.server.util.config.dynamic.DynamicConfigCsv;
import com.yahoo.athenz.zts.ResourceError;
import com.yahoo.athenz.zts.ResourceException;
import org.slf4j.Logger;
Expand Down Expand Up @@ -44,7 +45,7 @@ public class OAuthTokenRequest {
boolean rolesScope = false;
int maxDomains;

public OAuthTokenRequest(final String scope, int maxDomains) {
public OAuthTokenRequest(final String scope, int maxDomains, DynamicConfigCsv systemAllowedRoles) {

this.maxDomains = maxDomains;
if (this.maxDomains < 1) {
Expand All @@ -64,6 +65,7 @@ public OAuthTokenRequest(final String scope, int maxDomains) {
// openid <domainName>:group.<groupName>
// openid [<domainName>:domain]+

String systemAllowedRole = null;
Map<String, Set<String>> scopeRoleNames = new HashMap<>();
Map<String, Set<String>> scopeGroupNames = new HashMap<>();
for (String scopeItem : scopeList) {
Expand All @@ -86,7 +88,7 @@ public OAuthTokenRequest(final String scope, int maxDomains) {
int idx = scopeItem.indexOf(OBJECT_SERVICE);
if (idx != -1) {
final String scopeDomainName = scopeItem.substring(0, idx);
addScopeDomain(scopeDomainName, scope);
addScopeDomain(scopeDomainName, scope, true);
final String scopeServiceName = scopeItem.substring(idx + OBJECT_SERVICE.length());
if (serviceName != null && !scopeServiceName.equals(serviceName)) {
throw error("Multiple services in scope", scope);
Expand All @@ -99,7 +101,7 @@ public OAuthTokenRequest(final String scope, int maxDomains) {

if (scopeItem.endsWith(OBJECT_DOMAIN)) {
final String scopeDomainName = scopeItem.substring(0, scopeItem.length() - OBJECT_DOMAIN.length());
addScopeDomain(scopeDomainName, scope);
addScopeDomain(scopeDomainName, scope, true);
sendScopeResponse = true;
continue;
}
Expand All @@ -108,10 +110,21 @@ public OAuthTokenRequest(final String scope, int maxDomains) {

idx = scopeItem.indexOf(OBJECT_ROLE);
if (idx != -1) {
final String scopeDomainName = scopeItem.substring(0, idx);
addScopeDomain(scopeDomainName, scope);
scopeRoleNames.putIfAbsent(scopeDomainName, new HashSet<>());
scopeRoleNames.get(scopeDomainName).add(scopeItem.substring(idx + OBJECT_ROLE.length()));
// if the role is one of our authorized roles, we're not going
// to process it right away to avoid counting it against
// the configured max domain setting. We'll process it
// at the end without checking the domain limit. However, we
// still allow only a single authorized role to be specified
// so if we have multiple, we'll handle the second one as a
// regular role scope
if (systemAllowedRole == null && systemAllowedRoles != null && systemAllowedRoles.hasItem(scopeItem)) {
systemAllowedRole = scopeItem;
} else {
final String scopeDomainName = scopeItem.substring(0, idx);
addScopeDomain(scopeDomainName, scope, true);
scopeRoleNames.putIfAbsent(scopeDomainName, new HashSet<>());
scopeRoleNames.get(scopeDomainName).add(scopeItem.substring(idx + OBJECT_ROLE.length()));
}
continue;
}

Expand All @@ -120,7 +133,7 @@ public OAuthTokenRequest(final String scope, int maxDomains) {
idx = scopeItem.indexOf(OBJECT_GROUP);
if (idx != -1) {
final String scopeDomainName = scopeItem.substring(0, idx);
addScopeDomain(scopeDomainName, scope);
addScopeDomain(scopeDomainName, scope, true);
scopeGroupNames.putIfAbsent(scopeDomainName, new HashSet<>());
scopeGroupNames.get(scopeDomainName).add(scopeItem.substring(idx + OBJECT_GROUP.length()));
continue;
Expand All @@ -131,6 +144,16 @@ public OAuthTokenRequest(final String scope, int maxDomains) {
}
}

// process our authorized role if one was specified

if (systemAllowedRole != null) {
int idx = systemAllowedRole.indexOf(OBJECT_ROLE);
final String scopeDomainName = systemAllowedRole.substring(0, idx);
addScopeDomain(scopeDomainName, scope, false);
scopeRoleNames.putIfAbsent(scopeDomainName, new HashSet<>());
scopeRoleNames.get(scopeDomainName).add(systemAllowedRole.substring(idx + OBJECT_ROLE.length()));
}

// if the scope response is set to true then we had
// an explicit request for all roles or groups in the domain
// then we're going to ignore the role and groups names requested,
Expand Down Expand Up @@ -197,16 +220,16 @@ public boolean isRolesScope() {
return rolesScope;
}

void addScopeDomain(final String scopeDomainName, final String scope) {
void addScopeDomain(final String scopeDomainName, final String scope, boolean enforceMaxDomainCheck) {
if (scopeDomainName.isEmpty()) {
throw error("empty domain name", scope);
}
final String domainName = getDomainName();
if (domainName != null && !scopeDomainName.equals(domainName)) {
if (enforceMaxDomainCheck && domainName != null && !scopeDomainName.equals(domainName)) {
throw error("Multiple domains in scope", scope);
}
if (!domainNames.contains(scopeDomainName)) {
if (domainNames.size() == maxDomains) {
if (enforceMaxDomainCheck && domainNames.size() == maxDomains) {
throw error("Domain limit: " + maxDomains + " has been reached", scope);
}
domainNames.add(scopeDomainName);
Expand Down
66 changes: 55 additions & 11 deletions servers/zts/src/test/java/com/yahoo/athenz/zts/token/OAuthTokenRequestTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.yahoo.athenz.zts.token;

import com.yahoo.athenz.common.server.util.config.dynamic.DynamicConfigCsv;
import com.yahoo.athenz.zts.ResourceException;
import org.testng.annotations.Test;

Expand All @@ -28,7 +29,7 @@ public class OAuthTokenRequestTest {
public void testOauthTokenInvalidRequest() {

try {
new OAuthTokenRequest("scope", 0);
new OAuthTokenRequest("scope", 0, null);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), ResourceException.BAD_REQUEST);
Expand All @@ -40,14 +41,14 @@ public void testOauthTokenInvalidRequest() {
public void testOauthTokenRequestMaxDomains() {

try {
new OAuthTokenRequest("openid sports:domain weather:domain finance:domain", 2);
new OAuthTokenRequest("openid sports:domain weather:domain finance:domain", 2, null);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), ResourceException.BAD_REQUEST);
assertTrue(ex.getMessage().contains("Domain limit: 2 has been reached"));
}

OAuthTokenRequest request = new OAuthTokenRequest("openid sports:domain weather:domain finance:domain", 3);
OAuthTokenRequest request = new OAuthTokenRequest("openid sports:domain weather:domain finance:domain", 3, null);
assertNotNull(request);
Set<String> domains = request.getDomainNames();
assertEquals(domains.size(), 3);
Expand All @@ -58,30 +59,30 @@ public void testOauthTokenRequestMaxDomains() {

@Test
public void testOauthTokenGetDomainName() {
OAuthTokenRequest request = new OAuthTokenRequest("openid", 1);
OAuthTokenRequest request = new OAuthTokenRequest("openid", 1, null);
assertNull(request.getDomainName());

request = new OAuthTokenRequest("openid sports:domain weather:domain", 2);
request = new OAuthTokenRequest("openid sports:domain weather:domain", 2, null);
assertNull(request.getDomainName());

request = new OAuthTokenRequest("openid sports:domain", 2);
request = new OAuthTokenRequest("openid sports:domain", 2, null);
assertNull(request.getDomainName());

request = new OAuthTokenRequest("openid sports:domain", 1);
request = new OAuthTokenRequest("openid sports:domain", 1, null);
assertEquals(request.getDomainName(), "sports");
}

@Test
public void testOauthTokenGetRoleNames() {
OAuthTokenRequest request = new OAuthTokenRequest("openid", 1);
OAuthTokenRequest request = new OAuthTokenRequest("openid", 1, null);
assertNull(request.getRoleNames("sports"));

request = new OAuthTokenRequest("openid weather:role.test", 1);
request = new OAuthTokenRequest("openid weather:role.test", 1, null);
assertNull(request.getRoleNames("sports"));
assertEquals(request.getRoleNames("weather").length, 1);
assertEquals(request.getRoleNames("weather")[0], "test");

request = new OAuthTokenRequest("openid weather:role.test weather:role.admin", 2);
request = new OAuthTokenRequest("openid weather:role.test weather:role.admin", 2, null);
assertNull(request.getRoleNames("sports"));
assertEquals(request.getRoleNames("weather").length, 2);
assertEquals(request.getRoleNames("weather")[0], "test");
Expand All @@ -90,9 +91,52 @@ public void testOauthTokenGetRoleNames() {

@Test
public void testOauthTokenMultipleIdenticalServiceNames() {
OAuthTokenRequest request = new OAuthTokenRequest("openid sports:service.api sports:service.api", 1);
OAuthTokenRequest request = new OAuthTokenRequest("openid sports:service.api sports:service.api", 1, null);
assertNull(request.getRoleNames("sports"));
assertEquals(request.getServiceName(), "api");
assertEquals(request.getDomainName(), "sports");
}

@Test
public void testOauthTokenRequestMaxDomainsWithAuthorizedRoles() {

DynamicConfigCsv systemAllowedRoles = new DynamicConfigCsv("system:role.reader,system:role.writer");

// without authorized roles we'll have failure

try {
new OAuthTokenRequest("openid system:role.reader sports:service.api", 1, null);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), ResourceException.BAD_REQUEST);
assertTrue(ex.getMessage().contains("Multiple domains in scope"));
}

// with authorized list, we're good

OAuthTokenRequest request = new OAuthTokenRequest("openid system:role.reader sports:service.api",
1, systemAllowedRoles);
Set<String> domains = request.getDomainNames();
assertEquals(domains.size(), 2);
assertTrue(domains.contains("sports"));
assertTrue(domains.contains("system"));
assertEquals(request.getRoleNames("system")[0], "reader");

// with 2 authorized roles - not allowed

try {
new OAuthTokenRequest("openid system:role.reader sports:service.api system:role.writer", 1, null);
fail();
} catch (ResourceException ex) {
assertEquals(ex.getCode(), ResourceException.BAD_REQUEST);
assertTrue(ex.getMessage().contains("Multiple domains in scope"));
}

// standard single role without any system should work fine

request = new OAuthTokenRequest("openid sports:service.api", 1, systemAllowedRoles);
domains = request.getDomainNames();
assertEquals(domains.size(), 1);
assertTrue(domains.contains("sports"));
}
}

0 comments on commit 3e4b8e9

Please sign in to comment.