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.

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

bitbucket oauth broken #127

Open
mrdfuse opened this issue May 29, 2019 · 17 comments
Open

bitbucket oauth broken #127

mrdfuse opened this issue May 29, 2019 · 17 comments

Comments

@mrdfuse
Copy link

mrdfuse commented May 29, 2019

With plugin 2.14.6.2, it seems bitbucket oauth is broken since today?

[2019-05-29 07:58:41,400] [HTTP-693] ERROR com.google.gerrit.pgm.http.jetty.HiddenErrorHandler : Error in GET /oauth?state=xxxxxxxxxxxxxxxxxxxxxxxxx&code=xxxxxxxxxxxxxxxx
java.lang.NullPointerException
        at com.googlesource.gerrit.plugins.oauth.BitbucketOAuthService.getUserInfo(BitbucketOAuthService.java:95)
        at com.google.gerrit.httpd.auth.oauth.OAuthSession.login(OAuthSession.java:105)
        at com.google.gerrit.httpd.auth.oauth.OAuthWebFilter.doFilter(OAuthWebFilter.java:105)

@davido
Copy link
Owner

davido commented May 29, 2019

Strange. What is different now?

@mrdfuse
Copy link
Author

mrdfuse commented May 29, 2019

I have no idea, I thought I logged this issue quicky while scrambling away to get our developers back on track. We switched to DEVELOPMENT_BECOME_ANY_ACCOUNT now, just to be able to work again. I have time now to further debug what might be wrong. What can I do? Can I increase the logging level to see more? We are using the openfrontier docker image, which does make it a bit harder to quickly change config here and there.

@davido
Copy link
Owner

davido commented May 29, 2019

May be Bitbucket changed something on their side, can you ask them? Yes, it should be possible to increase logging level in gerrit.

@mrdfuse
Copy link
Author

mrdfuse commented May 29, 2019

Yes that is also my guess. We are trying to debug it, will keep you posted.

@azhou2019
Copy link

We have the same issue with BitBucket OAuth starting May 29th. It is odd as only some BitBucket users would experience this server error and other users are fine.

@davido
Copy link
Owner

davido commented May 30, 2019

Looking at the code:

    JsonElement usernameElement = userObject.get("username");
    String username = usernameElement.getAsString(); // <== NPE is here, so that username JSON element in the result seems to be null.

we have a problem to extract username from the https://bitbucket.org/api/1.0/user endpoint. I think that someone would have to open issue on BitBucket side.

@davido
Copy link
Owner

davido commented May 30, 2019

Nothing unusual on the API page: [1]. However, we are still using api version 1.0, while there is api version 2. available. Can someone try to patch the plugin with this diff ans see if this helps:

Date:   Thu May 30 10:15:08 2019 +0200

    Bump BitBucket api version to 2.0
    
    Change-Id: Ie7e42cf9e4e4e0eb3300d2f0f034d7ca04b8ec0f

diff --git a/src/main/java/com/googlesource/gerrit/plugins/oauth/BitbucketOAuthService.java b/src/main/java/com/googlesource/gerrit/plugins/oauth/BitbucketOAuthService.java
index e600067..ada287c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/oauth/BitbucketOAuthService.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/oauth/BitbucketOAuthService.java
@@ -47,7 +47,7 @@ public class BitbucketOAuthService implements OAuthServiceProvider {
   private static final Logger log = getLogger(BitbucketOAuthService.class);
   static final String CONFIG_SUFFIX = "-bitbucket-oauth";
   private static final String BITBUCKET_PROVIDER_PREFIX = "bitbucket-oauth:";
-  private static final String PROTECTED_RESOURCE_URL = "https://bitbucket.org/api/1.0/user/";
+  private static final String PROTECTED_RESOURCE_URL = "https://bitbucket.org/api/2.0/user/";
   private final boolean fixLegacyUserId;
   private final OAuthService service;

@azhou2019
Copy link

@davido @mrdfuse I have received the following response from Bitbucket Support Team:

Looks like the plugin :
https://github.com/davido/gerrit-oauth-provider/blob/master/src/main/java/com/googlesource/gerrit/plugins/oauth/BitbucketOAuthService.java#L95
hasn't been updated per our deprecation/removal documentation that has been announced over 6 months ago.
For reference:
https://developer.atlassian.com/cloud/bitbucket/bitbucket-api-changes-gdpr/
The owner of the plugin will need to update this string:
String username = usernameElement.getAsString();
to use the UUID instead of a username for the plugin to work with the changes that we have introduced to improve account privacy.

@mrdfuse
Copy link
Author

mrdfuse commented May 30, 2019

Thanks for the feedback. If no else has done it by next week, I'll try to patch it. Would need to setup a dev environment first though.

@davido
Copy link
Owner

davido commented May 31, 2019

@azhou2019 Thanks so much, for letting us know.

I am working on a fix and will try to release it this weekend (I'm doing all this here in my spare free time).

@davido
Copy link
Owner

davido commented May 31, 2019

OK, I'm looking deeper now into the spec and deprecation announcement, particularly:

Replace username fields with Atlassian Account ID (account_id)

The Bitbucket's user object schema will lose its username field after 29 April 2019. This will affect clients that use username to identify accounts in API request objects.

and this example, old:

$ curl https://api.bitbucket.org/2.0/repositories/foo/bar/forks \ -H 'Content-Type: application/json' \ -d '{
    "name": "my_fork",
    "owner": {
      "username": "evzijst"
    }
}'

new:

$ curl https://api.bitbucket.org/2.0/repositories/foo/bar/forks \ -H 'Content-Type: application/json' \ -d '{
    "name": "my_fork",
    "owner": {
      "account_id": "557058:c0b72ad0-1cb5-4018-9cdc-0cde8492c443"
    }
}'

