chore: Add an ability to manually configure bitbucket oAuth token #313

vinokurig · 2022-06-10T15:11:52Z

Signed-off-by: Igor Vinokur ivinokur@redhat.com

What does this PR do?

Read user namespace oauth token before checking the oAuth configuration in case when user manually added a bitbucket oAuth secret to the user namespace.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

eclipse-che/che#21189

How to test this PR?

Deploy che with quay.io/ivinokur/che-server:next image.
Deploy bitbucket server.
Add a secret to the user namespace:

kind: Secret
apiVersion: v1
metadata:
  name: personal-access-token-<github/gitlab/bitbucket-server>
  namespace: <namespace name>
  labels:
    app.kubernetes.io/component: scm-personal-access-token
    app.kubernetes.io/part-of: che.eclipse.org
  annotations:
    che.eclipse.org/che-userid: <che user id>
    che.eclipse.org/scm-personal-access-token-name: <github/gitlab/bitbucket-server>
    che.eclipse.org/scm-userid: <github/gitlab/gitbucket-server user id>
    che.eclipse.org/scm-url: >-
      <github/gitlab/gitbucket-server endpoint Url>
    che.eclipse.org/scm-username: <github/gitlab/bitbucket-server username>
data:
  token: <base 64 encoded token>
type: Opaque

Try to create a factory from the bitbucket server repo.

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: Igor Vinokur <ivinokur@redhat.com>

vinokurig · 2022-06-20T10:26:49Z

@tolusha @ibuziuk please take a look

...factory/src/main/java/org/eclipse/che/api/factory/server/scm/PersonalAccessTokenManager.java

tolusha · 2022-06-21T11:17:49Z

...api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/NoopOAuthAuthenticator.java

@@ -49,7 +49,6 @@ public String computeAuthorizationHeader(String userId, String requestMethod, St

  @Override
  public String getLocalAuthenticateUrl() {
-    throw new RuntimeException(
-        "The fallback noop authenticator cannot be used for authentication. Make sure OAuth is properly configured.");
+    return "Empty URL";


Why not to keep throwing RuntimeException ?

To avoid the exception when the NoopOAuthAuthenticator is used to create an API client with an empty authenticator:

che-server/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java

Line 114 in 17c82bb

personalAccessToken.getScmProviderUrl(), new NoopOAuthAuthenticator());

.../org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java

…Auth token

.../org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java

.../org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java

...-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabOAuthTokenFetcher.java

tolusha · 2022-06-24T07:29:32Z

...-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParser.java

@@ -57,9 +67,49 @@ public GitlabUrlParser(
    }
  }

