Skip to content

Commit

Permalink
Bug 578554 - Review comments
Browse files Browse the repository at this point in the history 
Change-Id: I9b2719d2c04e11095356d5b203cabe808fc23379
Signed-off-by: Leif Geiger <leif.geiger@yatta.de>
  • Loading branch information
@l3-g5
l3-g5 committed Apr 29, 2022
1 parent 7889e9e commit 49d695b
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 76 deletions.
96 changes: 28 additions & 68 deletions ...se/epp/internal/mpc/core/transport/httpclient/CacheCredentialsAuthenticationStrategy.java
View file
Expand Up @@ -12,107 +12,67 @@
*******************************************************************************/
package org.eclipse.epp.internal.mpc.core.transport.httpclient;

import java.util.List;
import java.util.Map;
import java.io.IOException;

import org.apache.hc.client5.http.AuthenticationStrategy;
import org.apache.hc.client5.http.auth.AuthChallenge;
import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.auth.AuthScheme;
import org.apache.hc.client5.http.auth.AuthScope;
import org.apache.hc.client5.http.auth.ChallengeType;
import org.apache.hc.client5.http.auth.Credentials;
import org.apache.hc.client5.http.auth.CredentialsProvider;
import org.apache.hc.client5.http.auth.CredentialsStore;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.http.EntityDetails;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.HttpResponseInterceptor;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.protocol.HttpContext;

