Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rework the Bitbucket Server oauth token validation #673

Merged
merged 4 commits into from
Apr 8, 2024
Merged

Rework the Bitbucket Server oauth token validation #673

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1baf3af
Rework the Bitbucket Server oauth token validation
vinokurig Mar 27, 2024
37494cd
fixup! Rework the Bitbucket Server oauth token validation
vinokurig Mar 28, 2024
8a47c21
Merge branch 'main' of github.com:eclipse-che/che-server into che-22836
vinokurig Apr 3, 2024
02fb3c3
fixup! Rework the Bitbucket Server oauth token validation
vinokurig Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions ...h-bitbucket/src/main/java/org/eclipse/che/security/oauth/BitbucketOAuthAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2023 Red Hat, Inc.
* Copyright (c) 2012-2024 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand All @@ -12,6 +12,7 @@
package org.eclipse.che.security.oauth;

import static com.google.common.base.Strings.isNullOrEmpty;
import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.api.client.auth.oauth2.AuthorizationCodeRequestUrl;
import com.google.api.client.util.store.MemoryDataStoreFactory;
Expand Down Expand Up @@ -77,7 +78,7 @@ public OAuthToken getToken(String userId) throws IOException {
private String getTestRequestUrl() {
return "https://bitbucket.org".equals(bitbucketEndpoint)
? "https://api.bitbucket.org/2.0/user"
: bitbucketEndpoint + "/rest/api/1.0/application-properties";
: bitbucketEndpoint + "/plugins/servlet/applinks/whoami";
}

@Override
Expand All @@ -89,11 +90,12 @@ private void testRequest(String requestUrl, String accessToken)
throws OAuthAuthenticationException {
HttpURLConnection urlConnection = null;
InputStream urlInputStream = null;

String result;
try {
urlConnection = (HttpURLConnection) new URL(requestUrl).openConnection();
urlConnection.setRequestProperty("Authorization", "Bearer " + accessToken);
urlInputStream = urlConnection.getInputStream();
result = new String(urlInputStream.readAllBytes(), UTF_8);
} catch (IOException e) {
throw new OAuthAuthenticationException(e.getMessage(), e);
} finally {
Expand All @@ -108,5 +110,8 @@ private void testRequest(String requestUrl, String accessToken)
urlConnection.disconnect();
}
}
if (isNullOrEmpty(result)) {
tolusha marked this conversation as resolved.
Show resolved Hide resolved
throw new OAuthAuthenticationException("Empty response from Bitbucket Server API");
}
}
}
6 changes: 4 additions & 2 deletions ...g/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken)
}
try {
BitbucketPersonalAccessToken bitbucketPersonalAccessToken =
bitbucketServerApiClient.getPersonalAccessToken(personalAccessToken.getScmTokenId());
bitbucketServerApiClient.getPersonalAccessToken(
personalAccessToken.getScmTokenId(), personalAccessToken.getToken());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is hard to understand, why to retrieve personal access token we have to pass personal access token.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the PersonalAccessToken interface has a bit misleading name. In this case we implement an oauth token, but not a PAT.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Bitbucket Server oauth works in a bit different way: it uses the oauth token to create a PAT and then uses the PAT for all other operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tolusha Do you think we need to rename the PersonalAccessToken interface to AccessToken in order to avoid such confusion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a javadoc with a TODO.

return Optional.of(DEFAULT_TOKEN_SCOPE.equals(bitbucketPersonalAccessToken.getPermissions()));
} catch (ScmItemNotFoundException e) {
return Optional.of(Boolean.FALSE);
Expand Down Expand Up @@ -169,7 +170,8 @@ public Optional<Pair<Boolean, String>> isValid(PersonalAccessTokenParams params)
}
// Token is added by OAuth. Token id is available.
BitbucketPersonalAccessToken bitbucketPersonalAccessToken =
bitbucketServerApiClient.getPersonalAccessToken(params.getScmTokenId());
bitbucketServerApiClient.getPersonalAccessToken(
params.getScmTokenId(), params.getToken());
return Optional.of(
Pair.of(
DEFAULT_TOKEN_SCOPE.equals(bitbucketPersonalAccessToken.getPermissions())
Expand Down
25 changes: 16 additions & 9 deletions .../main/java/org/eclipse/che/api/factory/server/bitbucket/HttpBitbucketServerApiClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2023 Red Hat, Inc.
* Copyright (c) 2012-2024 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand Down Expand Up @@ -145,7 +145,9 @@ public List<BitbucketUser> getUsers(String filter)
@Override
public void deletePersonalAccessTokens(String tokenId)
throws ScmItemNotFoundException, ScmUnauthorizedException, ScmCommunicationException {
URI uri = serverUri.resolve("./rest/access-tokens/1.0/users/" + getUserSlug() + "/" + tokenId);
URI uri =
serverUri.resolve(
"./rest/access-tokens/1.0/users/" + getUserSlug(Optional.empty()) + "/" + tokenId);
HttpRequest request =
HttpRequest.newBuilder(uri)
.DELETE()
Expand Down Expand Up @@ -185,7 +187,7 @@ public BitbucketPersonalAccessToken createPersonalAccessTokens(
ScmItemNotFoundException {
BitbucketPersonalAccessToken token =
new BitbucketPersonalAccessToken(tokenName, permissions, 90);
URI uri = serverUri.resolve("./rest/access-tokens/1.0/users/" + getUserSlug());
URI uri = serverUri.resolve("./rest/access-tokens/1.0/users/" + getUserSlug(Optional.empty()));

try {
HttpRequest request =
Expand Down Expand Up @@ -229,22 +231,27 @@ public List<BitbucketPersonalAccessToken> getPersonalAccessTokens()
return doGetItems(
Optional.empty(),
BitbucketPersonalAccessToken.class,
"./rest/access-tokens/1.0/users/" + getUserSlug(),
"./rest/access-tokens/1.0/users/" + getUserSlug(Optional.empty()),
null);
} catch (ScmBadRequestException e) {
throw new ScmCommunicationException(e.getMessage(), e);
}
}

@Override
public BitbucketPersonalAccessToken getPersonalAccessToken(String tokenId)
public BitbucketPersonalAccessToken getPersonalAccessToken(String tokenId, String oauthToken)
throws ScmItemNotFoundException, ScmUnauthorizedException, ScmCommunicationException {
URI uri = serverUri.resolve("./rest/access-tokens/1.0/users/" + getUserSlug() + "/" + tokenId);
URI uri =
serverUri.resolve(
"./rest/access-tokens/1.0/users/"
+ getUserSlug(Optional.of(oauthToken))
+ "/"
+ tokenId);
HttpRequest request =
HttpRequest.newBuilder(uri)
.headers(
"Authorization",
computeAuthorizationHeader("GET", uri.toString()),
"Bearer " + oauthToken,
HttpHeaders.ACCEPT,
MediaType.APPLICATION_JSON)
.timeout(DEFAULT_HTTP_TIMEOUT)
Expand All @@ -269,9 +276,9 @@ public BitbucketPersonalAccessToken getPersonalAccessToken(String tokenId)
}
}

private String getUserSlug()
private String getUserSlug(Optional<String> token)
throws ScmItemNotFoundException, ScmCommunicationException, ScmUnauthorizedException {
return getUser(Optional.empty()).getSlug();
return getUser(token).getSlug();
}

private BitbucketUser getUser(Optional<String> token)
Expand Down
5 changes: 3 additions & 2 deletions ...in/java/org/eclipse/che/api/factory/server/bitbucket/server/BitbucketServerApiClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2023 Red Hat, Inc.
* Copyright (c) 2012-2024 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand Down Expand Up @@ -100,9 +100,10 @@ List<BitbucketPersonalAccessToken> getPersonalAccessTokens()

/**
* @param tokenId - bitbucket personal access token id.
* @param oauthToken - bitbucket oauth token.
* @return - Bitbucket personal access token.
* @throws ScmCommunicationException
*/
BitbucketPersonalAccessToken getPersonalAccessToken(String tokenId)
BitbucketPersonalAccessToken getPersonalAccessToken(String tokenId, String oauthToken)
throws ScmItemNotFoundException, ScmUnauthorizedException, ScmCommunicationException;
}
4 changes: 2 additions & 2 deletions ...ava/org/eclipse/che/api/factory/server/bitbucket/server/NoopBitbucketServerApiClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2023 Red Hat, Inc.
* Copyright (c) 2012-2024 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand Down Expand Up @@ -77,7 +77,7 @@ public List<BitbucketPersonalAccessToken> getPersonalAccessTokens()
}

@Override
public BitbucketPersonalAccessToken getPersonalAccessToken(String tokenId)
public BitbucketPersonalAccessToken getPersonalAccessToken(String tokenId, String oauthToken)
throws ScmItemNotFoundException, ScmUnauthorizedException, ScmCommunicationException {
throw new RuntimeException("Invalid usage of BitbucketServerApi");
}
Expand Down
6 changes: 4 additions & 2 deletions ...lipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2023 Red Hat, Inc.
* Copyright (c) 2012-2024 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand Down Expand Up @@ -221,10 +221,12 @@ public void shouldBeAbleToValidateToken()
throws ScmUnauthorizedException, ScmCommunicationException, ScmItemNotFoundException {
// given
when(personalAccessTokenParams.getScmProviderUrl()).thenReturn(someBitbucketURL);
when(personalAccessTokenParams.getToken()).thenReturn(bitbucketPersonalAccessToken.getToken());
when(personalAccessTokenParams.getScmTokenId())
.thenReturn(bitbucketPersonalAccessToken.getId());
when(bitbucketServerApiClient.isConnected(eq(someBitbucketURL))).thenReturn(true);
when(bitbucketServerApiClient.getPersonalAccessToken(eq(bitbucketPersonalAccessToken.getId())))
when(bitbucketServerApiClient.getPersonalAccessToken(
eq(bitbucketPersonalAccessToken.getId()), eq(bitbucketPersonalAccessToken.getToken())))
.thenReturn(bitbucketPersonalAccessToken);
// when
Optional<Pair<Boolean, String>> result = fetcher.isValid(personalAccessTokenParams);
Expand Down
20 changes: 10 additions & 10 deletions ...t/java/org/eclipse/che/api/factory/server/bitbucket/HttpBitbucketServerApiClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2023 Red Hat, Inc.
* Copyright (c) 2012-2024 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
Expand Down Expand Up @@ -312,14 +312,14 @@ public void shouldBeAbleToGetExistedPAT()
// given
stubFor(
get(urlPathEqualTo("/rest/access-tokens/1.0/users/ksmster/5"))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo(AUTHORIZATION_TOKEN))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("Bearer token"))
.withHeader(HttpHeaders.ACCEPT, equalTo(MediaType.APPLICATION_JSON))
.willReturn(
ok().withBodyFile("bitbucket/rest/access-tokens/1.0/users/ksmster/newtoken.json")));

