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.

Sign up for GitHub

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

Jump to bottom

support system allowed roles in id tokens by skipping limit check #2626

Merged
merged 1 commit into from
May 24, 2024
Merged

support system allowed roles in id tokens by skipping limit check #2626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions servers/zts/conf/zts.properties
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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"));
}
}
Loading