Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
smiroslav merged PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
t-rana commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1609259400 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); Review Comment: I would prefer the first way here since that is tested. Reduced the token refresh interval to 1 minute -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
rootpea commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1604916308 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); Review Comment: Alternatively instead of ClientSecretCredential, you can create directly a ConfidentialClientApplication and set skipCache(true) in the parameters ``` // configure once, then store these for later use in the tokenrefresher: ConfidentialClientApplication confidentialClientApplication = ConfidentialClientApplication .builder(clientId, ClientCredentialFactory.createFromSecret(clientSecret)) .authority("https://login.microsoftonline.com/; + tenantId + "/") .build(); ClientCredentialParameters clientCredentialParameters = ClientCredentialParameters.builder(Collections.singleton(AZURE_DEFAULT_SCOPE)).skipCache(true) .tenant(IdentityUtil .resolveTenantId(tenantId, new TokenRequestContext().addScopes(AZURE_DEFAULT_SCOPE), new IdentityClientOptions())).build(); // refreshing anytime: IAuthenticationResult authResult = confidentialClientApplication.acquireToken(clientCredentialParameters).join(); AccessToken accessToken = new AccessToken(authResult.accessToken(), OffsetDateTime.ofInstant(authResult.expiresOnDate().toInstant(), ZoneOffset.UTC)); ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
rootpea commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1604916308 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); Review Comment: Alternatively instead of ClientSecretCredential, you can create directly a ConfidentialClientApplication and set skipCache(true) in the parameters ``` // configure once, then store these for later use in the tokenrefresher: ConfidentialClientApplication confidentialClientApplication = ConfidentialClientApplication .builder(client_id, ClientCredentialFactory.createFromSecret(client_secret)) .authority("https://login.microsoftonline.com/; + tenant_id + "/") .build(); ClientCredentialParameters clientCredentialParameters = ClientCredentialParameters.builder(Collections.singleton(AZURE_DEFAULT_SCOPE)).skipCache(true) .tenant(IdentityUtil .resolveTenantId(tenant_id, new TokenRequestContext().addScopes(AZURE_DEFAULT_SCOPE), new IdentityClientOptions())).build(); // refreshing anytime: IAuthenticationResult authResult = confidentialClientApplication.acquireToken(clientCredentialParameters).join(); AccessToken accessToken = new AccessToken(authResult.accessToken(), OffsetDateTime.ofInstant(authResult.expiresOnDate().toInstant(), ZoneOffset.UTC)); ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
rootpea commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1604916308 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); Review Comment: Alternatively instead of ClientSecretCredential, you can create directly a ConfidentialClientApplication and set skipCache(true) in the parameters ``` // configure once, then use these for later use in the tokenrefresher: ConfidentialClientApplication confidentialClientApplication = ConfidentialClientApplication .builder(client_id, ClientCredentialFactory.createFromSecret(client_secret)) .authority("https://login.microsoftonline.com/; + tenant_id + "/") .build(); ClientCredentialParameters clientCredentialParameters = ClientCredentialParameters.builder(Collections.singleton(AZURE_DEFAULT_SCOPE)).skipCache(true) .tenant(IdentityUtil .resolveTenantId(tenant_id, new TokenRequestContext().addScopes(AZURE_DEFAULT_SCOPE), new IdentityClientOptions())).build(); // refreshing anytime: IAuthenticationResult authResult = confidentialClientApplication.acquireToken(clientCredentialParameters).join(); AccessToken accessToken = new AccessToken(authResult.accessToken(), OffsetDateTime.ofInstant(authResult.expiresOnDate().toInstant(), ZoneOffset.UTC)); ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
rootpea commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1604794648 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); Review Comment: Thanks for the clarification. In this case we should reduce the TOKEN_REFRESHER_DELAY to 1 minute so that even in the case of transient failures we will still successfully refresh before the token expires. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
t-rana commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1604654469 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); Review Comment: I guess you are referring to an older version of azure identity. we are using azure-identity@1.11.3. The token is first looked up in cache, and after the cache miss, a new token is generated. https://github.com/Azure/azure-sdk-for-java/blob/azure-identity_1.11.3/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientSecretCredential.java#L126 https://github.com/Azure/azure-sdk-for-java/blob/azure-identity_1.11.3/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentitySyncClient.java#L176 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
t-rana commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1604654469 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); Review Comment: I guess you are referring to an older version of azure identity. we are using azure-identity@1.11.3. The token is first looked up in cache, and after the cache miss, a new token is generated. https://github.com/Azure/azure-sdk-for-java/blob/azure-identity_1.11.3/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentitySyncClient.java#L176 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
rootpea commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1604489667 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); Review Comment: There is no cache in IdentityClient.authenticateWithClientSecret. So the token will definitely be refreshed even at 20 minutes. With 5 minutes there is a risk of a single transient failure causing token expiration. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
t-rana commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1604391789 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); +if (accessToken.getExpiresAt() != null && accessToken.getExpiresAt().isBefore(tokenExpiryThreshold)) { +log.info("Access token is about to expire (5 minutes or less) at: {}. New access token will be generated", Review Comment: same as above. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
t-rana commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1604391435 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); Review Comment: azure identity serves the token from cache, and will only refresh the token when there are 5 minutes left in token expiry. so keeping a 20 minutes threshold will not help as the token will be served from cache only and will not be refreshed. hence i have taken a threshold of 5 minutes similar to what is being used in azure identity. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
rootpea commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1602892162 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); +if (accessToken.getExpiresAt() != null && accessToken.getExpiresAt().isBefore(tokenExpiryThreshold)) { +log.info("Access token is about to expire (5 minutes or less) at: {}. New access token will be generated", Review Comment: ```suggestion log.info("Access token is about to expire (20 minutes or less) at: {}. New access token will be generated", ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
rootpea commented on code in PR #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441#discussion_r1602891748 ## oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java: ## @@ -207,6 +230,61 @@ public void write(@NotNull byte[] bytes, int offset, int length) { } } +/** + * This class represents a token refresher responsible for ensuring the validity of the access token used for azure AD authentication. + * The access token generated by the Azure client is valid for 1 hour only. Therefore, this class periodically checks the validity + * of the access token and refreshes it if necessary. The refresh is triggered when the current access token is about to expire, + * defined by a threshold of 5 minutes from the current time. This threshold is similar to what is being used in azure identity to + * generate a new token + */ +private static class TokenRefresher implements Runnable { + +private final ClientSecretCredential clientSecretCredential; +private AccessToken accessToken; +private final StorageCredentialsToken storageCredentialsToken; + + +/** + * Constructs a new TokenRefresher object with the specified parameters. + * + * @param clientSecretCredential The client secret credential used to obtain the access token. + * @param accessToken The current access token. + * @param storageCredentialsToken The storage credentials token associated with the access token. + */ +public TokenRefresher(ClientSecretCredential clientSecretCredential, + AccessToken accessToken, + StorageCredentialsToken storageCredentialsToken) { +this.clientSecretCredential = clientSecretCredential; +this.accessToken = accessToken; +this.storageCredentialsToken = storageCredentialsToken; +} + +@Override +public void run() { +try { +log.debug("Checking for azure access token expiry at: {}", LocalDateTime.now()); +OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(5); Review Comment: ```suggestion OffsetDateTime tokenExpiryThreshold = OffsetDateTime.now().plusMinutes(20); ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10780: add azure access token refresh logic [jackrabbit-oak]
t-rana opened a new pull request, #1441: URL: https://github.com/apache/jackrabbit-oak/pull/1441 Azure Access token is valid for 1 hour only. Added code to check for token expiry every 4 minutes, and refresh the token when the same is close to expiry. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org