stubFor(
get(urlPathEqualTo("/rest/api/1.0/users"))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo(AUTHORIZATION_TOKEN))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("Bearer token"))
.withQueryParam("start", equalTo("0"))
.withQueryParam("limit", equalTo("25"))
.willReturn(
Expand All @@ -328,7 +328,7 @@ public void shouldBeAbleToGetExistedPAT()
.withBodyFile("bitbucket/rest/api/1.0/users/filtered/response.json")));

// when
BitbucketPersonalAccessToken result = bitbucketServer.getPersonalAccessToken("5");
BitbucketPersonalAccessToken result = bitbucketServer.getPersonalAccessToken("5", "token");
// then
assertNotNull(result);
assertEquals(result.getToken(), "MTU4OTEwNTMyOTA5Ohc88HcY8k7gWOzl2mP5TtdtY5Qs");
Expand All @@ -346,7 +346,7 @@ public void shouldBeAbleToThrowNotFoundOnGePAT()
.willReturn(notFound()));

// when
bitbucketServer.getPersonalAccessToken("5");
bitbucketServer.getPersonalAccessToken("5", "token");
}

@Test(expectedExceptions = ScmUnauthorizedException.class)
Expand All @@ -361,18 +361,18 @@ public void shouldBeAbleToThrowScmUnauthorizedExceptionOnGetUser()
}