or alternatively, use uuid, example: uuid | {33e297d1-567f-4c32-89b4-4b2759c5f5ae}.

But this switch would mean, that we would create a virgin new account (!) with new account id. This would mean that site admin would have to merge those accounts. This would mean siome SQL statements or All-Users repository surgery, depending on whether ReviewDb or NoteDb is used.

We would also not be able to automagically link the deprecated usernae to the new uuid value, because username is not returned any more.

To be honest this is pretty much disruptive change! Bitbucket Site can argue, that username was deprecated and we should have enough time to react on this announcement and link meantime uuid to deprecated username in Gerrit Code Review backend.

However, I am maintaining a number of different OAuth 2 providers in this plugin in my spare free time, and am not going to follow any API announcements of those providers.

I would expect Bitbucket Site to re-consider this disruptive removal of username attribute and extend the support for this attribute (may be also protected by addition request parameter, like enforce_deprecated_username_attribute) for another one or two years.

That way, we could automagically link new uuid to the same username Bitbucket account, without site admn intervention (accounts merge procedure).

@mrdfuse
Copy link
Author

mrdfuse commented Jun 1, 2019

I doubt Bitbucket is going to revert their change just for this project :(

@davido
Copy link
Owner

davido commented Jun 9, 2019

I am re-reading the spec change announcement in: [1] again and see this:

The exception to the removal of usernames is the /2.0/user endpoint,
which will continue to contain the username field. This endpoint only
ever returns the authenticated user's own user object, not that of other users.

So that my diff in this comment should probably fix the problem. Can someone try? I do not have access to Atlassian test site.

[1] https://developer.atlassian.com/cloud/bitbucket/bitbucket-api-changes-gdpr

@davido
Copy link
Owner

davido commented Jun 9, 2019

I have just sent this CL: [1]. Can someone test it?

If you cannot build it yourself, I can build it for you.
Just tell me, which Gerrit version you are using. The
above CL was uploaded to stable-2.14 branch.

@mrdfuse
Copy link
Author

mrdfuse commented Jun 9, 2019

I can test it on 2.14, but I can't build it myself.

@mionch
Copy link

mionch commented Jun 11, 2019

It looks like the v1 API has some further issues - over the last couple hours #I would get an error on any call, pointing out that this API is no longer available and linking to https://developer.atlassian.com/cloud/bitbucket/deprecation-notice-v1-apis/ for an explanation. I have tried changing the API url to 2.0 as @davido suggested, but it looks like the structure of the response has also slightly changed.

I have created a pull request with the changes that I have managed to get to work in our case: #128
Hope this helps!

@davido
Copy link
Owner

davido commented Jun 12, 2019

Thanks, @mionch! I started to work on the issue already and uploaded this CL to canonical repository for this plugin: [1]. However, I am not able to test this change and thus I have not merged it yet.

Can you please upload your change to https://gerrit-review.googlesource.com, to stable-2.14 branch? I would merge that change then to this plugin as well.

[1] https://gerrit-review.googlesource.com/c/plugins/oauth/+/227253

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants