From 3e4b8e9f522c1ceb3e31cdc7aec626342ae6dd63 Mon Sep 17 00:00:00 2001 From: Henry Avetisyan Date: Fri, 24 May 2024 14:30:44 -0700 Subject: [PATCH] support system allowed roles in id tokens by skipping limit check (#2626) Signed-off-by: Henry Avetisyan Co-authored-by: Henry Avetisyan --- servers/zts/conf/zts.properties | 8 +++ .../java/com/yahoo/athenz/zts/ZTSConsts.java | 1 + .../athenz/zts/token/AccessTokenRequest.java | 2 +- .../athenz/zts/token/IdTokenRequest.java | 8 ++- .../athenz/zts/token/OAuthTokenRequest.java | 45 +++++++++---- .../zts/token/OAuthTokenRequestTest.java | 66 +++++++++++++++---- 6 files changed, 106 insertions(+), 24 deletions(-) diff --git a/servers/zts/conf/zts.properties b/servers/zts/conf/zts.properties index 5069fc8d285..e062da4564f 100644 --- a/servers/zts/conf/zts.properties +++ b/servers/zts/conf/zts.properties @@ -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= diff --git a/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSConsts.java b/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSConsts.java index 2df35de8e9d..b5d9e0f0c71 100644 --- a/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSConsts.java +++ b/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSConsts.java @@ -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"; diff --git a/servers/zts/src/main/java/com/yahoo/athenz/zts/token/AccessTokenRequest.java b/servers/zts/src/main/java/com/yahoo/athenz/zts/token/AccessTokenRequest.java index 4d546ad212e..183bf8152da 100644 --- a/servers/zts/src/main/java/com/yahoo/athenz/zts/token/AccessTokenRequest.java +++ b/servers/zts/src/main/java/com/yahoo/athenz/zts/token/AccessTokenRequest.java @@ -31,7 +31,7 @@ public AccessTokenRequest(final String scope) { // :role. // openid :service. - super(scope, 1); + super(scope, 1, null); // if we don't have a domain then it's invalid scope diff --git a/servers/zts/src/main/java/com/yahoo/athenz/zts/token/IdTokenRequest.java b/servers/zts/src/main/java/com/yahoo/athenz/zts/token/IdTokenRequest.java index 34758f710ed..ae087fad50b 100644 --- a/servers/zts/src/main/java/com/yahoo/athenz/zts/token/IdTokenRequest.java +++ b/servers/zts/src/main/java/com/yahoo/athenz/zts/token/IdTokenRequest.java @@ -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; } @@ -34,7 +40,7 @@ public IdTokenRequest(final String scope) { // openid :role. // openid :group. - super(scope, maxDomains); + super(scope, maxDomains, systemAllowedRoles); // make sure openid scope is requested diff --git a/servers/zts/src/main/java/com/yahoo/athenz/zts/token/OAuthTokenRequest.java b/servers/zts/src/main/java/com/yahoo/athenz/zts/token/OAuthTokenRequest.java index d70ed6a8e6b..47c02e4acc1 100644 --- a/servers/zts/src/main/java/com/yahoo/athenz/zts/token/OAuthTokenRequest.java +++ b/servers/zts/src/main/java/com/yahoo/athenz/zts/token/OAuthTokenRequest.java @@ -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; @@ -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) { @@ -64,6 +65,7 @@ public OAuthTokenRequest(final String scope, int maxDomains) { // openid :group. // openid [:domain]+ + String systemAllowedRole = null; Map> scopeRoleNames = new HashMap<>(); Map> scopeGroupNames = new HashMap<>(); for (String scopeItem : scopeList) { @@ -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); @@ -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; } @@ -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; } @@ -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; @@ -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, @@ -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); diff --git a/servers/zts/src/test/java/com/yahoo/athenz/zts/token/OAuthTokenRequestTest.java b/servers/zts/src/test/java/com/yahoo/athenz/zts/token/OAuthTokenRequestTest.java index f8691c8867c..a0cce507706 100644 --- a/servers/zts/src/test/java/com/yahoo/athenz/zts/token/OAuthTokenRequestTest.java +++ b/servers/zts/src/test/java/com/yahoo/athenz/zts/token/OAuthTokenRequestTest.java @@ -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; @@ -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); @@ -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 domains = request.getDomainNames(); assertEquals(domains.size(), 3); @@ -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"); @@ -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 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")); + } }