abstract class CacheCredentialsAuthenticationStrategy implements AuthenticationStrategy {
class CacheCredentialsAuthenticationStrategy implements HttpResponseInterceptor {

public static final String CREDENTIALS_CACHE_ATTRIBUTE = CacheCredentialsAuthenticationStrategy.class.getName()
+ ".credentialsCache"; //$NON-NLS-1$

static class Target extends CacheCredentialsAuthenticationStrategy { // TODO httpclient5: not needed like that

public Target(AuthenticationStrategy delegate) {
super(delegate);
}

@Override
protected CredentialsProvider getAuthState(HttpClientContext clientContext) {
return clientContext.getCredentialsProvider();
}

}

static class Proxy extends CacheCredentialsAuthenticationStrategy { // TODO httpclient5: not needed like that

public Proxy(AuthenticationStrategy delegate) {
super(delegate);
}

@Override
protected CredentialsProvider getAuthState(HttpClientContext clientContext) {
return clientContext.getCredentialsProvider();
}
}

private final AuthenticationStrategy delegate;

public CacheCredentialsAuthenticationStrategy(AuthenticationStrategy delegate) {
this.delegate = delegate;
}
public static final String CURRENT_CREDENTIALS = CacheCredentialsAuthenticationStrategy.class.getName()
+ ".currentCredentials"; //$NON-NLS-1$

@Override
public List<AuthScheme> select(ChallengeType challengeType, Map<String, AuthChallenge> challenges,
HttpContext context) {
return delegate.select(challengeType, challenges, context);
}

// TODO httpclient5: @Override
public void authSucceeded(HttpHost authhost, AuthScheme authScheme, HttpContext context) {
// TODO httpclient5: delegate.authSucceeded(authhost, authScheme, context);
if (authScheme != null && authScheme.isChallengeComplete()) {
cacheCredentials(authhost, authScheme, context);
public void process(HttpResponse response, EntityDetails entity, HttpContext context)
throws HttpException, IOException {
final HttpClientContext clientContext = HttpClientContext.adapt(context);
HttpRoute route = (HttpRoute) clientContext.getHttpRoute();
HttpHost authhost = route.getProxyHost();
// also for target: HttpHost authhost = AuthSupport.resolveAuthTarget(clientContext.getRequest(), route);
AuthScheme authScheme = clientContext.getAuthExchange(authhost).getAuthScheme();
if (response.getCode() == HttpStatus.SC_UNAUTHORIZED
|| response.getCode() == HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED) {
uncacheCredentials(authhost, authScheme, clientContext);
} else {
if (authScheme != null && authScheme.isChallengeComplete()) {
cacheCredentials(authhost, authScheme, clientContext);
}
}
}

// TODO httpclient5: @Override
public void authFailed(HttpHost authhost, AuthScheme authScheme, HttpContext context) {
// TODO httpclient5: delegate.authFailed(authhost, authScheme, context);
uncacheCredentials(authhost, authScheme, context);
}

private void uncacheCredentials(HttpHost authhost, AuthScheme authScheme, HttpContext context) {
private void uncacheCredentials(HttpHost authhost, AuthScheme authScheme, HttpClientContext context) {
CredentialsStore credentialsCache = getCredentialsCache(context);
if (credentialsCache != null) {
AuthScope scope = createAuthScope(authhost, authScheme);
credentialsCache.setCredentials(scope, null);
}
}

private void cacheCredentials(HttpHost authhost, AuthScheme authScheme, HttpContext context) {
Credentials credentials = getCredentials(context);
private void cacheCredentials(HttpHost authhost, AuthScheme authScheme, HttpClientContext context) {
AuthScope scope = createAuthScope(authhost, authScheme);
Credentials credentials = context.getAttribute(CURRENT_CREDENTIALS, Credentials.class);
if (credentials != null) {
CredentialsStore credentialsCache = getCredentialsCache(context);
if (credentialsCache != null) {
AuthScope scope = createAuthScope(authhost, authScheme);
credentialsCache.setCredentials(scope, credentials);
}
}
}

protected Credentials getCredentials(HttpContext context) {
HttpClientContext clientContext = HttpClientContext.adapt(context);
CredentialsProvider authState = getAuthState(clientContext);
if (authState != null) {
return authState.getCredentials(null, context);
}
return null;
}

protected abstract CredentialsProvider getAuthState(HttpClientContext clientContext);

private static CredentialsStore getCredentialsCache(HttpContext context) {
CredentialsStore credentialsCache = null;
Object value = context.getAttribute(CREDENTIALS_CACHE_ATTRIBUTE);
Expand Down
4 changes: 3 additions & 1 deletion ...rc/org/eclipse/epp/internal/mpc/core/transport/httpclient/ChainedCredentialsProvider.java
View file
Expand Up @@ -40,7 +40,9 @@ public Credentials getCredentials(AuthScope authscope, HttpContext context) {
if (credentials != null) {
return credentials;
}
return second.getCredentials(authscope, context);
credentials = second.getCredentials(authscope, context);
context.setAttribute(CacheCredentialsAuthenticationStrategy.CURRENT_CREDENTIALS, credentials);
return credentials;
}

@Override
Expand Down
6 changes: 1 addition & 5 deletions ...pc.core/src/org/eclipse/epp/internal/mpc/core/transport/httpclient/HttpClientFactory.java
View file
Expand Up @@ -18,7 +18,6 @@
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.cookie.BasicCookieStore;
import org.apache.hc.client5.http.cookie.CookieStore;
import org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy;
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
import org.apache.hc.core5.http.HttpRequestInterceptor;
Expand Down Expand Up @@ -120,10 +119,7 @@ protected HttpClientBuilder builder() {
builder.setConnectionManager(connManager);
setClientDefaultTimeouts(builder);

builder.setTargetAuthenticationStrategy(
new CacheCredentialsAuthenticationStrategy.Target(DefaultAuthenticationStrategy.INSTANCE)); // TODO httpclient5: was TargetAuthenticationStrategy.INSTANCE
builder.setProxyAuthenticationStrategy(
new CacheCredentialsAuthenticationStrategy.Proxy(DefaultAuthenticationStrategy.INSTANCE)); // TODO httpclient5: was ProxyAuthenticationStrategy.INSTANCE
builder.addResponseInterceptorLast(new CacheCredentialsAuthenticationStrategy());

builder.setUserAgent(HttpClientTransport.USER_AGENT);

Expand Down
2 changes: 1 addition & 1 deletion ...pc.core/src/org/eclipse/epp/internal/mpc/core/transport/httpclient/HttpClientService.java
View file
Expand Up @@ -121,7 +121,7 @@ private static ClassicHttpRequest setConfig(ClassicHttpRequest request, RequestC
if (request instanceof HttpUriRequestBase) {
((HttpUriRequestBase) request).setConfig(config);
} else {
// TODO httpclient5: how to set the config
// could not set the config, here
request = ClassicRequestBuilder.copy(request)/*.setConfig(config)*/.build();
}
return request;
Expand Down
1 change: 0 additions & 1 deletion ...eclipse.epp.mpc.tests/src/org/eclipse/epp/mpc/tests/service/UserFavoritesServiceTest.java
View file
Expand Up @@ -22,7 +22,6 @@
import java.net.URI;
import java.util.List;

import org.apache.hc.core5.http.ClassicHttpRequest;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.epp.internal.mpc.core.ServiceLocator;
Expand Down

0 comments on commit 49d695b

Please sign in to comment.