github sending alert emails that they will disallow access tokens in the URL query string
zzzeek opened this issue · 33 comments
Hi there -
Very few hits on google for this and I think this is all new. Github is sending these emails:
[GitHub API] Deprecation notice for authentication via URL query parameter
Hi @sqla-tester,
On February 3rd, 2020 at 23:36 (UTC) your application (sqlalchemy-gerrit) used an access token (with the User-Agent Java/1.8.0_212) as part of a query parameter to access an endpoint through the GitHub API:
Please use the Authorization HTTP header instead as using the
access_tokenquery parameter is deprecated.Depending on your API usage, we'll be sending you this email reminder once every 3 days for each token and User-Agent used in API calls made on your behalf.
Just one URL that was accessed with a token and User-Agent combination will be listed in the email reminder, not all.Visit https://developer.github.com/changes/2019-11-05-deprecated-passwords-and-authorizations-api/#authenticating-using-query-parameters for more information.
The gerrit oauth provider accesses the https://api.github.com/user URL right here:
public OAuthUserInfo getUserInfo(OAuthToken token) throws IOException {
OAuthRequest request = new OAuthRequest(Verb.GET, PROTECTED_RESOURCE_URL);
Token t = new Token(token.getToken(), token.getSecret(), token.getRaw());
service.signRequest(t, request);
Response response = request.send();
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
public void signRequest(Token accessToken, OAuthRequest request)
{
request.addQuerystringParameter(OAuthConstants.ACCESS_TOKEN, accessToken.getToken());
}
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!
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 ServiceBuilder() in GitHubOAuthService.java, just added signatureType, like so:
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.
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:
GitHub is deprecating authentication to the GitHub API using query parameters,
such as using a access_token query parameter for OAuth user authentication
or a client_id/client_secret query parameter for OAuth application authentication.
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:
https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java#123
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:
https://github.com/login/oauth/authorize?client_id=<FOO BAR>&redirect_uri=https%3A%2F%2Fgerrit.sqlalchemy.org%2Foauth&state=<FOO BAR>
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
the default signature type is already "SignatureType.Header" and this doesn't seem to have any effect if it were something else either.
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.
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:
$ sha1sum oauth-g0e144214cc.jar
4419ef34936e0c9c75c869ce8bcdd1c9040217db oauth-g0e144214cc.jar
Can someone test it?
[1] https://github.com/davido/scribejava/releases/tag/v1.3.8
[2] https://gerrit-review.googlesource.com/c/plugins/oauth/+/253926
[3] https://ostrovsky.org/gerrit/oauth-g0e144214cc.jar
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
[2] https://ostrovsky.org/gerrit/oauth-g470058ed4a.jar
I think that in scribejava/scribejava@d82112b the ability to actually sign request in header was added.
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.
[2020-02-10 03:30:53,615] [HTTP-93] WARN org.eclipse.jetty.server.HttpChannel : /oauth
java.lang.NoClassDefFoundError: com/google/gerrit/json/OutputFormat
at com.googlesource.gerrit.plugins.oauth.GitHubOAuthService.getUserInfo(GitHubOAuthService.java:107)
at com.google.gerrit.httpd.auth.oauth.OAuthSession.login(OAuthSession.java:106)
at com.google.gerrit.httpd.auth.oauth.OAuthWebFilter.doFilter(OAuthWebFilter.java:105)
at com.google.gerrit.httpd.RequireSslFilter.doFilter(RequireSslFilter.java:72)
at com.google.gerrit.httpd.RunAsFilter.doFilter(RunAsFilter.java:121)
at com.google.gerrit.httpd.GwtCacheControlFilter.doFilter(GwtCacheControlFilter.java:72)
at com.google.gerrit.httpd.SetThreadNameFilter.doFilter(SetThreadNameFilter.java:62)
at com.google.gerrit.httpd.AllRequestFilter$FilterProxy$1.doFilter(AllRequestFilter.java:133)
at com.google.gerrit.httpd.AllRequestFilter$FilterProxy.doFilter(AllRequestFilter.java:135)
at com.google.gerrit.httpd.RequestCleanupFilter.doFilter(RequestCleanupFilter.java:60)
at com.google.gerrit.httpd.RequestMetricsFilter.doFilter(RequestMetricsFilter.java:57)
at com.google.gerrit.httpd.RequestContextFilter.doFilter(RequestContextFilter.java:64)
at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:121)
at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:133)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1610)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:540)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:255)
at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1588)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:255)
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1345)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:203)
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:480)
at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1557)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:201)
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1247)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:144)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:132)
at org.eclipse.jetty.server.Server.handle(Server.java:502)
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:364)
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:260)
at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:305)
at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:118)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:333)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:310)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:168)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:126)
at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:366)
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:765)
at org.eclipse.jetty.util.thread.QueuedThreadPool$2.run(QueuedThreadPool.java:683)
at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.ClassNotFoundException: com.google.gerrit.json.OutputFormat
at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
at java.lang.ClassLoader.loadClass(ClassLoader.java:419)
at java.net.FactoryURLClassLoader.loadClass(URLClassLoader.java:817)
at java.lang.ClassLoader.loadClass(ClassLoader.java:352)
... 41 more
[2020-02-10 03:30:53,621] [HTTP-93] ERROR com.google.gerrit.pgm.http.jetty.HiddenErrorHandler : Error in GET /oauth?code=a113984f67679c59965f&state=K4bYgyJwlcVbBGgKdl3vHOFoc1CWas0fY0EdsEXPAF0
java.lang.NoClassDefFoundError: com/google/gerrit/json/OutputFormat
at com.googlesource.gerrit.plugins.oauth.GitHubOAuthService.getUserInfo(GitHubOAuthService.java:107)
at com.google.gerrit.httpd.auth.oauth.OAuthSession.login(OAuthSession.java:106)
at com.google.gerrit.httpd.auth.oauth.OAuthWebFilter.doFilter(OAuthWebFilter.java:105)
at com.google.gerrit.httpd.RequireSslFilter.doFilter(RequireSslFilter.java:72)
at com.google.gerrit.httpd.RunAsFilter.doFilter(RunAsFilter.java:121)
at com.google.gerrit.httpd.GwtCacheControlFilter.doFilter(GwtCacheControlFilter.java:72)
at com.google.gerrit.httpd.SetThreadNameFilter.doFilter(SetThreadNameFilter.java:62)
at com.google.gerrit.httpd.AllRequestFilter$FilterProxy$1.doFilter(AllRequestFilter.java:133)
at com.google.gerrit.httpd.AllRequestFilter$FilterProxy.doFilter(AllRequestFilter.java:135)
at com.google.gerrit.httpd.RequestCleanupFilter.doFilter(RequestCleanupFilter.java:60)
at com.google.gerrit.httpd.RequestMetricsFilter.doFilter(RequestMetricsFilter.java:57)
at com.google.gerrit.httpd.RequestContextFilter.doFilter(RequestContextFilter.java:64)
at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:121)
at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:133)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1610)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:540)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:255)
at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1588)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:255)
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1345)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:203)
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:480)
at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1557)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:201)
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1247)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:144)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:132)
at org.eclipse.jetty.server.Server.handle(Server.java:502)
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:364)
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:260)
at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:305)
at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:118)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:333)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:310)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:168)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:126)
at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:366)
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:765)
at org.eclipse.jetty.util.thread.QueuedThreadPool$2.run(QueuedThreadPool.java:683)
at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.ClassNotFoundException: com.google.gerrit.json.OutputFormat
at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
at java.lang.ClassLoader.loadClass(ClassLoader.java:419)
at java.net.FactoryURLClassLoader.loadClass(URLClassLoader.java:817)
at java.lang.ClassLoader.loadClass(ClassLoader.java:352)
... 41 more
[2020-02-10 03:31:31,310] [PluginScanner] WA
OK I used the .jar file at https://ostrovsky.org/gerrit/oauth-g0e144214cc.jar against gerrit-2.16.16
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
[2] https://ostrovsky.org/gerrit/oauth-g5e0febd09d.jar
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. :)
gerrit-oauth-provider 801c00b Enabled
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
gerrit-oauth-provider-v2.16.1.jar in my plugins folder?
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.
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?
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
stable-3.0: https://gerrit-review.googlesource.com/c/plugins/oauth/+/254332
stable-3.1 and master: https://gerrit-review.googlesource.com/c/plugins/oauth/+/254372
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.
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?
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
[2] https://gerrit-ci.gerritforge.com/job/plugin-oauth-bazel-stable-2.16