-
Notifications
You must be signed in to change notification settings - Fork 70
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.
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
chore: Add an ability to manually configure bitbucket oAuth token #313
Changes from all commits
cc03412
17c82bb
e627d6a
e3e7378
9fdb554
afea2c7
bac2296
3944a01
19cfabf
b09e14a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
* Copyright (c) 2012-2021 Red Hat, Inc. | ||
* Copyright (c) 2012-2022 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/ | ||
|
@@ -25,6 +25,7 @@ | |
import org.eclipse.che.api.factory.server.bitbucket.server.BitbucketPersonalAccessToken; | ||
import org.eclipse.che.api.factory.server.bitbucket.server.BitbucketServerApiClient; | ||
import org.eclipse.che.api.factory.server.bitbucket.server.BitbucketUser; | ||
import org.eclipse.che.api.factory.server.bitbucket.server.HttpBitbucketServerApiClient; | ||
import org.eclipse.che.api.factory.server.scm.PersonalAccessToken; | ||
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenFetcher; | ||
import org.eclipse.che.api.factory.server.scm.exception.ScmBadRequestException; | ||
|
@@ -33,6 +34,7 @@ | |
import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException; | ||
import org.eclipse.che.commons.env.EnvironmentContext; | ||
import org.eclipse.che.commons.subject.Subject; | ||
import org.eclipse.che.security.oauth1.NoopOAuthAuthenticator; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
|
@@ -104,8 +106,20 @@ public PersonalAccessToken fetchPersonalAccessToken(Subject cheUser, String scmS | |
public Optional<Boolean> isValid(PersonalAccessToken personalAccessToken) | ||
throws ScmCommunicationException, ScmUnauthorizedException { | ||
if (!bitbucketServerApiClient.isConnected(personalAccessToken.getScmProviderUrl())) { | ||
LOG.debug("not a valid url {} for current fetcher ", personalAccessToken.getScmProviderUrl()); | ||
return Optional.empty(); | ||
// If BitBucket oAuth is not configured check the manually added user namespace token. | ||
HttpBitbucketServerApiClient apiClient = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a bit lost in the flow, why are we trying to check the token if BitBucket server is not configured? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main idea of the PR is to be able to use manually added user namespace secrets with tokeen for the oAuth flow in case when oAuth is not configured by the admin. |
||
new HttpBitbucketServerApiClient( | ||
personalAccessToken.getScmProviderUrl(), new NoopOAuthAuthenticator()); | ||
try { | ||
apiClient.getUser(personalAccessToken.getScmUserName(), personalAccessToken.getToken()); | ||
return Optional.of(Boolean.TRUE); | ||
} catch (ScmItemNotFoundException | ||
| ScmUnauthorizedException | ||
| ScmCommunicationException exception) { | ||
LOG.debug( | ||
"not a valid url {} for current fetcher ", personalAccessToken.getScmProviderUrl()); | ||
return Optional.empty(); | ||
} | ||
} | ||
try { | ||
BitbucketPersonalAccessToken bitbucketPersonalAccessToken = | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be const. In general, why do we need a dummy implementation in the case if no Bitbucket Server
integration is configured? Could you please add more details in the Javadocs for this class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This string is used only once, not sure we need a separate const for that.
It is needed to initialise an empty
BitbucketServerApiClient
which returnsfalse
onisConnected()
reqests. This is needed to check if bitbucket oAuth is configured or not. Updated the javadoc.