Fixup SCM URL matching sometimes fails
parent
943273d82d
commit
f2ab7bd680
|
|
@ -39,6 +39,7 @@ import org.eclipse.che.api.factory.server.scm.exception.ScmConfigurationPersiste
|
|||
import org.eclipse.che.api.factory.server.scm.exception.UnsatisfiedScmPreconditionException;
|
||||
import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
|
||||
import org.eclipse.che.commons.lang.NameGenerator;
|
||||
import org.eclipse.che.commons.lang.StringUtils;
|
||||
import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory;
|
||||
import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta;
|
||||
import org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory;
|
||||
|
|
@ -103,7 +104,10 @@ public class KubernetesGitCredentialManager implements GitCredentialManager {
|
|||
s.getMetadata().getAnnotations().get(ANNOTATION_GIT_CREDENTIALS))
|
||||
&& personalAccessToken
|
||||
.getScmProviderUrl()
|
||||
.equals(s.getMetadata().getAnnotations().get(ANNOTATION_SCM_URL))
|
||||
.equals(
|
||||
StringUtils.trimEnd(
|
||||
s.getMetadata().getAnnotations().get(ANNOTATION_SCM_URL),
|
||||
'/'))
|
||||
&& personalAccessToken
|
||||
.getCheUserId()
|
||||
.equals(s.getMetadata().getAnnotations().get(ANNOTATION_CHE_USERID))
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ import org.eclipse.che.api.factory.server.scm.exception.UnknownScmProviderExcept
|
|||
import org.eclipse.che.api.factory.server.scm.exception.UnsatisfiedScmPreconditionException;
|
||||
import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
|
||||
import org.eclipse.che.commons.lang.NameGenerator;
|
||||
import org.eclipse.che.commons.lang.StringUtils;
|
||||
import org.eclipse.che.commons.subject.Subject;
|
||||
import org.eclipse.che.workspace.infrastructure.kubernetes.KubernetesClientFactory;
|
||||
import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta;
|
||||
|
|
@ -143,11 +144,12 @@ public class KubernetesPersonalAccessTokenManager implements PersonalAccessToken
|
|||
.get(KUBERNETES_PERSONAL_ACCESS_TOKEN_LABEL_SELECTOR);
|
||||
for (Secret secret : secrets) {
|
||||
Map<String, String> annotations = secret.getMetadata().getAnnotations();
|
||||
String trimmedUrl = StringUtils.trimEnd(annotations.get(ANNOTATION_SCM_URL), '/');
|
||||
if (annotations.get(ANNOTATION_CHE_USERID).equals(cheUser.getUserId())
|
||||
&& annotations.get(ANNOTATION_SCM_URL).equals(scmServerUrl)) {
|
||||
&& trimmedUrl.equals(StringUtils.trimEnd(scmServerUrl, '/'))) {
|
||||
PersonalAccessToken token =
|
||||
new PersonalAccessToken(
|
||||
annotations.get(ANNOTATION_SCM_URL),
|
||||
trimmedUrl,
|
||||
annotations.get(ANNOTATION_CHE_USERID),
|
||||
annotations.get(ANNOTATION_SCM_USERNAME),
|
||||
annotations.get(ANNOTATION_SCM_USERID),
|
||||
|
|
|
|||
|
|
@ -129,7 +129,7 @@ public class KubernetesGitCredentialManagerTest {
|
|||
|
||||
Map<String, String> annotations = new HashMap<>(DEFAULT_SECRET_ANNOTATIONS);
|
||||
|
||||
annotations.put(ANNOTATION_SCM_URL, token.getScmProviderUrl());
|
||||
annotations.put(ANNOTATION_SCM_URL, token.getScmProviderUrl() + "/");
|
||||
annotations.put(ANNOTATION_SCM_USERNAME, token.getScmUserName());
|
||||
annotations.put(ANNOTATION_CHE_USERID, token.getCheUserId());
|
||||
ObjectMeta objectMeta =
|
||||
|
|
|
|||
|
|
@ -150,7 +150,7 @@ public class KubernetesPersonalAccessTokenManagerTest {
|
|||
ObjectMeta meta3 =
|
||||
new ObjectMetaBuilder()
|
||||
.withAnnotations(
|
||||
Map.of(ANNOTATION_CHE_USERID, "user2", ANNOTATION_SCM_URL, "http://host2"))
|
||||
Map.of(ANNOTATION_CHE_USERID, "user2", ANNOTATION_SCM_URL, "http://host3"))
|
||||
.build();
|
||||
|
||||
Secret secret1 = new SecretBuilder().withMetadata(meta1).withData(data1).build();
|
||||
|
|
@ -172,6 +172,53 @@ public class KubernetesPersonalAccessTokenManagerTest {
|
|||
assertEquals(token.getToken(), "token1");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testGetTokenFromNamespaceWithTrailingSlashMismatch() throws Exception {
|
||||
|
||||
KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
|
||||
when(namespaceFactory.list()).thenReturn(Collections.singletonList(meta));
|
||||
KubernetesNamespace kubernetesnamespace = Mockito.mock(KubernetesNamespace.class);
|
||||
KubernetesSecrets secrets = Mockito.mock(KubernetesSecrets.class);
|
||||
when(namespaceFactory.access(eq(null), eq(meta.getName()))).thenReturn(kubernetesnamespace);
|
||||
when(kubernetesnamespace.secrets()).thenReturn(secrets);
|
||||
when(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessToken.class))).thenReturn(true);
|
||||
|
||||
Map<String, String> data1 =
|
||||
Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));
|
||||
Map<String, String> data2 =
|
||||
Map.of("token", Base64.getEncoder().encodeToString("token2".getBytes(UTF_8)));
|
||||
|
||||
ObjectMeta meta1 =
|
||||
new ObjectMetaBuilder()
|
||||
.withAnnotations(
|
||||
Map.of(ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, "http://host1.com/"))
|
||||
.build();
|
||||
ObjectMeta meta2 =
|
||||
new ObjectMetaBuilder()
|
||||
.withAnnotations(
|
||||
Map.of(ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, "http://host2.com"))
|
||||
.build();
|
||||
|
||||
Secret secret1 = new SecretBuilder().withMetadata(meta1).withData(data1).build();
|
||||
Secret secret2 = new SecretBuilder().withMetadata(meta2).withData(data2).build();
|
||||
|
||||
when(secrets.get(any(LabelSelector.class))).thenReturn(Arrays.asList(secret1, secret2));
|
||||
|
||||
// when
|
||||
PersonalAccessToken token1 =
|
||||
personalAccessTokenManager
|
||||
.get(new SubjectImpl("user", "user1", "t1", false), "http://host1.com")
|
||||
.get();
|
||||
PersonalAccessToken token2 =
|
||||
personalAccessTokenManager
|
||||
.get(new SubjectImpl("user", "user1", "t1", false), "http://host2.com/")
|
||||
.get();
|
||||
|
||||
// then
|
||||
assertNotNull(token1);
|
||||
assertNotNull(token2);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldDeleteInvalidTokensOnGet() throws Exception {
|
||||
// given
|
||||
|
|
|
|||
|
|
@ -13,8 +13,11 @@ package org.eclipse.che.security.oauth1;
|
|||
|
||||
import static com.google.common.base.Strings.isNullOrEmpty;
|
||||
|
||||
import com.google.common.base.Splitter;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
import java.util.Set;
|
||||
import java.util.stream.Collectors;
|
||||
import javax.inject.Inject;
|
||||
import javax.inject.Named;
|
||||
import javax.inject.Provider;
|
||||
|
|
@ -23,6 +26,7 @@ import org.eclipse.che.api.factory.server.bitbucket.server.BitbucketServerApiCli
|
|||
import org.eclipse.che.api.factory.server.bitbucket.server.HttpBitbucketServerApiClient;
|
||||
import org.eclipse.che.api.factory.server.bitbucket.server.NoopBitbucketServerApiClient;
|
||||
import org.eclipse.che.commons.annotation.Nullable;
|
||||
import org.eclipse.che.commons.lang.StringUtils;
|
||||
import org.eclipse.che.inject.ConfigurationException;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
|
@ -49,17 +53,25 @@ public class BitbucketServerApiProvider implements Provider<BitbucketServerApiCl
|
|||
}
|
||||
|
||||
private static BitbucketServerApiClient doGet(
|
||||
String bitbucketEndpoints,
|
||||
String rawBitbucketEndpoints,
|
||||
String bitbucketOauth1Endpoint,
|
||||
Set<OAuthAuthenticator> authenticators) {
|
||||
if (isNullOrEmpty(bitbucketOauth1Endpoint)) {
|
||||
return new NoopBitbucketServerApiClient();
|
||||
} else {
|
||||
if (isNullOrEmpty(bitbucketEndpoints)) {
|
||||
if (isNullOrEmpty(rawBitbucketEndpoints)) {
|
||||
throw new ConfigurationException(
|
||||
"`che.integration.bitbucket.server_endpoints` bitbucket configuration is missing."
|
||||
+ " It should contain values from 'che.oauth1.bitbucket.endpoint'");
|
||||
} else {
|
||||
// sanitise URL-s first
|
||||
bitbucketOauth1Endpoint = StringUtils.trimEnd(bitbucketOauth1Endpoint, '/');
|
||||
List<String> bitbucketEndpoints =
|
||||
Splitter.on(",")
|
||||
.splitToList(rawBitbucketEndpoints)
|
||||
.stream()
|
||||
.map(s -> StringUtils.trimEnd(s, '/'))
|
||||
.collect(Collectors.toList());
|
||||
if (bitbucketEndpoints.contains(bitbucketOauth1Endpoint)) {
|
||||
Optional<OAuthAuthenticator> authenticator =
|
||||
authenticators
|
||||
|
|
|
|||
|
|
@ -52,6 +52,22 @@ public class BitbucketServerApiClientProviderTest {
|
|||
assertTrue(HttpBitbucketServerApiClient.class.isAssignableFrom(actual.getClass()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldNormalizeURLsBeforeCreateBitbucketServerApi() {
|
||||
// given
|
||||
BitbucketServerApiProvider bitbucketServerApiProvider =
|
||||
new BitbucketServerApiProvider(
|
||||
"https://bitbucket.server.com/, https://bitbucket2.server.com/",
|
||||
"https://bitbucket.server.com/",
|
||||
ImmutableSet.of(oAuthAuthenticator));
|
||||
// when
|
||||
BitbucketServerApiClient actual = bitbucketServerApiProvider.get();
|
||||
// then
|
||||
assertNotNull(actual);
|
||||
// internal representation always w/out slashes
|
||||
assertTrue(actual.isConnected("https://bitbucket.server.com"));
|
||||
}
|
||||
|
||||
@Test(dataProvider = "noopConfig")
|
||||
public void shouldProvideNoopOAuthAuthenticatorIfSomeConfigurationIsNotSet(
|
||||
@Nullable String bitbucketEndpoints,
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl;
|
|||
*/
|
||||
public class GithubUrl implements RemoteFactoryUrl {
|
||||
|
||||
private static final String HOSTNAME = "https://github.com/";
|
||||
private static final String HOSTNAME = "https://github.com";
|
||||
|
||||
/** Username part of github URL */
|
||||
private String username;
|
||||
|
|
@ -173,6 +173,6 @@ public class GithubUrl implements RemoteFactoryUrl {
|
|||
* @return location of the repository.
|
||||
*/
|
||||
protected String repositoryLocation() {
|
||||
return HOSTNAME + this.username + "/" + this.repository;
|
||||
return HOSTNAME + "/" + this.username + "/" + this.repository;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ import org.eclipse.che.api.factory.server.scm.exception.ScmItemNotFoundException
|
|||
import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException;
|
||||
import org.eclipse.che.commons.annotation.Nullable;
|
||||
import org.eclipse.che.commons.lang.NameGenerator;
|
||||
import org.eclipse.che.commons.lang.StringUtils;
|
||||
import org.eclipse.che.commons.subject.Subject;
|
||||
import org.eclipse.che.security.oauth.OAuthAPI;
|
||||
import org.slf4j.Logger;
|
||||
|
|
@ -45,6 +46,7 @@ public class GitlabOAuthTokenFetcher implements PersonalAccessTokenFetcher {
|
|||
private static final String OAUTH_PROVIDER_NAME = "gitlab";
|
||||
public static final Set<String> DEFAULT_TOKEN_SCOPES =
|
||||
ImmutableSet.of("api", "write_repository", "openid");
|
||||
public static final String OAUTH_2_PREFIX = "oauth2-";
|
||||
|
||||
private final OAuthAPI oAuthAPI;
|
||||
private final String apiEndpoint;
|
||||
|
|
@ -58,8 +60,8 @@ public class GitlabOAuthTokenFetcher implements PersonalAccessTokenFetcher {
|
|||
this.apiEndpoint = apiEndpoint;
|
||||
this.oAuthAPI = oAuthAPI;
|
||||
if (gitlabEndpoints != null) {
|
||||
this.gitlabApiClient =
|
||||
new GitlabApiClient(Splitter.on(",").splitToList(gitlabEndpoints).get(0));
|
||||
final String oAuthEndpoint = Splitter.on(",").splitToList(gitlabEndpoints).get(0);
|
||||
this.gitlabApiClient = new GitlabApiClient(StringUtils.trimEnd(oAuthEndpoint, '/'));
|
||||
} else {
|
||||
this.gitlabApiClient = null;
|
||||
}
|
||||
|
|
@ -68,6 +70,7 @@ public class GitlabOAuthTokenFetcher implements PersonalAccessTokenFetcher {
|
|||
@Override
|
||||
public PersonalAccessToken fetchPersonalAccessToken(Subject cheSubject, String scmServerUrl)
|
||||
throws ScmUnauthorizedException, ScmCommunicationException {
|
||||
scmServerUrl = StringUtils.trimEnd(scmServerUrl, '/');
|
||||
if (gitlabApiClient == null || !gitlabApiClient.isConnected(scmServerUrl)) {
|
||||
LOG.debug("not a valid url {} for current fetcher ", scmServerUrl);
|
||||
return null;
|
||||
|
|
@ -82,7 +85,7 @@ public class GitlabOAuthTokenFetcher implements PersonalAccessTokenFetcher {
|
|||
cheSubject.getUserId(),
|
||||
user.getUsername(),
|
||||
Long.toString(user.getId()),
|
||||
NameGenerator.generate("oauth2-", 5),
|
||||
NameGenerator.generate(OAUTH_2_PREFIX, 5),
|
||||
NameGenerator.generate("id-", 5),
|
||||
oAuthToken.getToken());
|
||||
Optional<Boolean> valid = isValid(token);
|
||||
|
|
@ -122,11 +125,24 @@ public class GitlabOAuthTokenFetcher implements PersonalAccessTokenFetcher {
|
|||
"not a valid url {} for current fetcher ", personalAccessToken.getScmProviderUrl());
|
||||
return Optional.empty();
|
||||
}
|
||||
try {
|
||||
GitlabOauthTokenInfo info = gitlabApiClient.getTokenInfo(personalAccessToken.getToken());
|
||||
return Optional.of(Sets.newHashSet(info.getScope()).containsAll(DEFAULT_TOKEN_SCOPES));
|
||||
} catch (ScmItemNotFoundException | ScmCommunicationException e) {
|
||||
return Optional.of(Boolean.FALSE);
|
||||
if (personalAccessToken.getScmTokenName() != null
|
||||
&& personalAccessToken.getScmTokenName().startsWith(OAUTH_2_PREFIX)) {
|
||||
// validation OAuth token by special API call
|
||||
try {
|
||||
GitlabOauthTokenInfo info = gitlabApiClient.getTokenInfo(personalAccessToken.getToken());
|
||||
return Optional.of(Sets.newHashSet(info.getScope()).containsAll(DEFAULT_TOKEN_SCOPES));
|
||||
} catch (ScmItemNotFoundException | ScmCommunicationException e) {
|
||||
return Optional.of(Boolean.FALSE);
|
||||
}
|
||||
} else {
|
||||
// validating personal access token from secret. Since PAT API is accessible only in
|
||||
// latest GitLab version, we just perform check by accessing something from API.
|
||||
try {
|
||||
gitlabApiClient.getUser(personalAccessToken.getToken());
|
||||
return Optional.of(Boolean.TRUE);
|
||||
} catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) {
|
||||
return Optional.of(Boolean.FALSE);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ import static org.eclipse.che.dto.server.DtoFactory.newDto;
|
|||
import static org.mockito.ArgumentMatchers.anyString;
|
||||
import static org.mockito.Mockito.when;
|
||||
import static org.testng.Assert.assertNotNull;
|
||||
import static org.testng.Assert.assertTrue;
|
||||
|
||||
import com.github.tomakehurst.wiremock.WireMockServer;
|
||||
import com.github.tomakehurst.wiremock.client.WireMock;
|
||||
|
|
@ -131,4 +132,27 @@ public class GitlabOAuthTokenFetcherTest {
|
|||
oAuthTokenFetcher.fetchPersonalAccessToken(subject, wireMockServer.url("/"));
|
||||
assertNotNull(token);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldValidatePersonalToken() throws Exception {
|
||||
stubFor(
|
||||
get(urlEqualTo("/api/v4/user"))
|
||||
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("Bearer token123"))
|
||||
.willReturn(
|
||||
aResponse()
|
||||
.withHeader("Content-Type", "application/json; charset=utf-8")
|
||||
.withBodyFile("gitlab/rest/api/v4/user/response.json")));
|
||||
|
||||
PersonalAccessToken token =
|
||||
new PersonalAccessToken(
|
||||
wireMockServer.baseUrl(),
|
||||
"cheUser",
|
||||
"username",
|
||||
"userId",
|
||||
"token-name",
|
||||
"tid-23434",
|
||||
"token123");
|
||||
|
||||
assertTrue(oAuthTokenFetcher.isValid(token).get());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue