Repository: AthenZ/athenz

Issue:
Regression bug in postAccessToken API when using empty proxy_for_principal parameter
We're trying to upgrade our ZTS server version from v1.12.9 to v1.12.14 and have encountered a problem with the postAccessToken API.
It appears that there is bug e in how the API handles empty strings for the `proxy_for_principal` parameter.

As shown below:


**v1.12.9**
```bash
# Issue access token
curl -s -XPOST --key <key.pem> --cert <cert.pem>
    -d "grant_type=client_credentials" -d "scope=<domain>:domain"
    "https://$ZTS/v1/oauth2/token"  | jq
{
  "access_token": "<token>",
  "token_type": "Bearer",
  "expires_in": <expiry>,
  "scope": "<scope>"
}


# This also works - API responds with the same (scope) token as above
curl -s -XPOST --key <key.pem> --cert <cert.pem>
    -d "grant_type=client_credentials" -d "scope=<domain>:domain" -d "proxy_for_principal="
    "https://$ZTS/v1/oauth2/token"  | jq
{
  "access_token": "<token>",
  "token_type": "Bearer",
  "expires_in": <expiry>,
  "scope": "<scope>"
}
```


**v1.12.14**

```bash
# Issue access token - this works
curl -s -XPOST --key <key.pem> --cert <cert.pem>
    -d "grant_type=client_credentials" -d "scope=<domain>:domain"
    "https://$ZTS/v1/oauth2/token"  | jq
{
  "access_token": "<token>",
  "token_type": "Bearer",
  "expires_in": <expiry>,
  "scope": "<scope>"
}

# This now fails
curl -s -XPOST --key <key.pem> --cert <cert.pem>
    -d "grant_type=client_credentials" -d "scope=<domain>:domain" -d "proxy_for_principal="
    "https://$ZTS/v1/oauth2/token"  | jq
{
  "code": 403,
  "message": "postaccesstokenrequest: principal  is not included in any role in domain <domain>"
}
```

This has become a problem in our environment because some of our users send empty strings for the proxy_for_principal parameter due to their client libraries.
Would it be possible to consider reverting this behavior to match the previous versions, or providing backward compatibility for empty string parameters?

Thank you for your consideration

Discussion:
@havetisyan: That's not a breaking change introduced in the API but rather just a regression bug that was introduced. @hiragi-gkuth: Sorry for misleading expression a bit.
And thank you for your prompt response!@hiragi-gkuth: resolved by https://github.com/AthenZ/athenz/pull/2951

Solution patch:
diff --git a/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSImpl.java b/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSImpl.java
index 5f0ab87e3..7c505861c 100644
--- a/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSImpl.java
+++ b/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSImpl.java
@@ -2612,7 +2612,7 @@ public class ZTSImpl implements ZTSHandler {
         // remove any roles that are authorized by only one of the principals
 
         String proxyUser = null;
-        if (accessTokenRequest.getProxyForPrincipal() != null) {
+        if (!StringUtil.isEmpty(accessTokenRequest.getProxyForPrincipal())) {
 
             // we also need to verify that we are not returning id tokens.
             // proxy principal functionality is only valid for access tokens



Test patch:
diff --git a/servers/zts/src/test/java/com/yahoo/athenz/zts/ZTSImplTest.java b/servers/zts/src/test/java/com/yahoo/athenz/zts/ZTSImplTest.java
index ee367a8ef..f5ac70730 100644
--- a/servers/zts/src/test/java/com/yahoo/athenz/zts/ZTSImplTest.java
+++ b/servers/zts/src/test/java/com/yahoo/athenz/zts/ZTSImplTest.java
@@ -9957,6 +9957,30 @@ public class ZTSImplTest {
         }
     }
 
+    @Test
+    public void testPostAccessTokenRequestEmptyProxyPrincipal() throws JOSEException {
+
+        System.setProperty(FilePrivateKeyStore.ATHENZ_PROP_PRIVATE_KEY, "src/test/resources/unit_test_zts_at_private.pem");
+
+        CloudStore cloudStore = new CloudStore();
+        ZTSImpl ztsImpl = new ZTSImpl(cloudStore, store);
+        // set back to our zts rsa private key
+        System.setProperty(FilePrivateKeyStore.ATHENZ_PROP_PRIVATE_KEY, "src/test/resources/unit_test_zts_private.pem");
+
+        SignedDomain signedDomain = createSignedDomain("coretech", "weather", "storage", true);
+        store.processSignedDomain(signedDomain, false);
+
+        Principal principal = SimplePrincipal.create("user_domain", "user",
+                "v=U1;d=user_domain;n=user;s=signature", 0, null);
+        ResourceContext context = createResourceContext(principal);
+
+        final String scope = URLEncoder.encode("coretech:domain", StandardCharsets.UTF_8);
+        AccessTokenResponse resp = ztsImpl.postAccessTokenRequest(context,
+                "grant_type=client_credentials&proxy_for_principal=&scope=" + scope);
+        assertNotNull(resp);
+        assertEquals(resp.getScope(), "coretech:role.writers");
+    }
+
     @Test
     public void testPostAccessTokenRequestRoleAuthority() {