CAS authentication problems

[Mark-Booth]

We have been trying to upgrade to the latest version of gerrit-oauth-provider before the end of the month, so our replication jobs don't stop working, but I've been having problems with the new version of the oauth provider.

Our current production Gerrit server is v2.16.11.1 with gerrit-oauth-provider v2.16.1 (from github) connecting to our CAS 5.3 server.

I have tried to upgrade Gerrit v2.16.11.1 with the latest gerrit-oauth-provider (from gerritforge) on our test server, but after completing the CAS login, Gerrit would just return a "Server Error" page.

I also tried upgrading Gerrit to versions v2.16.16, v2.16.20 and v3.1.5 as well, but got the same result.

Looking in the Gerrit error_log we just see a WARN/ERRORpair of the following (abridged) form:

[2020-02-26 13:23:30,940] [HTTP-143] WARN  org.eclipse.jetty.server.HttpChannel : /oauth
java.lang.NullPointerException: secret
    at java.util.Objects.requireNonNull(Objects.java:228)
    at com.google.gerrit.extensions.auth.oauth.OAuthToken.<init>(OAuthToken.java:59)
...
[2020-02-26 13:23:30,941] [HTTP-143] ERROR com.google.gerrit.pgm.http.jetty.HiddenErrorHandler : Error in GET /oauth?code=OC-430-D1r...1VY&state=5fB...t0g
java.lang.NullPointerException: secret
    at java.util.Objects.requireNonNull(Objects.java:228)
    at com.google.gerrit.extensions.auth.oauth.OAuthToken.<init>(OAuthToken.java:59)

This morning we tried switching the CAS response format to JSON, but it doesn't appear to support that:

[2020-06-04 07:06:54,804] [HTTP-272] WARN  org.eclipse.jetty.server.HttpChannel : /oauth
com.github.scribejava.core.exceptions.OAuthException: Response body is incorrect. Can't extract a 'access_token=([^&]+)' from this: '{"access_token":"AT-2-kf5...mVF","token_type":"bearer","expires_in":28800}'
        at com.github.scribejava.core.extractors.OAuth2AccessTokenExtractor.extractParameter(OAuth2AccessTokenExtractor.java:70)
        at com.github.scribejava.core.extractors.OAuth2AccessTokenExtractor.extract(OAuth2AccessTokenExtractor.java:48)
        at com.github.scribejava.core.extractors.OAuth2AccessTokenExtractor.extract(OAuth2AccessTokenExtractor.java:16)
        at com.github.scribejava.core.oauth.OAuth20Service.sendAccessTokenRequestSync(OAuth20Service.java:53)
        at com.github.scribejava.core.oauth.OAuth20Service.getAccessToken(OAuth20Service.java:97)
        at com.github.scribejava.core.oauth.OAuth20Service.getAccessToken(OAuth20Service.java:92)
        at com.googlesource.gerrit.plugins.oauth.CasOAuthService.getAccessToken(CasOAuthService.java:160)
[2020-06-04 07:06:54,806] [HTTP-272] ERROR com.google.gerrit.pgm.http.jetty.HiddenErrorHandler : Error in GET /oauth?code=OC-2-YRx...xtH&state=3cT...ofM
com.github.scribejava.core.exceptions.OAuthException: Response body is incorrect. Can't extract a 'access_token=([^&]+)' from this: '{"access_token":"AT-2-kf5...mVF","token_type":"bearer","expires_in":28800}'
        at com.github.scribejava.core.extractors.OAuth2AccessTokenExtractor.extractParameter(OAuth2AccessTokenExtractor.java:70)
        at com.github.scribejava.core.extractors.OAuth2AccessTokenExtractor.extract(OAuth2AccessTokenExtractor.java:48)
        at com.github.scribejava.core.extractors.OAuth2AccessTokenExtractor.extract(OAuth2AccessTokenExtractor.java:16)
        at com.github.scribejava.core.oauth.OAuth20Service.sendAccessTokenRequestSync(OAuth20Service.java:53)
        at com.github.scribejava.core.oauth.OAuth20Service.getAccessToken(OAuth20Service.java:97)
        at com.github.scribejava.core.oauth.OAuth20Service.getAccessToken(OAuth20Service.java:92)
        at com.googlesource.gerrit.plugins.oauth.CasOAuthService.getAccessToken(CasOAuthService.java:160)

