Explicitly set Bitbucket Server provider name, fix GitHub url parser #689
Conversation
Signed-off-by: ivinokur <ivinokur@redhat.com>
| @@ -52,7 +54,7 @@ public String getAuthenticateUrl(URL requestUrl, List<String> scopes) { | |||
|
|
|||
| @Override | |||
| public final String getOAuthProvider() { | |||
| return "bitbucket"; | |||
| return BITBUCKET_CLOUD_ENDPOINT.equals(bitbucketEndpoint) ? "bitbucket" : "bitbucket-server"; | |||
There was a problem hiding this comment.
should we extract consts as well for bitbucket / bitbucket-server ? would be nice to add comments java docs as well
| // Try to call an API request to see if the URL matches GitHub. | ||
| || isApiRequestRelevant(trimmedUrl); | ||
| // Try to call an API request to see if the URL matches self-hosted GitHub Enterprise. | ||
| || (!GITHUB_SAAS_ENDPOINT.equals(endpoint) && isApiRequestRelevant(trimmedUrl)); |
There was a problem hiding this comment.
Can we rename isApiRequestRelevant to isGitHubServerURLValid ?
Add some comments please:
// We need to check if the given URL is a valid GitHub URL.
// 1. Check if the given URL matches the GitHub URL patterns. It works if OAuth is configured and
// GitHub server URL is known or the repository URL points to GitHub SaaS (https://github.com).
// 2. Check whether PAT is configured for the GitHub server URL. It is sufficient to confirm that the URL
// is a valid GitHub URL.
// 3. Check if the given URL is a valid GitHub URL by reaching the endpoint of the GitHub server and
// Analysing the response. This query basically only needs to be performed if the specified repository URL
// does not point to GitHub SaaS.
There was a problem hiding this comment.
Can we rename isApiRequestRelevant to isGitHubServerURLValid ?
I think that isApiRequestRelevant is a more specific name. We already validate the GitHub Server url on each step:
- The GitHub Server url might be configured so the regex will detect it.
- A user token secret from the GitHub Server might be present.
- If all the checks above returned
false, we finally send a test request.
There was a problem hiding this comment.
Added the javadocs to the corresponding steps.
|
Verified on Eclipse Che with Responses on the |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: artaleks9, ibuziuk, tolusha, vinokurig The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What does this PR do?
bitbucket.Screenshot/screencast of this PR
What issues does this PR fix or reference?
eclipse-che/che#22971
How to test this PR?
quay.io/eclipse/che-server:pr-689<che-endpoint/api/oauth> api request, see: the provider name isbitbucket-server`<che-endpoint/api/oauth> api request, see: the provider name isbitbucket`PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or referenceandHow to test this PRcompletedReviewers
Reviewers, please comment how you tested the PR when approving it.