+  private boolean userTokenExists(String url) {


Duplicates?

Unfortunately I can't easyaly combine this functions into one common, as they are located in separate maven modules.

...-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParser.java

...et-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketURLParser.java

…rg/eclipse/che/api/factory/server/bitbucket/BitbucketURLParser.java Co-authored-by: Anatolii Bazko <abazko@redhat.com>

...api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/NoopOAuthAuthenticator.java

ibuziuk · 2022-06-27T08:51:08Z

...api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/NoopOAuthAuthenticator.java

@@ -49,7 +49,6 @@ public String computeAuthorizationHeader(String userId, String requestMethod, St

  @Override
  public String getLocalAuthenticateUrl() {
-    throw new RuntimeException(
-        "The fallback noop authenticator cannot be used for authentication. Make sure OAuth is properly configured.");
+    return "Noop URL";


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?

should be const.

This string is used only once, not sure we need a separate const for that.

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?

It is needed to initialise an empty BitbucketServerApiClient which returns false on isConnected() reqests. This is needed to check if bitbucket oAuth is configured or not. Updated the javadoc.

ibuziuk · 2022-06-27T08:54:50Z

.../org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java

-      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 =


I'm a bit lost in the flow, why are we trying to check the token if BitBucket server is not configured?

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.

ibuziuk · 2022-06-27T08:59:06Z

...-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParser.java

@@ -57,9 +68,55 @@ public GitlabUrlParser(
    }
  }

+  private boolean isUserTokenExists(String repositoryUrl) {


grammatically it should be isUserTokenExist
also, since there is isPresent call, consider changing the name to isUserTokenPresent

chore: Add an ability to manually configure bitbucket oAuth token

cc03412

Signed-off-by: Igor Vinokur <ivinokur@redhat.com>

vinokurig requested review from skabashnyuk and nickboldt as code owners June 10, 2022 15:11

fixup! chore: Add an ability to manually configure bitbucket oAuth token

17c82bb

vinokurig requested review from mshaposhnik, amisevsk and metlos as code owners June 17, 2022 13:48

vinokurig requested review from ibuziuk and tolusha June 20, 2022 10:26

tolusha reviewed Jun 21, 2022

View reviewed changes

vinokurig mentioned this pull request Jun 21, 2022

chore: Add instructions how to add a manuall PAT secret eclipse-che/che-docs#2383

Merged

6 tasks

vinokurig added 3 commits June 22, 2022 09:06

fixup! fixup! chore: Add an ability to manually configure bitbucket o…

e627d6a

…Auth token

fixup! fixup! chore: Add an ability to manually configure bitbucket o…

e3e7378

…Auth token

fixup! chore: Add an ability to manually configure bitbucket oAuth token

9fdb554

tolusha reviewed Jun 24, 2022

View reviewed changes

.../org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java Outdated Show resolved Hide resolved

.../org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcher.java Outdated Show resolved Hide resolved

tolusha reviewed Jun 24, 2022

View reviewed changes

vinokurig and others added 3 commits June 24, 2022 14:54

Update wsmaster/che-core-api-factory-bitbucket-server/src/main/java/o…

afea2c7

…rg/eclipse/che/api/factory/server/bitbucket/BitbucketURLParser.java Co-authored-by: Anatolii Bazko <abazko@redhat.com>

Update wsmaster/che-core-api-factory-bitbucket-server/src/main/java/o…

bac2296

…rg/eclipse/che/api/factory/server/bitbucket/BitbucketURLParser.java Co-authored-by: Anatolii Bazko <abazko@redhat.com>

fixup! chore: Add an ability to manually configure bitbucket oAuth token

3944a01

tolusha approved these changes Jun 27, 2022

View reviewed changes

...api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/NoopOAuthAuthenticator.java Outdated Show resolved Hide resolved

fixup! chore: Add an ability to manually configure bitbucket oAuth token

19cfabf

ibuziuk reviewed Jun 27, 2022

View reviewed changes

vinokurig force-pushed the che-21189 branch from 87f297d to 28b2220 Compare June 27, 2022 12:03

fixup! chore: Add an ability to manually configure bitbucket oAuth token

b09e14a

vinokurig force-pushed the che-21189 branch from 28b2220 to b09e14a Compare June 27, 2022 12:08

ibuziuk approved these changes Jun 28, 2022

View reviewed changes

vinokurig merged commit a47f115 into main Jun 29, 2022

vinokurig deleted the che-21189 branch June 29, 2022 08:17

che-bot added this to the 7.50 milestone Jun 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Add an ability to manually configure bitbucket oAuth token #313

chore: Add an ability to manually configure bitbucket oAuth token #313

vinokurig commented Jun 10, 2022 •

edited

Loading

vinokurig commented Jun 20, 2022

tolusha Jun 21, 2022

vinokurig Jun 22, 2022

tolusha Jun 24, 2022

vinokurig Jun 24, 2022

ibuziuk Jun 27, 2022

vinokurig Jun 27, 2022

ibuziuk Jun 27, 2022

vinokurig Jun 27, 2022

ibuziuk Jun 27, 2022

vinokurig Jun 27, 2022

chore: Add an ability to manually configure bitbucket oAuth token #313

chore: Add an ability to manually configure bitbucket oAuth token #313

Conversation

vinokurig commented Jun 10, 2022 • edited Loading

What does this PR do?

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Reviewers

vinokurig commented Jun 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinokurig commented Jun 10, 2022 •

edited

Loading