Apologies if I'm missing something, but any ideas or suggestions would be appreciated.

[davido]

I think it was something broken. Have you tried the latest version gerrit: 3.2 and plugin downloaded from official gerrit CI, from master: [1].

[1] https://gerrit-ci.gerritforge.com/view/Plugins-master/job/plugin-oauth-bazel-master-master/

[Mark-Booth]

I'll give it a try and let you know how it goes.

[davido]

This change introduced some regressions: [1], that were fixed in this change: [2].

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

[davido]

A similar problem was reported in #139.

[davido]

From quick looking what was done in the principle upgrade to the new Scribe java library, it seems correct to me: The extractor was preserved: [1] to be non JSON extractor: [2].

[1] https://gerrit-review.googlesource.com/c/plugins/oauth/+/253929/2/src/main/java/com/googlesource/gerrit/plugins/oauth/CasApi.java#49 [2] https://github.com/scribejava/scribejava/blob/master/scribejava-core/src/main/java/com/github/scribejava/core/extractors/OAuth2AccessTokenExtractor.java

[Mark-Booth]

I've upgraded our test Gerrit instance to v3.2.0, and tried the latest master build of gerrit-oauth-provider ([build 24|https://gerrit-ci.gerritforge.com/job/plugin-oauth-bazel-master-master/]) but got the same results:

[2020-06-05T12:20:21.403+0100] [HTTP-283] WARN  org.eclipse.jetty.server.HttpChannel : /oauth
java.lang.NullPointerException: secret
        at java.util.Objects.requireNonNull(Objects.java:228)
        at com.google.gerrit.extensions.auth.oauth.OAuthToken.<init>(OAuthToken.java:59)
...
[2020-06-05T12:20:21.409+0100] [HTTP-283] ERROR com.google.gerrit.pgm.http.jetty.HiddenErrorHandler : Error in GET /oauth?code=OC-18-8BC...996x&state=Utc...nwg%3D
java.lang.NullPointerException: secret
        at java.util.Objects.requireNonNull(Objects.java:228)
        at com.google.gerrit.extensions.auth.oauth.OAuthToken.<init>(OAuthToken.java:59)
{noformat}

[salk31]

Sorry if I've got the wrong end the stick but I think the dodgy line is: https://gerrit.googlesource.com/plugins/oauth/+/refs/heads/master/src/main/java/com/googlesource/gerrit/plugins/oauth/CasOAuthService.java#162

Which seems to be the same in 3.1 and master. It is passing the token_type into the OAuthToken constructor as the secret? At the very least this leads to a confusing error message.?

[davido]

Yes, something went wrong during Scribe version upgrade. @Mark-Booth can you dump (obfuscated) the response? The workaround would be to change this line and pass "" empty string as secret:

OAuth2AccessToken accessToken = service.getAccessToken(rv.getValue());
return new OAuthToken(
    accessToken.getAccessToken(),
    accessToken.getTokenType() == null ? "" : accessToken.getTokenType(),
    accessToken.getRawResponse());

[salk31]

@davido I didn't make a note of the actual non-JSON response. I only made a note for JSON and gerrit only logs it when it is JSON.

Not a lot to it. I think this is the code that generates it on the CAS side (in 5.3 anyway):

protected void generateTextInternal(final HttpServletRequest request,
                                        final HttpServletResponse response,
                                        final AccessToken accessTokenId,
                                        final RefreshToken refreshTokenId,
                                        final long timeout) {
        final StringBuilder builder = new StringBuilder(
                String.format("%s=%s&%s=%s", OAuth20Constants.ACCESS_TOKEN, accessTokenId.getId(),
                        OAuth20Constants.EXPIRES_IN, timeout));

        if (refreshTokenId != null) {
            builder.append('&')
                    .append(OAuth20Constants.REFRESH_TOKEN)
                    .append('=')
                    .append(refreshTokenId.getId());
        }
        OAuth20Utils.writeText(response, builder.toString(), HttpStatus.SC_OK);
    }

I can turn logging on again and grab it if you still need it.

[davido]

@salk31 Thanks. So, the "secret" is not provided by the CAS anyway. And given that Gerrit code wasn't changed, the problem was introduced by the Scribejava 6.9.0 migration change: [1].

Before that change it was:

Token to = service.getAccessToken(null, vi);
return new OAuthToken(to.getToken(), to.getSecret(), to.getRawResponse());

And it didn't ended with NPE because the service is:

public OAuthService createService(OAuthConfig config) {
    return new OAuth20ServiceImpl(this, config);
}

and get AccessToken is defined as

https://github.com/scribejava/scribejava/blob/1.3.7/src/main/java/org/scribe/oauth/OAuth20ServiceImpl.java#L28-L38

public Token getAccessToken(Token requestToken, Verifier verifier)
  {
    OAuthRequest request = new OAuthRequest(api.getAccessTokenVerb(), api.getAccessTokenEndpoint());
    request.addQuerystringParameter(OAuthConstants.CLIENT_ID, config.getApiKey());
    request.addQuerystringParameter(OAuthConstants.CLIENT_SECRET, config.getApiSecret());
    request.addQuerystringParameter(OAuthConstants.CODE, verifier.getValue());
    request.addQuerystringParameter(OAuthConstants.REDIRECT_URI, config.getCallback());
    if(config.hasScope()) request.addQuerystringParameter(OAuthConstants.SCOPE, config.getScope());
    Response response = request.send();
    return api.getAccessTokenExtractor().extract(response.getBody());
  }

And getAccessTokenExtractor() is defined as:

https://github.com/scribejava/scribejava/blob/1.3.7/src/main/java/org/scribe/builder/api/DefaultApi20.java#L31-L34

return new TokenExtractor20Impl();

where it's:

https://github.com/scribejava/scribejava/blob/1.3.7/src/main/java/org/scribe/extractors/TokenExtractor20Impl.java#L12-L35

that is hard code the secret to be EMPTY_LINE anway:

public class TokenExtractor20Impl implements AccessTokenExtractor
{
  private static final String TOKEN_REGEX = "access_token=([^&]+)";
  private static final String EMPTY_SECRET = "";

  /**
   * {@inheritDoc} 
   */
  public Token extract(String response)
  {
    Preconditions.checkEmptyString(response, "Response body is incorrect. Can't extract a token from an empty string");

    Matcher matcher = Pattern.compile(TOKEN_REGEX).matcher(response);
    if (matcher.find())
    {
      String token = OAuthEncoder.decode(matcher.group(1));
      return new Token(token, EMPTY_SECRET, response);
    } 
    else
    {
      throw new OAuthException("Response body is incorrect. Can't extract a token from this: '" + response + "'", null);
    }
  }
}

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

[salk31]

@davido yes. gerrit complaining that the "secret" was null threw me as I assumed it must be failing to get it from config. I believe the secret is only used by the client in the final request to the CAS.

In the JSON version of the CAS response token_type is hard coded to "Bearer" anyway so doesn't add lot of value.

Seems confusing to me that the token implementation require a secret when not used at every stage.

Much appreciate your help.

[davido]

Much appreciate your help.

Let me upload a fix for CAS OAuth2 provider regression.

[davido]

Let me upload a fix for CAS OAuth2 provider regression.

The fix is here: [1].

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

[Mark-Booth]

After good results on our test server, we have deployed this to our production server.

Thanks for all your help @davido it is very much appreciated.

[davido]

@Mark-Booth Thanks for the confirmation and sorry for the breakage. I will merge this series up to master right now.