Review locking.

Author: dkocher

s3/src/main/java/ch/cyberduck/core/sts/STSAssumeRoleCredentialsStrategy.java

@@ -29,15 +29,12 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-import java.util.concurrent.locks.ReentrantLock;
-
 /**
  * Swap static access key id and secret access key with temporary credentials obtained from STS AssumeRole
  */
 public class STSAssumeRoleCredentialsStrategy extends STSCredentialsStrategy implements S3CredentialsStrategy {
     private static final Logger log = LogManager.getLogger(STSAssumeRoleCredentialsStrategy.class);
 
-    private final ReentrantLock lock = new ReentrantLock();
     private final Host host;
 
     public STSAssumeRoleCredentialsStrategy(final Host host, final X509TrustManager trust, final X509KeyManager key, final LoginCallback prompt) {
@@ -47,14 +44,8 @@ public STSAssumeRoleCredentialsStrategy(final Host host, final X509TrustManager
 
     @Override
     public TemporaryAccessTokens refresh(final Credentials credentials) throws BackgroundException {
-        lock.lock();
-        try {
-            final String arn = new ProxyPreferencesReader(host, credentials).getProperty(Profile.STS_ROLE_ARN_PROPERTY_KEY, "s3.assumerole.rolearn");
-            log.debug("Retrieve temporary credentials with {} for role ARN {}", credentials, arn);
-            return this.assumeRole(credentials, arn);
-        }
-        finally {
-            lock.unlock();
-        }
+        final String arn = new ProxyPreferencesReader(host, credentials).getProperty(Profile.STS_ROLE_ARN_PROPERTY_KEY, "s3.assumerole.rolearn");
+        log.debug("Retrieve temporary credentials with {} for role ARN {}", credentials, arn);
+        return this.assumeRole(credentials, arn);
     }
 }

s3/src/main/java/ch/cyberduck/core/sts/STSAssumeRoleWithWebIdentityCredentialsStrategy.java

@@ -31,16 +31,12 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-import java.util.concurrent.locks.ReentrantLock;
-
 /**
  * Swap OIDC Id token for temporary security credentials
  */
 public class STSAssumeRoleWithWebIdentityCredentialsStrategy extends STSCredentialsStrategy implements S3CredentialsStrategy {
     private static final Logger log = LogManager.getLogger(STSAssumeRoleWithWebIdentityCredentialsStrategy.class);
 
-    private final ReentrantLock lock = new ReentrantLock();
-
     /**
      * Handle authentication with OpenID connect retrieving token for STS
      */
@@ -57,7 +53,6 @@ public STSAssumeRoleWithWebIdentityCredentialsStrategy(final OAuth2RequestInterc
 
     @Override
     public TemporaryAccessTokens refresh(final Credentials credentials) throws BackgroundException {
-        lock.lock();
         final String arn = new ProxyPreferencesReader(host, credentials).getProperty(Profile.STS_ROLE_ARN_PROPERTY_KEY, "s3.assumerole.rolearn");
         try {
             log.debug("Retrieve temporary credentials with {} for role ARN {}", credentials, arn);
@@ -68,8 +63,5 @@ public TemporaryAccessTokens refresh(final Credentials credentials) throws Backg
             log.warn("Failure {} authorizing. Retry with refreshed OAuth tokens", e.getMessage());
             return this.assumeRoleWithWebIdentity(oauth.authorize(), arn);
         }
-        finally {
-            lock.unlock();
-        }
     }
 }

s3/src/main/java/ch/cyberduck/core/sts/STSCredentialsStrategy.java

@@ -24,11 +24,18 @@
 import ch.cyberduck.core.ssl.X509KeyManager;
 import ch.cyberduck.core.ssl.X509TrustManager;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.util.concurrent.locks.ReentrantLock;
+
 /**
  * Swap static access key id and secret access key with temporary credentials obtained from STS AssumeRole
  */
 public abstract class STSCredentialsStrategy extends STSAuthorizationService implements S3CredentialsStrategy {
+    private static final Logger log = LogManager.getLogger(STSCredentialsStrategy.class);
 
+    private final ReentrantLock lock = new ReentrantLock();
     private final Host host;
 
     public STSCredentialsStrategy(final Host host, final X509TrustManager trust, final X509KeyManager key, final LoginCallback prompt) {
@@ -46,9 +53,19 @@ public STSCredentialsStrategy(final Host host, final X509TrustManager trust, fin
 
     @Override
     public Credentials get() throws BackgroundException {
-        final Credentials credentials = host.getCredentials();
-        final TemporaryAccessTokens tokens = credentials.getTokens();
-        // Get temporary credentials from STS using static long-lived credentials
-        return credentials.setTokens(tokens.isExpired() ? this.refresh(credentials) : tokens);
+        lock.lock();
+        try {
+            final Credentials credentials = host.getCredentials();
+            final TemporaryAccessTokens tokens = credentials.getTokens();
+            // Get temporary credentials from STS using static long-lived credentials
+            if(tokens.isExpired()) {
+                log.debug("Refresh expired tokens {} for {}", tokens, host);
+                credentials.setTokens(this.refresh(credentials));
+            }
+            return credentials;
+        }
+        finally {
+            lock.unlock();
+        }
     }
 }

s3/src/main/java/ch/cyberduck/core/sts/STSGetSessionTokenCredentialsStrategy.java

@@ -29,16 +29,12 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-import java.util.concurrent.locks.ReentrantLock;
-
 /**
  * Swap static access key id and secret access key with temporary credentials obtained from STS AssumeRole
  */
 public class STSGetSessionTokenCredentialsStrategy extends STSCredentialsStrategy implements S3CredentialsStrategy {
     private static final Logger log = LogManager.getLogger(STSGetSessionTokenCredentialsStrategy.class);
 
-    private final ReentrantLock lock = new ReentrantLock();
-
     private final Host host;
 
     public STSGetSessionTokenCredentialsStrategy(final Host host, final X509TrustManager trust, final X509KeyManager key, final LoginCallback prompt) {
@@ -48,14 +44,8 @@ public STSGetSessionTokenCredentialsStrategy(final Host host, final X509TrustMan
 
     @Override
     public TemporaryAccessTokens refresh(final Credentials credentials) throws BackgroundException {
-        lock.lock();
-        try {
-            final String arn = new ProxyPreferencesReader(host, credentials).getProperty(Profile.STS_MFA_ARN_PROPERTY_KEY);
-            log.debug("Retrieve temporary credentials with {} for role ARN {}", credentials, arn);
-            return this.getSessionToken(credentials, arn);
-        }
-        finally {
-            lock.unlock();
-        }
+        final String arn = new ProxyPreferencesReader(host, credentials).getProperty(Profile.STS_MFA_ARN_PROPERTY_KEY);
+        log.debug("Retrieve temporary credentials with {} for role ARN {}", credentials, arn);
+        return this.getSessionToken(credentials, arn);
     }
 }