Skip to content

Commit

Permalink
Merge pull request #16556 from iterate-ch/bugfix/MD-22429
Browse files Browse the repository at this point in the history 
Synchronize access to in memory tokens
  • Loading branch information
@dkocher
dkocher authored Nov 20, 2024
2 parents e4a59bd + 55b40fa commit 8c9dbd7
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 76 deletions.
8 changes: 4 additions & 4 deletions brick/src/main/java/ch/cyberduck/core/brick/BrickUnauthorizedRetryStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@
import org.apache.logging.log4j.Logger;

import java.io.IOException;
import java.util.concurrent.Semaphore;
import java.util.concurrent.locks.ReentrantLock;

public class BrickUnauthorizedRetryStrategy extends DisabledServiceUnavailableRetryStrategy implements HttpRequestInterceptor {
private static final Logger log = LogManager.getLogger(BrickUnauthorizedRetryStrategy.class);

private final Semaphore semaphore = new Semaphore(1);
private final ReentrantLock lock = new ReentrantLock();
private final HostPasswordStore store = PasswordStoreFactory.get();
private final BrickSession session;
private final LoginCallback prompt;
Expand All @@ -64,7 +64,7 @@ public boolean retryRequest(final HttpResponse response, final int executionCoun
case HttpStatus.SC_UNAUTHORIZED:
log.debug("Try to acquire semaphore for {}", session);
// Pairing token no longer valid
if(!semaphore.tryAcquire()) {
if(!lock.tryLock()) {
log.warn("Skip pairing because semaphore cannot be aquired for {}", session);
return false;
}
Expand All @@ -84,7 +84,7 @@ public boolean retryRequest(final HttpResponse response, final int executionCoun
}
finally {
log.debug("Release semaphore for {}", session);
semaphore.release();
lock.unlock();
}
}
return false;
Expand Down
96 changes: 55 additions & 41 deletions ctera/src/main/java/ch/cyberduck/core/ctera/CteraAuthenticationHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
import java.util.Optional;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.ReentrantLock;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -84,6 +85,7 @@ public class CteraAuthenticationHandler implements ServiceUnavailableRetryStrate
public static final String ATTACH_DEVICE_ACTIVATION_CODE_PATH = "/ServicesPortal/public/users?format=jsonext";
public static final String ATTACH_DEVICE_USERNAME_PATH = "/ServicesPortal/public/users/%s?format=jsonext";

private final ReentrantLock lock = new ReentrantLock();
private final CteraSession session;
private final Host host;
private final LoginCallback prompt;
Expand Down Expand Up @@ -117,23 +119,29 @@ public CteraAuthenticationHandler withInfo(final PublicInfo info) {
}

public CteraTokens validate() throws BackgroundException {
if(tokens.validate()) {
log.debug("Authorize with saved tokens {}", tokens);
}
else {
tokens = this.attach();
}
log.debug("Authorize with tokens {}", tokens);
lock.lock();
try {
this.authorize();
if(tokens.validate()) {
log.debug("Authorize with saved tokens {}", tokens);
}
else {
tokens = this.attach();
}
log.debug("Authorize with tokens {}", tokens);
try {
this.authorize();
}
catch(AccessDeniedException e) {
// Try to re-authenticate with new tokens
log.warn("Failure {} authorizing with tokens {}", e, tokens);
tokens = this.attach();
this.authorize();
}
return tokens;
}
catch(AccessDeniedException e) {
// Try to re-authenticate with new tokens
log.warn("Failure {} authorizing with tokens {}", e, tokens);
tokens = this.attach();
this.authorize();
finally {
lock.unlock();
}
return tokens;
}

/**
Expand Down Expand Up @@ -162,37 +170,43 @@ private CteraTokens attach() throws BackgroundException {
}

public void authorize() throws BackgroundException {
final HttpPost login = new HttpPost(AUTH_PATH);
lock.lock();
try {
login.setEntity(
new StringEntity(String.format("j_username=device%%5c%s&j_password=%s",
tokens.getDeviceId(), tokens.getSharedSecret()), ContentType.APPLICATION_FORM_URLENCODED
)
);
session.getClient().execute(login, new AbstractResponseHandler<Void>() {
@Override
public Void handleResponse(final HttpResponse response) throws IOException {
if(!response.containsHeader("Set-Cookie")) {
log.warn("No cookie in response {}", response);
final HttpPost login = new HttpPost(AUTH_PATH);
try {
login.setEntity(
new StringEntity(String.format("j_username=device%%5c%s&j_password=%s",
tokens.getDeviceId(), tokens.getSharedSecret()), ContentType.APPLICATION_FORM_URLENCODED
)
);
session.getClient().execute(login, new AbstractResponseHandler<Void>() {
@Override
public Void handleResponse(final HttpResponse response) throws IOException {
if(!response.containsHeader("Set-Cookie")) {
log.warn("No cookie in response {}", response);
}
else {
final Header header = response.getFirstHeader("Set-Cookie");
log.debug("Received cookie {}", header);
}
return super.handleResponse(response);
}
else {
final Header header = response.getFirstHeader("Set-Cookie");
log.debug("Received cookie {}", header);
}
return super.handleResponse(response);
}

@Override
public Void handleEntity(final HttpEntity entity) {
return null;
}
});
}
catch(HttpResponseException e) {
throw new DefaultHttpResponseExceptionMappingService().map(e);
@Override
public Void handleEntity(final HttpEntity entity) {
return null;
}
});
}
catch(HttpResponseException e) {
throw new DefaultHttpResponseExceptionMappingService().map(e);
}
catch(IOException e) {
throw new HttpExceptionMappingService().map(e);
}
}
catch(IOException e) {
throw new HttpExceptionMappingService().map(e);
finally {
lock.unlock();
}
}

Expand Down
66 changes: 48 additions & 18 deletions oauth/src/main/java/ch/cyberduck/core/oauth/OAuth2RequestInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@

import java.io.IOException;
import java.util.List;
import java.util.concurrent.locks.ReentrantLock;

import com.google.api.client.auth.oauth2.Credential;

public class OAuth2RequestInterceptor extends OAuth2AuthorizationService implements HttpRequestInterceptor {
private static final Logger log = LogManager.getLogger(OAuth2RequestInterceptor.class);

private final ReentrantLock lock = new ReentrantLock();

/**
* Currently valid tokens
*/
Expand Down Expand Up @@ -75,51 +78,72 @@ public Credentials validate() throws BackgroundException {

@Override
public OAuthTokens authorize() throws BackgroundException {
return tokens = super.authorize();
lock.lock();
try {
return tokens = super.authorize();
}
finally {
lock.unlock();
}
}

/**
* Refresh with cached refresh token
*/
public OAuthTokens refresh() throws BackgroundException {
return tokens = this.refresh(tokens);
lock.lock();
try {
return tokens = this.refresh(tokens);
}
finally {
lock.unlock();
}
}

/**
*
* @param previous Refresh token
*/
@Override
public OAuthTokens refresh(final OAuthTokens previous) throws BackgroundException {
lock.lock();
try {
return tokens = super.refresh(previous);
}
catch(LoginFailureException e) {
log.warn("Failure {} refreshing OAuth tokens", e.getMessage());
return tokens = this.authorize();
}
finally {
lock.unlock();
}
}

@Override
public void process(final HttpRequest request, final HttpContext context) throws HttpException, IOException {
if(tokens.isExpired()) {
try {
final OAuthTokens previous = tokens;
final OAuthTokens refreshed = this.refresh(tokens);
// Skip saving tokens when not changed
if(!refreshed.equals(previous)) {
this.save(refreshed);
lock.lock();
try {
if(tokens.isExpired()) {
try {
final OAuthTokens previous = tokens;
final OAuthTokens refreshed = this.refresh(tokens);
// Skip saving tokens when not changed
if(!refreshed.equals(previous)) {
this.save(refreshed);
}
}
catch(BackgroundException e) {
log.warn("Failure {} refreshing OAuth tokens {}", e, tokens);
// Follow-up error 401 handled in error interceptor
}
}
catch(BackgroundException e) {
log.warn("Failure {} refreshing OAuth tokens {}", e, tokens);
// Follow-up error 401 handled in error interceptor
if(StringUtils.isNotBlank(tokens.getAccessToken())) {
log.info("Authorizing service request with OAuth2 tokens {}", tokens);
request.removeHeaders(HttpHeaders.AUTHORIZATION);
request.addHeader(new BasicHeader(HttpHeaders.AUTHORIZATION, String.format("Bearer %s", tokens.getAccessToken())));
}
}
if(StringUtils.isNotBlank(tokens.getAccessToken())) {
log.info("Authorizing service request with OAuth2 tokens {}", tokens);
request.removeHeaders(HttpHeaders.AUTHORIZATION);
request.addHeader(new BasicHeader(HttpHeaders.AUTHORIZATION, String.format("Bearer %s", tokens.getAccessToken())));
finally {
lock.unlock();
}
}

Expand Down Expand Up @@ -148,6 +172,12 @@ public OAuth2RequestInterceptor withParameter(final String key, final String val
}

public OAuthTokens getTokens() {
return tokens;
lock.lock();
try {
return tokens;
}
finally {
lock.unlock();
}
}
}
36 changes: 23 additions & 13 deletions s3/src/main/java/ch/cyberduck/core/sts/STSAssumeRoleCredentialsRequestInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@
*/

import ch.cyberduck.core.Credentials;
import ch.cyberduck.core.HostPasswordStore;
import ch.cyberduck.core.LoginCallback;
import ch.cyberduck.core.OAuthTokens;
import ch.cyberduck.core.PasswordStoreFactory;
import ch.cyberduck.core.TemporaryAccessTokens;
import ch.cyberduck.core.exception.BackgroundException;
import ch.cyberduck.core.exception.LoginFailureException;
Expand All @@ -38,6 +36,7 @@
import org.jets3t.service.security.AWSSessionCredentials;

import java.io.IOException;
import java.util.concurrent.locks.ReentrantLock;

import com.amazonaws.services.securitytoken.AWSSecurityTokenService;
import com.auth0.jwt.JWT;
Expand All @@ -49,12 +48,13 @@
public class STSAssumeRoleCredentialsRequestInterceptor extends STSAssumeRoleAuthorizationService implements S3CredentialsStrategy, HttpRequestInterceptor {
private static final Logger log = LogManager.getLogger(STSAssumeRoleCredentialsRequestInterceptor.class);

private final ReentrantLock lock = new ReentrantLock();

/**
* Currently valid tokens
*/
private TemporaryAccessTokens tokens = TemporaryAccessTokens.EMPTY;

private final HostPasswordStore store = PasswordStoreFactory.get();
/**
* Handle authentication with OpenID connect retrieving token for STS
*/
Expand All @@ -77,6 +77,7 @@ public STSAssumeRoleCredentialsRequestInterceptor(final OAuth2RequestInterceptor
}

public TemporaryAccessTokens refresh(final OAuthTokens oidc) throws BackgroundException {
lock.lock();
try {
return this.tokens = this.authorize(oidc);
}
Expand All @@ -85,22 +86,31 @@ public TemporaryAccessTokens refresh(final OAuthTokens oidc) throws BackgroundEx
log.warn("Failure {} authorizing. Retry with refreshed OAuth tokens", e.getMessage());
return this.tokens = this.authorize(oauth.refresh(oidc));
}
finally {
lock.unlock();
}
}

@Override
public void process(final HttpRequest request, final HttpContext context) throws HttpException, IOException {
if(tokens.isExpired()) {
try {
this.refresh(oauth.getTokens());
log.info("Authorizing service request with STS tokens {}", tokens);
session.getClient().setProviderCredentials(new AWSSessionCredentials(tokens.getAccessKeyId(), tokens.getSecretAccessKey(),
tokens.getSessionToken()));
}
catch(BackgroundException e) {
log.warn("Failure {} refreshing STS tokens {}", e, tokens);
// Follow-up error 401 handled in error interceptor
lock.lock();
try {
if(tokens.isExpired()) {
try {
this.refresh(oauth.getTokens());
log.info("Authorizing service request with STS tokens {}", tokens);
session.getClient().setProviderCredentials(new AWSSessionCredentials(tokens.getAccessKeyId(), tokens.getSecretAccessKey(),
tokens.getSessionToken()));
}
catch(BackgroundException e) {
log.warn("Failure {} refreshing STS tokens {}", e, tokens);
// Follow-up error 401 handled in error interceptor
}
}
}
finally {
lock.unlock();
}
}

@Override
Expand Down

0 comments on commit 8c9dbd7

Please sign in to comment.