joscha / play-authenticate

An authentication plugin for Play Framework 2.x (Java)
http://joscha.github.com/play-authenticate/
Other
807 stars 369 forks source link

IllegalArgumentException on authentication with Facebook provider #334

Closed velika12 closed 7 years ago

velika12 commented 7 years ago

Hello, we have recently moved to 0.7.1 version of Play Authenticate and got an weird exception today (the stacktrace is at the bottom). It seems like the response body from Facebook server happens to be in json format, but in FacebookAuthProvider you treat it as right formatted query parameter string

final List<NameValuePair> pairs = URLEncodedUtils.parse(
                    URI.create("/?" + query), "utf-8");

I have managed to fix it writing my own MyFacebookAuthProvider with the method

@Override
protected MyFacebookAuthInfo buildInfo(final WSResponse r) throws AccessTokenException {
    if (r.getStatus() >= 400) {
        throw new AccessTokenException(r.asJson().get(ERROR).get(MESSAGE).asText());
    } else {
        final JsonNode json = r.asJson();

        ObjectMapper mapper = new ObjectMapper();
        Map<String, Object> resultMap = mapper.convertValue(json, Map.class);

        URIBuilder builder = new URIBuilder();
        resultMap.forEach((k, v) -> builder.addParameter(k, v.toString()));
        String query = builder.toString();

    Logger.debug(query);

    final List<NameValuePair> pairs = URLEncodedUtils.parse(
    URI.create("/" + query), "utf-8");
    if (pairs.size() < 2) {
        throw new AccessTokenException();
    }
    final Map<String, String> m = new HashMap<String, String>(
        pairs.size());
        for (final NameValuePair nameValuePair : pairs) {
            m.put(nameValuePair.getName(), nameValuePair.getValue());
        }

        return new MyFacebookAuthInfo(m);
    }
}

I also had to change the constructor of MyFacebookAuthInfo this way as one of the fields in json response is not expires but expires_in.

public MyFacebookAuthInfo(final Map<String, String> m) {
        super(  m.get(OAuth2AuthProvider.Constants.ACCESS_TOKEN),
                new Date().getTime() +           Long.parseLong(m.get(OAuth2AuthProvider.Constants.EXPIRES_IN)) * 1000,
                m.get(OAuth2AuthProvider.Constants.REFRESH_TOKEN));
}

Am I doing smth wrong or is there really a bug in your provider?

Here is the stacktrace:

java.lang.IllegalArgumentException: Illegal character in query at index 2: /?{"access_token":"EAAD6U0NeXqoBAFnj5vmqsSZBYenSEoZArQYIADVHaycmAIfizmE1K76b79CbEbZBgoZBAwqhtwuZBqBsaMyPnqzD8cuutAjUZC3CgB6d41bpq9ZAnto2fSSCsDJ5x1gfCqU2r8Jm2QZBZCvD3fcqegkMGZCHjmsgsAxrsZD","token_type":"bearer","expires_in":5098517}
    at java.net.URI.create(URI.java:852)
    at com.feth.play.module.pa.providers.oauth2.facebook.FacebookAuthProvider.buildInfo(FacebookAuthProvider.java:89)
    at com.feth.play.module.pa.providers.oauth2.facebook.FacebookAuthProvider.buildInfo(FacebookAuthProvider.java:25)
    at com.feth.play.module.pa.providers.oauth2.OAuth2AuthProvider.getAccessToken(OAuth2AuthProvider.java:111)
    at com.feth.play.module.pa.providers.oauth2.OAuth2AuthProvider.authenticate(OAuth2AuthProvider.java:202)
    at com.feth.play.module.pa.PlayAuthenticate.handleAuthentication(PlayAuthenticate.java:438)
    at com.feth.play.module.pa.controllers.Authenticate.authenticate(Authenticate.java:13)
    at router.Routes$$anonfun$routes$1$$anonfun$applyOrElse$197$$anonfun$apply$237.apply(Routes.scala:5370)
    at router.Routes$$anonfun$routes$1$$anonfun$applyOrElse$197$$anonfun$apply$237.apply(Routes.scala:5370)
    at play.core.routing.HandlerInvokerFactory$$anon$4.resultCall(HandlerInvoker.scala:136)
    at play.core.routing.HandlerInvokerFactory$JavaActionInvokerFactory$$anon$14$$anon$3$$anon$1.invocation(HandlerInvoker.scala:127)
    at play.core.j.JavaAction$$anon$1.call(JavaAction.scala:70)
    at play.GlobalSettings$1.call(GlobalSettings.java:67)
    at play.core.j.JavaAction$$anonfun$7.apply(JavaAction.scala:94)
    at play.core.j.JavaAction$$anonfun$7.apply(JavaAction.scala:94)
    at scala.concurrent.impl.Future$PromiseCompletingRunnable.liftedTree1$1(Future.scala:24)
    at scala.concurrent.impl.Future$PromiseCompletingRunnable.run(Future.scala:24)
    at play.core.j.HttpExecutionContext$$anon$2.run(HttpExecutionContext.scala:40)
    at play.api.libs.iteratee.Execution$trampoline$.execute(Execution.scala:70)
    at play.core.j.HttpExecutionContext.execute(HttpExecutionContext.scala:32)
    at scala.concurrent.impl.Future$.apply(Future.scala:31)
    at scala.concurrent.Future$.apply(Future.scala:492)
    at play.core.j.JavaAction.apply(JavaAction.scala:94)
    at play.api.mvc.Action$$anonfun$apply$1$$anonfun$apply$4$$anonfun$apply$5.apply(Action.scala:105)
    at play.api.mvc.Action$$anonfun$apply$1$$anonfun$apply$4$$anonfun$apply$5.apply(Action.scala:105)
    at play.utils.Threads$.withContextClassLoader(Threads.scala:21)
    at play.api.mvc.Action$$anonfun$apply$1$$anonfun$apply$4.apply(Action.scala:104)
    at play.api.mvc.Action$$anonfun$apply$1$$anonfun$apply$4.apply(Action.scala:103)
    at scala.Option.map(Option.scala:146)
    at play.api.mvc.Action$$anonfun$apply$1.apply(Action.scala:103)
    at play.api.mvc.Action$$anonfun$apply$1.apply(Action.scala:96)
    at play.api.libs.iteratee.Iteratee$$anonfun$mapM$1.apply(Iteratee.scala:524)
    at play.api.libs.iteratee.Iteratee$$anonfun$mapM$1.apply(Iteratee.scala:524)
    at play.api.libs.iteratee.Iteratee$$anonfun$flatMapM$1.apply(Iteratee.scala:560)
    at play.api.libs.iteratee.Iteratee$$anonfun$flatMapM$1.apply(Iteratee.scala:560)
    at play.api.libs.iteratee.Iteratee$$anonfun$flatMap$1$$anonfun$apply$13.apply(Iteratee.scala:536)
    at play.api.libs.iteratee.Iteratee$$anonfun$flatMap$1$$anonfun$apply$13.apply(Iteratee.scala:536)
    at scala.concurrent.impl.Future$PromiseCompletingRunnable.liftedTree1$1(Future.scala:24)
    at scala.concurrent.impl.Future$PromiseCompletingRunnable.run(Future.scala:24)
    at akka.dispatch.TaskInvocation.run(AbstractDispatcher.scala:40)
    at akka.dispatch.ForkJoinExecutorConfigurator$AkkaForkJoinTask.exec(AbstractDispatcher.scala:397)
    at scala.concurrent.forkjoin.ForkJoinTask.doExec(ForkJoinTask.java:260)
    at scala.concurrent.forkjoin.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1339)
    at scala.concurrent.forkjoin.ForkJoinPool.runWorker(ForkJoinPool.java:1979)
    at scala.concurrent.forkjoin.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:107)
