From a39a6f9822071518884143eb4617d332aeebefdb Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Thu, 25 Jun 2026 13:17:57 +0200 Subject: [PATCH] OSIS-165: cap getCredentials access-denied recovery to a single retry --- .../service/tenant/ScalityTenantSession.java | 53 ++++++++++++------- .../tenant/ScalityTenantSessionTest.java | 19 +++++++ 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/osis-core/src/main/java/com/scality/osis/service/tenant/ScalityTenantSession.java b/osis-core/src/main/java/com/scality/osis/service/tenant/ScalityTenantSession.java index e5528ed..e4712c4 100644 --- a/osis-core/src/main/java/com/scality/osis/service/tenant/ScalityTenantSession.java +++ b/osis-core/src/main/java/com/scality/osis/service/tenant/ScalityTenantSession.java @@ -87,37 +87,54 @@ public T run(String tenantId, TenantOp op) throws Exception { /** * Gets the tenant's temporary assume-role credentials. * - *

On an access-denied response the role is recreated via - * {@code setupAssumeRole} and the assume-role is retried. + *

On the first access-denied response the role is recreated via + * {@code setupAssumeRole} and the assume-role is retried exactly once. If the + * retry still returns access-denied (a persistent Vault misconfiguration), the + * {@link VaultServiceException} is rethrown rather than recursing again, so the + * call cannot loop forever and overflow the stack. * * @param accountID the account id * @return the credentials */ private Credentials getCredentials(String accountID) { - Credentials credentials; try { - AssumeRoleRequest assumeRoleRequest = ScalityModelConverter.getAssumeRoleRequestForAccount(accountID, - appEnv.getAssumeRoleName()); - logger.debug("[Vault] Assume Role request:{}", assumeRoleRequest); - credentials = vaultAdmin.getTempAccountCredentials(assumeRoleRequest); - logger.debug("[Vault] Assume Role response received with access key:{}, expiration:{}", - credentials.getAccessKeyId(), credentials.getExpiration()); + return assumeRole(accountID); } catch (VaultServiceException e) { + if (!isAccessDenied(e)) { + throw e; + } + // access denied: the osis role is not provisioned yet, set it up and retry once + logger.debug("Assume role not ready for account {} ({}); recreating the role", accountID, e.getReason()); + // Call get Account with Account ID to retrieve account name + AccountData account = vaultAdmin.getAccount(ScalityModelConverter.toGetAccountRequestWithID(accountID)); + asyncScalityOsisService.setupAssumeRole(accountID, account.getName()); - if (!StringUtils.isNullOrEmpty(e.getErrorCode()) && - ACCESS_DENIED.equals(e.getErrorCode())) { - // if access denied, the osis role is not provisioned yet: set it up and retry - logger.debug("Assume role not ready for account {} ({}); recreating the role", accountID, e.getReason()); - // Call get Account with Account ID to retrieve account name - AccountData account = vaultAdmin.getAccount(ScalityModelConverter.toGetAccountRequestWithID(accountID)); - asyncScalityOsisService.setupAssumeRole(accountID, account.getName()); - return getCredentials(accountID); + try { + return assumeRole(accountID); + } catch (VaultServiceException retryError) { + if (isAccessDenied(retryError)) { + logger.error("Assume role still access-denied for account {} after recreating the role; " + + "giving up to avoid unbounded retries", accountID); + } + throw retryError; } - throw e; } + } + + private Credentials assumeRole(String accountID) { + AssumeRoleRequest assumeRoleRequest = ScalityModelConverter.getAssumeRoleRequestForAccount(accountID, + appEnv.getAssumeRoleName()); + logger.debug("[Vault] Assume Role request:{}", assumeRoleRequest); + Credentials credentials = vaultAdmin.getTempAccountCredentials(assumeRoleRequest); + logger.debug("[Vault] Assume Role response received with access key:{}, expiration:{}", + credentials.getAccessKeyId(), credentials.getExpiration()); return credentials; } + private boolean isAccessDenied(VaultServiceException e) { + return !StringUtils.isNullOrEmpty(e.getErrorCode()) && ACCESS_DENIED.equals(e.getErrorCode()); + } + private void generateAdminPolicy(String tenantId) throws Exception { AccountData account = vaultAdmin.getAccount(ScalityModelConverter.toGetAccountRequestWithID(tenantId)); asyncScalityOsisService.setupAdminPolicy(tenantId, account.getName(), appEnv.getAssumeRoleName()); diff --git a/osis-core/src/test/java/com/scality/osis/service/tenant/ScalityTenantSessionTest.java b/osis-core/src/test/java/com/scality/osis/service/tenant/ScalityTenantSessionTest.java index f3d899e..cee9c54 100644 --- a/osis-core/src/test/java/com/scality/osis/service/tenant/ScalityTenantSessionTest.java +++ b/osis-core/src/test/java/com/scality/osis/service/tenant/ScalityTenantSessionTest.java @@ -24,6 +24,7 @@ import java.util.Collections; import java.util.Date; +import static com.scality.osis.utils.ScalityConstants.ACCESS_DENIED; import static com.scality.osis.utils.ScalityTestUtils.*; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.*; @@ -124,6 +125,24 @@ void recreatesRoleOnAccessDeniedThenRetries() { verify(vaultAdminMock, times(2)).getTempAccountCredentials(any(AssumeRoleRequest.class)); } + @Test + void stopsRetryingWhenAccessDeniedPersistsAfterRecreatingRole() { + when(vaultAdminMock.getTempAccountCredentials(any(AssumeRoleRequest.class))) + .thenThrow(new VaultServiceException(HttpStatus.FORBIDDEN, "AccessDenied", + "User: backbeat is not allowed to assume role")); + + // persistent access-denied must surface as a VaultServiceException, not recurse forever + final VaultServiceException error = assertThrows(VaultServiceException.class, + () -> tenantSessionUnderTest.getIamClient(TEST_TENANT_ID)); + assertEquals(ACCESS_DENIED, error.getErrorCode()); + + // recovery routine invoked at most once + verify(asyncScalityOsisServiceMock, times(1)).setupAssumeRole(TEST_TENANT_ID, SAMPLE_TENANT_NAME); + // exactly one recovery means one original attempt plus one retry + verify(vaultAdminMock, times(2)).getTempAccountCredentials(any(AssumeRoleRequest.class)); + verifyNoInteractions(iamMock); + } + @Test void rethrowsNonAccessDeniedVaultError() { when(vaultAdminMock.getTempAccountCredentials(any(AssumeRoleRequest.class)))