@Test(expectedExceptions = ScmUnauthorizedException.class)
public void shouldBeAbleToThrowScmUnauthorizedExceptionOnGePAT()
public void shouldBeAbleToThrowScmUnauthorizedExceptionOnGetPAT()
throws ScmCommunicationException, ScmUnauthorizedException, ScmItemNotFoundException {

// given
stubFor(
get(urlPathEqualTo("/rest/access-tokens/1.0/users/ksmster/5"))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo(AUTHORIZATION_TOKEN))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("Bearer token"))
.withHeader(HttpHeaders.ACCEPT, equalTo(MediaType.APPLICATION_JSON))
.willReturn(unauthorized()));
stubFor(
get(urlPathEqualTo("/rest/api/1.0/users"))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo(AUTHORIZATION_TOKEN))
.withHeader(HttpHeaders.AUTHORIZATION, equalTo("Bearer token"))
.withQueryParam("start", equalTo("0"))
.withQueryParam("limit", equalTo("25"))
.willReturn(
Expand All @@ -381,7 +381,7 @@ public void shouldBeAbleToThrowScmUnauthorizedExceptionOnGePAT()
.withBodyFile("bitbucket/rest/api/1.0/users/filtered/response.json")));

// when
bitbucketServer.getPersonalAccessToken("5");
bitbucketServer.getPersonalAccessToken("5", "token");
}

@Test(
Expand All @@ -400,7 +400,7 @@ public void shouldThrowScmCommunicationExceptionInNoOauthAuthenticator()
wireMockServer.url("/"), new NoopOAuthAuthenticator(), oAuthAPI, apiEndpoint);

// when
localServer.getPersonalAccessToken("5");
localServer.getUser();
}

@Test
Expand Down
Loading