Caused by: java.net.URISyntaxException: Illegal character in query at index 2: /?{"access_token":"EAAD6U0NeXqoBAFnj5vmqsSZBYenSEoZArQYIADVHaycmAIfizmE1K76b79CbEbZBgoZBAwqhtwuZBqBsaMyPnqzD8cuutAjUZC3CgB6d41bpq9ZAnto2fSSCsDJ5x1gfCqU2r8Jm2QZBZCvD3fcqegkMGZCHjmsgsAxrsZD","token_type":"bearer","expires_in":5098517}
    at java.net.URI$Parser.fail(URI.java:2848)
    at java.net.URI$Parser.checkChars(URI.java:3021)
    at java.net.URI$Parser.parseHierarchical(URI.java:3111)
    at java.net.URI$Parser.parse(URI.java:3063)
    at java.net.URI.(URI.java:588)
    at java.net.URI.create(URI.java:850)
    ... 44 more
joscha commented 7 years ago

unless the response changed, this appears to be a bug - can you tried the latest version and see if it still appears?

gkovbasenko commented 7 years ago

I have got the same issue today as well. Yesterday Connect with Facebook was working. Is there a chance that Facebook has changed something on their side?

We use version 0.7.1.

makxx91 commented 7 years ago

I have same issue. Using 0.8.1 version.

joscha commented 7 years ago

Sounds like something widespread - can someone investigate further (@velika12, @gkovbasenko, @makxx91)? I can release a version with a fix and backport it to 0.7.x.

makxx91 commented 7 years ago

About MyFacebookAuthInfo and the Facebook response JSON. From facebook changelog - https://developers.facebook.com/docs/apps/changelog in section "Changes from v2.2 to v2.3" we can see that it always was expires_in and not expires

[Oauth Access Token] Format - The response format of https://www.facebook.com/v2.3/oauth/access_token returned when you exchange a code for an access_token now return valid JSON instead of being URL encoded. The new format of this response is {"access_token": {TOKEN}, "token_type":{TYPE}, "expires_in":{TIME}}. We made this update to be compliant with section 5.1 of RFC 6749.

gkovbasenko commented 7 years ago

Provided code by @velika12 helped me to resolve this issue.

joscha commented 7 years ago

Do you want to open a pull request for this?

On 29 Mar. 2017 8:19 pm, "gkovbasenko" notifications@github.com wrote:

Provided code by @velika12 https://github.com/velika12 helped me to resolve this issue.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/joscha/play-authenticate/issues/334#issuecomment-290032561, or mute the thread https://github.com/notifications/unsubscribe-auth/AALehsuezBmRVhkOIFY4CXxFVTEcMJJQks5rqiIJgaJpZM4MreZ2 .

gkovbasenko commented 7 years ago

I have done this in our custom class that we had created before and that extends FacebookAuthProvider. I could create a new PR for the 0.7.1 version.

schmave commented 7 years ago

Based on the API changelog (thanks @makxx91) it looks like Facebook turned off API v2.2 this past Saturday, March 25, so that's why the response changed format.

cnmuc commented 7 years ago

Based on @gkovbasenko's PR I could create a PR for Play 2.3. Think all Play versions need a fix for this problem.

joscha commented 7 years ago

@cnmuc :thumbsup:

joscha commented 7 years ago

fixed in 0.8.3 via #336