From 4c2e89d584cc102f6937b0bdf90c0d3eb847d6a8 Mon Sep 17 00:00:00 2001 From: Yamini Kashyap Date: Tue, 25 Nov 2025 10:56:46 -0800 Subject: [PATCH 1/2] iam: getInlinePolicyDetails API(GCP) --- .../multicloudj/iam/client/IamClient.java | 9 +++-- .../multicloudj/iam/driver/AbstractIam.java | 11 +++-- .../multicloudj/iam/driver/Identity.java | 7 +++- .../multicloudj/iam/client/IamClientTest.java | 11 ++--- .../multicloudj/iam/client/TestIam.java | 2 +- .../iam/driver/AbstractIamTest.java | 13 +++--- .../multicloudj/iam/gcp/GcpIam.java | 40 +++++++++++++++---- .../multicloudj/iam/gcp/GcpIamTest.java | 28 ++++++++++--- 8 files changed, 87 insertions(+), 34 deletions(-) diff --git a/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/client/IamClient.java b/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/client/IamClient.java index c793e810a..e7e7fad9b 100644 --- a/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/client/IamClient.java +++ b/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/client/IamClient.java @@ -125,14 +125,17 @@ public void attachInlinePolicy(PolicyDocument policyDocument, String tenantId, S * Retrieves the details of a specific inline policy attached to an identity. * * @param identityName the name of the identity - * @param policyName the name of the policy + * @param policyName the name of the policy. This parameter is optional and subject to cloud semantics. + * Some cloud providers may not support named policies, in which case this parameter may be ignored. + * @param roleName the role name. This parameter is optional and subject to cloud semantics. Some cloud providers + * may require this parameter to identify the policy, while others may not use it. * @param tenantId the tenant ID * @param region the region * @return the policy document details as a string */ - public String getInlinePolicyDetails(String identityName, String policyName, String tenantId, String region) { + public String getInlinePolicyDetails(String identityName, String policyName, String roleName, String tenantId, String region) { try { - return this.iam.getInlinePolicyDetails(identityName, policyName, tenantId, region); + return this.iam.getInlinePolicyDetails(identityName, policyName, roleName, tenantId, region); } catch (Throwable t) { Class exception = this.iam.getException(t); ExceptionHandler.handleAndPropagate(exception, t); diff --git a/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/driver/AbstractIam.java b/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/driver/AbstractIam.java index e6327ee7a..cc846b517 100644 --- a/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/driver/AbstractIam.java +++ b/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/driver/AbstractIam.java @@ -153,9 +153,9 @@ public void attachInlinePolicy(PolicyDocument policyDocument, String tenantId, * {@inheritDoc} */ @Override - public String getInlinePolicyDetails(String identityName, String policyName, + public String getInlinePolicyDetails(String identityName, String policyName, String roleName, String tenantId, String region) { - return doGetInlinePolicyDetails(identityName, policyName, tenantId, region); + return doGetInlinePolicyDetails(identityName, policyName, roleName, tenantId, region); } /** @@ -225,12 +225,15 @@ protected abstract void doAttachInlinePolicy(PolicyDocument policyDocument, Stri * Provider-specific implementations should override this method. * * @param identityName the name of the identity - * @param policyName the name of the policy + * @param policyName the name of the policy. This parameter is optional and subject to cloud semantics. + * Some cloud providers may not support named policies, in which case this parameter may be ignored. + * @param roleName the role name. This parameter is optional and subject to cloud semantics. Some cloud providers + * may require this parameter to identify the policy, while others may not use it. * @param tenantId the tenant ID * @param region the region * @return the policy document details as a string */ - protected abstract String doGetInlinePolicyDetails(String identityName, String policyName, + protected abstract String doGetInlinePolicyDetails(String identityName, String policyName, String roleName, String tenantId, String region); /** diff --git a/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/driver/Identity.java b/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/driver/Identity.java index 490337da3..f80e6826d 100644 --- a/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/driver/Identity.java +++ b/iam/iam-client/src/main/java/com/salesforce/multicloudj/iam/driver/Identity.java @@ -42,12 +42,15 @@ String createIdentity(String identityName, String description, String tenantId, * Retrieves the details of an inline policy attached to an identity. * * @param identityName the name of the identity - * @param policyName the name of the policy to retrieve + * @param policyName the name of the policy to retrieve. This parameter is optional and subject to cloud semantics. + * Some cloud providers may not support named policies, in which case this parameter may be ignored. + * @param roleName the role name. This parameter is optional and subject to cloud semantics. Some cloud providers + * may require this parameter to identify the policy, while others may not use it. * @param tenantId the tenant/project/account ID * @param region the region where the identity exists * @return the policy document as a JSON string */ - String getInlinePolicyDetails(String identityName, String policyName, String tenantId, String region); + String getInlinePolicyDetails(String identityName, String policyName, String roleName, String tenantId, String region); /** * Lists all policies attached to an identity. diff --git a/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/IamClientTest.java b/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/IamClientTest.java index e66c1862f..fdc0ec140 100644 --- a/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/IamClientTest.java +++ b/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/IamClientTest.java @@ -142,25 +142,26 @@ void testAttachInlinePolicyThrowsException() { @Test void testGetInlinePolicyDetails() { - when(mockIam.getInlinePolicyDetails(anyString(), anyString(), anyString(), anyString())) + when(mockIam.getInlinePolicyDetails(eq(TEST_ROLE), eq(TEST_POLICY_NAME), eq((String)null), + eq(TEST_TENANT_ID), eq(TEST_REGION))) .thenReturn(POLICY_RESPONSE); - String result = client.getInlinePolicyDetails(TEST_ROLE, TEST_POLICY_NAME, + String result = client.getInlinePolicyDetails(TEST_ROLE, TEST_POLICY_NAME, null, TEST_TENANT_ID, TEST_REGION); assertEquals(POLICY_RESPONSE, result); verify(mockIam, times(1)).getInlinePolicyDetails( - eq(TEST_ROLE), eq(TEST_POLICY_NAME), eq(TEST_TENANT_ID), eq(TEST_REGION)); + eq(TEST_ROLE), eq(TEST_POLICY_NAME), eq((String)null), eq(TEST_TENANT_ID), eq(TEST_REGION)); } @Test void testGetInlinePolicyDetailsThrowsException() { doReturn(UnAuthorizedException.class).when(mockIam).getException(any()); doThrow(RuntimeException.class).when(mockIam).getInlinePolicyDetails( - anyString(), anyString(), anyString(), anyString()); + anyString(), anyString(), anyString(), anyString(), anyString()); assertThrows(UnAuthorizedException.class, () -> - client.getInlinePolicyDetails(TEST_ROLE, TEST_POLICY_NAME, TEST_TENANT_ID, TEST_REGION)); + client.getInlinePolicyDetails(TEST_ROLE, TEST_POLICY_NAME, null, TEST_TENANT_ID, TEST_REGION)); } @Test diff --git a/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/TestIam.java b/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/TestIam.java index 9291d3cee..836039d21 100644 --- a/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/TestIam.java +++ b/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/TestIam.java @@ -40,7 +40,7 @@ protected void doAttachInlinePolicy(PolicyDocument policyDocument, String tenant } @Override - protected String doGetInlinePolicyDetails(String identityName, String policyName, + protected String doGetInlinePolicyDetails(String identityName, String policyName, String roleName, String tenantId, String region) { return "mock-policy-details"; } diff --git a/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/driver/AbstractIamTest.java b/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/driver/AbstractIamTest.java index 2bb20d3cc..e0b3069da 100644 --- a/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/driver/AbstractIamTest.java +++ b/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/driver/AbstractIamTest.java @@ -49,7 +49,7 @@ void setup() { mockIam = spy(iam); doCallRealMethod().when(mockIam).createIdentity(anyString(), anyString(), anyString(), anyString(), any(), any()); doCallRealMethod().when(mockIam).attachInlinePolicy(any(), anyString(), anyString(), anyString()); - doCallRealMethod().when(mockIam).getInlinePolicyDetails(anyString(), anyString(), anyString(), anyString()); + doCallRealMethod().when(mockIam).getInlinePolicyDetails(anyString(), anyString(), anyString(), anyString(), anyString()); doCallRealMethod().when(mockIam).getAttachedPolicies(anyString(), anyString(), anyString()); doCallRealMethod().when(mockIam).removePolicy(anyString(), anyString(), anyString(), anyString()); doCallRealMethod().when(mockIam).deleteIdentity(anyString(), anyString(), anyString()); @@ -110,12 +110,12 @@ void testAttachInlinePolicy() { @Test void testGetInlinePolicyDetails() { - doReturn(POLICY_RESPONSE).when(mockIam).getInlinePolicyDetails( - anyString(), anyString(), anyString(), anyString()); - + // Since we're using doCallRealMethod, it will call the actual TestIam implementation + // which returns "mock-policy-details", so we expect that value String policyDetails = mockIam.getInlinePolicyDetails( TEST_ROLE, TEST_POLICY_NAME, + null, TEST_TENANT_ID, TEST_REGION ); @@ -123,10 +123,11 @@ void testGetInlinePolicyDetails() { verify(mockIam, times(1)).getInlinePolicyDetails( eq(TEST_ROLE), eq(TEST_POLICY_NAME), + eq((String)null), eq(TEST_TENANT_ID), eq(TEST_REGION) ); - assertEquals(POLICY_RESPONSE, policyDetails); + assertEquals("mock-policy-details", policyDetails); } @Test @@ -229,7 +230,7 @@ void testDirectPublicMethodDelegation() { assertEquals("mock-identity-id", identity); // Test getInlinePolicyDetails delegation - String policyDetails = testIam.getInlinePolicyDetails(TEST_ROLE, TEST_POLICY_NAME, + String policyDetails = testIam.getInlinePolicyDetails(TEST_ROLE, TEST_POLICY_NAME, null, TEST_TENANT_ID, TEST_REGION); assertEquals("mock-policy-details", policyDetails); diff --git a/iam/iam-gcp/src/main/java/com/salesforce/multicloudj/iam/gcp/GcpIam.java b/iam/iam-gcp/src/main/java/com/salesforce/multicloudj/iam/gcp/GcpIam.java index 4d312680a..f2e365893 100644 --- a/iam/iam-gcp/src/main/java/com/salesforce/multicloudj/iam/gcp/GcpIam.java +++ b/iam/iam-gcp/src/main/java/com/salesforce/multicloudj/iam/gcp/GcpIam.java @@ -14,6 +14,7 @@ import com.google.iam.v1.Policy; import com.google.iam.v1.SetIamPolicyRequest; +import com.salesforce.multicloudj.common.exceptions.InvalidArgumentException; import com.salesforce.multicloudj.common.exceptions.SubstrateSdkException; import com.salesforce.multicloudj.common.exceptions.UnknownException; import com.salesforce.multicloudj.common.gcp.CommonErrorCodeMapping; @@ -24,6 +25,7 @@ import com.salesforce.multicloudj.iam.model.Statement; import com.salesforce.multicloudj.iam.model.TrustConfiguration; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import java.io.IOException; @@ -262,12 +264,24 @@ private Policy addBinding(Policy policy, String role, String member) { } /** - * Retrieves the details of a specific inline policy (role) attached to an IAM member. + * Retrieves the details of a specific inline policy attached to an IAM member. * In GCP, this retrieves the role binding information and converts it back to a PolicyDocument format. + * + *

GCP-specific behavior: + *

* * @param identityName the IAM member (e.g., "serviceAccount:my-sa@project.iam.gserviceaccount.com", * "user:user@example.com", "group:group@example.com") - * @param policyName the role name (e.g., "roles/iam.serviceAccountUser") + * @param policyName the name of the policy. This parameter is optional and subject to cloud semantics. + * In GCP, named policies are not supported, so this parameter is not used but kept for interface compatibility. + * @param roleName the role name. This parameter is optional and subject to cloud semantics. + * In GCP, this parameter is required and must be a valid GCP IAM role name (e.g., "roles/iam.serviceAccountUser"). + * Must not be null or empty. * @param tenantId the resource name that owns the IAM policy. Examples include: * "organizations/123456789012", * "folders/987654321098", @@ -276,9 +290,14 @@ private Policy addBinding(Policy policy, String role, String member) { * Can be any GCP resource that supports IAM policies. * @param region the region (optional for GCP) * @return the policy document as a JSON string, or null if the policy doesn't exist + * @throws InvalidArgumentException if roleName is null or empty */ @Override - protected String doGetInlinePolicyDetails(String identityName, String policyName, String tenantId, String region) { + protected String doGetInlinePolicyDetails(String identityName, String policyName, String roleName, String tenantId, String region) { + if (roleName == null || roleName.trim().isEmpty()) { + throw new InvalidArgumentException("roleName is required for GCP IAM"); + } + // Get the current IAM policy for the resource GetIamPolicyRequest getRequest = GetIamPolicyRequest.newBuilder() .setResource(tenantId) @@ -291,7 +310,7 @@ protected String doGetInlinePolicyDetails(String identityName, String policyName // Find the binding for the specified role Optional binding = policy.getBindingsList().stream() - .filter(b -> b.getRole().equals(policyName)) + .filter(b -> b.getRole().equals(roleName)) .findFirst(); // Check if the service account is a member of this binding @@ -305,7 +324,7 @@ protected String doGetInlinePolicyDetails(String identityName, String policyName .version("") .statement(Statement.builder() .effect(EFFECT_ALLOW) - .action(policyName) + .action(roleName) .build()) .build(); @@ -323,7 +342,7 @@ protected String doGetInlinePolicyDetails(String identityName, String policyName private String toJsonString(PolicyDocument policyDocument) { try { return new ObjectMapper().writeValueAsString(policyDocument); - } catch (Exception e) { + } catch (JsonProcessingException e) { throw new SubstrateSdkException("Failed to serialize policy document to JSON", e); } } @@ -398,9 +417,16 @@ protected void doRemovePolicy(String identityName, String policyName, String ten return; } - // Remove the binding - we know a change will occur because of the check above + // Remove the binding Policy updatedPolicy = removeBinding(policy, policyName, identityName); + // Only make the remote call if the policy actually changed + if (policy.getBindingsCount() == updatedPolicy.getBindingsCount() + && policy.getBindingsList().equals(updatedPolicy.getBindingsList())) { + // Policy didn't change, skip the remote call + return; + } + // Set the updated policy back to the resource SetIamPolicyRequest setRequest = SetIamPolicyRequest.newBuilder() .setResource(tenantId) diff --git a/iam/iam-gcp/src/test/java/com/salesforce/multicloudj/iam/gcp/GcpIamTest.java b/iam/iam-gcp/src/test/java/com/salesforce/multicloudj/iam/gcp/GcpIamTest.java index b62c231bc..18bd0f8a0 100644 --- a/iam/iam-gcp/src/test/java/com/salesforce/multicloudj/iam/gcp/GcpIamTest.java +++ b/iam/iam-gcp/src/test/java/com/salesforce/multicloudj/iam/gcp/GcpIamTest.java @@ -613,7 +613,7 @@ void testDoGetInlinePolicyDetailsSuccess() { // Execute String result = gcpIam.doGetInlinePolicyDetails( - TEST_SERVICE_ACCOUNT, TEST_ROLE, TEST_TENANT_ID, TEST_REGION); + TEST_SERVICE_ACCOUNT, null, TEST_ROLE, TEST_TENANT_ID, TEST_REGION); // Verify Assertions.assertNotNull(result); @@ -643,7 +643,7 @@ void testDoGetInlinePolicyDetailsWithNonExistentBinding() { // Execute String result = gcpIam.doGetInlinePolicyDetails( - TEST_SERVICE_ACCOUNT, TEST_ROLE, TEST_TENANT_ID, TEST_REGION); + TEST_SERVICE_ACCOUNT, null, TEST_ROLE, TEST_TENANT_ID, TEST_REGION); // Verify: Should return null when binding doesn't exist Assertions.assertNull(result); @@ -663,7 +663,7 @@ void testDoGetInlinePolicyDetailsWithMemberNotInBinding() { // Execute String result = gcpIam.doGetInlinePolicyDetails( - TEST_SERVICE_ACCOUNT, TEST_ROLE, TEST_TENANT_ID, TEST_REGION); + TEST_SERVICE_ACCOUNT, null, TEST_ROLE, TEST_TENANT_ID, TEST_REGION); // Verify: Should return null when service account is not in binding Assertions.assertNull(result); @@ -676,7 +676,7 @@ void testDoGetInlinePolicyDetailsWithNullPolicy() { // Execute String result = gcpIam.doGetInlinePolicyDetails( - TEST_SERVICE_ACCOUNT, TEST_ROLE, TEST_TENANT_ID, TEST_REGION); + TEST_SERVICE_ACCOUNT, null, TEST_ROLE, TEST_TENANT_ID, TEST_REGION); // Verify: Should return null when policy is null Assertions.assertNull(result); @@ -689,8 +689,24 @@ void testDoGetInlinePolicyDetailsWithApiException() { when(mockProjectsClient.getIamPolicy(any(GetIamPolicyRequest.class))).thenThrow(apiException); // Execute and verify exception is thrown - assertThrows(ApiException.class, () -> { - gcpIam.doGetInlinePolicyDetails(TEST_SERVICE_ACCOUNT, TEST_ROLE, TEST_TENANT_ID, TEST_REGION); + Assertions.assertThrows(ApiException.class, () -> { + gcpIam.doGetInlinePolicyDetails(TEST_SERVICE_ACCOUNT, null, TEST_ROLE, TEST_TENANT_ID, TEST_REGION); + }); + } + + @Test + void testDoGetInlinePolicyDetailsWithNullRoleName() { + // Execute and verify InvalidArgumentException is thrown when roleName is null + Assertions.assertThrows(InvalidArgumentException.class, () -> { + gcpIam.doGetInlinePolicyDetails(TEST_SERVICE_ACCOUNT, null, null, TEST_TENANT_ID, TEST_REGION); + }); + } + + @Test + void testDoGetInlinePolicyDetailsWithEmptyRoleName() { + // Execute and verify InvalidArgumentException is thrown when roleName is empty + Assertions.assertThrows(InvalidArgumentException.class, () -> { + gcpIam.doGetInlinePolicyDetails(TEST_SERVICE_ACCOUNT, null, "", TEST_TENANT_ID, TEST_REGION); }); } From 11a2cbdce0d03ab1aacc5f9a784ed1e990dc3cca Mon Sep 17 00:00:00 2001 From: Yamini Kashyap Date: Sat, 6 Dec 2025 09:44:21 -0800 Subject: [PATCH 2/2] iam: getInlinePolicyDetails API(GCP) --- .../com/salesforce/multicloudj/iam/client/AbstractIamIT.java | 1 + 1 file changed, 1 insertion(+) diff --git a/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/AbstractIamIT.java b/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/AbstractIamIT.java index 8e73d4bc2..536bf2a70 100644 --- a/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/AbstractIamIT.java +++ b/iam/iam-client/src/test/java/com/salesforce/multicloudj/iam/client/AbstractIamIT.java @@ -129,6 +129,7 @@ public void testGetInlinePolicyDetails() { String policyDetails = iamClient.getInlinePolicyDetails( harness.getIdentityName(), + null, harness.getTestPolicyName(), harness.getTenantId(), harness.getRegion()