github sending alert emails that they will disallow access tokens in the URL query string #135
Comments
|
Just saw this same thing, an hour ago. |
|
Thanks for the report. I will try to look into it. GitLab seems to be affected as well, though: [1]. @lucamilanesio: Is gerrithub.io also affected? |
|
@davido thanks for the feedback, I'll check if we use it and make a fix. |
|
We ran into the same issue, and the fix we came up seemed easy enough (not sure if it's the correct one though, didn't go digging around too much). When calling out to service =
new ServiceBuilder()
.provider(new GitHub2Api(rootUrl))
.apiKey(cfg.getString(InitOAuth.CLIENT_ID))
.apiSecret(cfg.getString(InitOAuth.CLIENT_SECRET))
.callback(canonicalWebUrl + "oauth")
.scope(SCOPE)
.signatureType(SignatureType.Header)
.build();Deployed the modified jar to our instances, and everything seems to be working. |
Looking at https://github.com/dsyer/scribe-java/blob/master/src/main/java/org/scribe/builder/ServiceBuilder.java the default signature type is already "SignatureType.Header" and this doesn't seem to have any effect if it were something else either. The email says it's only sent every three days have you watched the actual HTTP requests go out ? |
|
I think that in scribejava/scribejava@d82112b the ability to actually sign request in header was added. And in scribejava/scribejava@73c2ef4#diff-163a0f46f10a315138f60858fc8981f8R118 it was switched per default in DefaultApi20.java. So I think one non invasive approach to fix it would be to add in currently used scribe 1.3.7 version the ability to sign request in header first and then switch GitHub OAuth service implementation to using it. |
|
yeah I noticed there's two really different versions of this "scribe" thing, not sure of the history there. |
|
Yeah. Another approach would be to just use the new release 6.9.0. It could even be used in the same time, given that the package name was changed between major version bump, I think 2.0 or something. |
|
I think there is one complication. If I read the spec correctly, they are saying also, that client_id/client_secret also should be sent using basic auth, quoting: While the request to retrieve the access token is executed in the oauth plugin itself, the application authorization request is issued from gerrit core, using this interface: in OAuthSession class, here: If I am not missing something very obvious, then I think we cannot fix it without extending Gerrit core. We would need to retrieve client api and client secret and be able to put them as basic auth in request header. This would have to happen, at least partly in gerrit core. Of course this would have to be done conditionally, depending on specific service provider. What a mess?! |
|
My vague recollection of Oauth coupled with the fact that those are redirect URLs suggests those would not have client_id / client_secret in them. the client_id/secret is only in the server-to-server POST call. on my current gerrit, looking at browser calls the redirect has client id in it:
which is described in step 1 at https://developer.github.com/apps/building-oauth-apps/authorizing-oauth-apps/#web-application-flow, which is the flow that the deprecation notice is saying that they're supporting. they're being a little lose with the terms "authentication" and "authorization" however if I recall the terminology correctly, gerrit is only doing the first part of "authentication". its that "api.github.com/user" request that requires "authorization". github has another form of app authentication which is the client_id/client_secret one I think they are referring towards, that's not web flow and has no access token, that's this one: https://developer.github.com/v3/guides/basics-of-authentication/#accepting-user-authorization |
I knew it couldn't be this simple, but one can always hope. And I should have dug a bit deeper, apologies for butting in! |
|
I have a working prototype and will upload a new CL in a moment and provide plugin artifact to test. |
davido
added
enhancement
change under review
|
New scribe patch release is here: [1] and the CL that switched GitHub OAuth provider to use client basic auth scheme and bearer token request signing is here: [2]. Plugin artifact is here: [3]. SHA1 is: Can someone test it? [1] https://github.com/davido/scribejava/releases/tag/v1.3.8 |
|
will this plugin work with Gerrit 2.15.4 ? that's what I'm running right now |
|
It would not, unfortunately, because of this change: [1]. We would have to move that change on stable-2.15 branch then. Note, though, that support for 2.15 was discontinued and you would have to upgrade to at least 2.16. [1] https://gerrit-review.googlesource.com/c/gerrit/+/208043 |
|
I moved that change to stable-2.15 branch: [1]. To test the plugin on stable-2.15 branch: [2]. [1] https://gerrit-review.googlesource.com/c/plugins/oauth/+/253926 |
In this major overhaul of this plugin dependency: [1], I bumped the scribe version from 1.3.7 to 6.9.0, so that the both features: client basic authentication scheme and bearer access token authorization should be fixed now. [1] https://gerrit-review.googlesource.com/c/plugins/oauth/+/253929 |
|
OK I can try out that .jar file now, what will the actual release be, will there be a 2.14.6.3 ? I currently use 2.14.6.2. OTOH If I have to upgrade my gerrit just let me know how far I have to go up. |
|
welp I have to upgrade to gerrit 2.16 anyway beacuse none of the plugins are available at https://gerrit-ci.gerritforge.com/view/Plugins-stable-2.15/ anymore and I blew away my docker image |
|
OK I used the .jar file at https://ostrovsky.org/gerrit/oauth-g0e144214cc.jar against gerrit-2.16.16, replacing it for the gerrit-oauth-provider.jar file, got the stack trace below. is that .jar usable for 2.16 or do I need a new build for that. |
This JAR was built against master and as I wrote is not compatible with 2.16 and older gerrit releases. For that I've uploaded plugin version built against 2.15: [1], or even bumped the scribe version to 6.9.0, see the CL link in my previous comment. |
|
OK, it wasn't clear to me if a jar that works with 2.15 would also work with 2.16 since I thought that was where the API changes were. you're still waiting on a positive test before releasing right? |
|
OK so I am running that and it "works", if you know offhand how I can get HTTPS request logging to come out when I run gerrit, I can confirm the headers are correct,otherwise will just see if I don't get an email in the next three days. |
|
Thanks for the feedback. Can you also test the major upgrade of scribe library (based on stable-2.16 branch)? The CL is here: [1] and the final artifact is here: [2]. [1] https://gerrit-review.googlesource.com/c/plugins/oauth/+/253929 |
|
I'm running the plugin built from [1] (hash e4457af1) with gerrit 3.1.0 and GitHub OAuth. So far it has been working nicely and I haven't noticed any issues. [1] https://gerrit-review.googlesource.com/c/plugins/oauth/+/254332 |
|
@borospeti Thanks for the feedback. Note, though, that for 3.1.x release you shoud rather use the master branch, so for 3.1.x gerrit release the right change to build from is: [1]. [1] https://gerrit-review.googlesource.com/c/plugins/oauth/+/254372 |
|
Indeed. I've just copied the wrong link/hash. :)
|
|
I know this issue is now closed, and I'm probably missing something really obvious, but what to I have to do to my current 2.16.11.1 deployment to stop Google sending out these messages? Do I just need to download https://ostrovsky.org/gerrit/oauth-g5e0febd09d.jar and replace my existing If so, I assume it would be the same if I update to 2.16.16, but what if I upgrade to 3.0.7 or 3.1.3? Would I then have to build the jar from source? Once you are sure these fixes are working, @davido, will you be making v2.14.2 and v3.0.1 releases on github? Or should I now be downloading from gerritforge instead? |
|
You can not use 2.16 with 3.x and vise versa. If you use gerrit 2.16 you need the plugin that is built against that gerrit version. Yes, just fetch the JAR from the link in my previous comment. If you upgrade to 3.x then you would have to use a different JAR, that I also provided in my previous comment.
When the pending changes for review will be merged, then the corresponding build artifacts will show up in GerritForge-CI. Note, though, that the changes are not merged yet (probably in next days). So, in case someone would like to build them from source, the changes would have to be fetched in the corresponding gerrit release branch: stable-2.16: https://gerrit-review.googlesource.com/c/plugins/oauth/+/253929 |
|
Thanks for the clarifications @davido. If its a matter of a few days I'll wait for a build (I've starred the change in gerrit, and it saves making my upgrade documentation more complex than it needs to be). I guess I should also apologise the the people getting the nag e-mails. *8') |
|
sorry to keep adding to the bother here but the jar file at https://gerrit-ci.gerritforge.com/job/plugin-oauth-bazel-stable-2.16/ is not updated yet, right? I get all my other plugins from this site so if oauth is there now that would be more convenient than getting this one from the local github download. |
Right. The changes linked in my previous comment are not merged yet. |
|
All changes were merged. New plugin release based on Gerrit 3.1.3 plugin API is here: [1]. For other releases, please consult GeritForge-CI, e.g. for 2.16 branch: [2]. [1] https://github.com/davido/gerrit-oauth-provider/releases/tag/v3.1.3 |
Hi there -
Very few hits on google for this and I think this is all new. Github is sending these emails:
The gerrit oauth provider accesses the https://api.github.com/user URL right here:
gerrit-oauth-provider/src/main/java/com/googlesource/gerrit/plugins/oauth/GitHubOAuthService.java
Line 79 in 0f807f6
and I believe the access_token is hardwired to be in the query string assuming the scribe library in use is here: https://github.com/dsyer/scribe-java/blob/master/src/main/java/org/scribe/oauth/OAuth20ServiceImpl.java#L59
I apologize if I'm getting this all wrong, I have a lot of github / gerrit integration going on but I wrote all of it in Python and it's using access_token in the headers; the oauth plugin is the only tool I have installed in my gerrit server that would report Java as the user agent,the api.github.com/user URL hardwired in the oauth plugin here and the behavior of "scribe" would seem to be the source of the issue. It just seems surprising to me that nobody has reported this yet!
The text was updated successfully, but these errors were encountered: