Merge pull request #366 from akkie/master

Use Plays new OpenIdClient implementation instead of the global OpenID object

silhouette/app/com/mohiva/play/silhouette/impl/providers/OpenIDProvider.scala

@@ -15,8 +15,6 @@
  */
 package com.mohiva.play.silhouette.impl.providers
 
-import java.net.URLEncoder
-
 import com.mohiva.play.silhouette.api._
 import com.mohiva.play.silhouette.api.util.ExtractableRequest
 import com.mohiva.play.silhouette.impl.exceptions.UnexpectedResponseException
@@ -63,38 +61,14 @@ trait OpenIDProvider extends SocialProvider with Logger {
         // Either we get the openID from request or we use the provider ID to retrieve the redirect URL
         val openID = request.extractString(OpenID).getOrElse(settings.providerURL)
         service.redirectURL(openID, resolveCallbackURL(settings.callbackURL)).map { url =>
-          val redirect = Results.Redirect(fix3749(url))
+          val redirect = Results.Redirect(url)
           logger.debug("[Silhouette][%s] Redirecting to: %s".format(id, url))
           Left(redirect)
         }.recover {
           case e => throw new UnexpectedResponseException(ErrorRedirectURL.format(id, e.getMessage), e)
         }
     }
   }
-
-  /**
-   * A temporary fix for: https://github.com/playframework/playframework/pull/3749
-   *
-   * @see https://github.com/playframework/playframework/issues/3740
-   * @see http://stackoverflow.com/questions/22041522/steam-openid-and-play-framework
-   * @param url The URL to fix.
-   * @param request The request.
-   * @tparam B The type of the request body.
-   * @return The fixed URL.
-   */
-  def fix3749[B](url: String)(implicit request: ExtractableRequest[B]) = {
-    if (request.extractString(OpenID).isDefined) {
-      // We've found a non-unique ID so this bug doesn't affect us
-      url
-    } else {
-      // We use "OpenID Provider driven identifier selection", so this bug affects us
-      val search = URLEncoder.encode(settings.providerURL, "UTF-8")
-      val replace = URLEncoder.encode("http://specs.openid.net/auth/2.0/identifier_select", "UTF-8")
-      url
-        .replace("openid.claimed_id=" + search, "openid.claimed_id=" + replace)
-        .replace("openid.identity=" + search, "openid.identity=" + replace)
-    }
-  }
 }
 
 /**

silhouette/app/com/mohiva/play/silhouette/impl/providers/openid/services/PlayOpenIDService.scala

@@ -20,19 +20,19 @@
 package com.mohiva.play.silhouette.impl.providers.openid.services
 
 import com.mohiva.play.silhouette.impl.providers.{ OpenIDInfo, OpenIDService, OpenIDSettings }
-import play.api.libs.openid.OpenID
+import play.api.libs.openid.OpenIdClient
 import play.api.mvc.Request
-import play.api.Play.current
 
 import scala.concurrent.{ ExecutionContext, Future }
 import scala.util.{ Failure, Success, Try }
 
 /**
  * The OpenID service implementation which wraps Play Framework's OpenID implementation.
  *
+ * @param client The OpenID client implementation.
  * @param settings The OpenID settings.
  */
-class PlayOpenIDService(settings: OpenIDSettings) extends OpenIDService {
+class PlayOpenIDService(client: OpenIdClient, settings: OpenIDSettings) extends OpenIDService {
 
   /**
    * Retrieve the URL where the user should be redirected to start the OpenID authentication process.
@@ -44,7 +44,7 @@ class PlayOpenIDService(settings: OpenIDSettings) extends OpenIDService {
    */
   override def redirectURL(openID: String, resolvedCallbackURL: String)(implicit ec: ExecutionContext): Future[String] = {
     Try {
-      OpenID.redirectURL(openID, resolvedCallbackURL, settings.axRequired, settings.axOptional, settings.realm)
+      client.redirectURL(openID, resolvedCallbackURL, settings.axRequired, settings.axOptional, settings.realm)
     } match {
       case Success(f) => f
       case Failure(e) => Future.failed(e)
@@ -60,7 +60,7 @@ class PlayOpenIDService(settings: OpenIDSettings) extends OpenIDService {
    * @return A OpenIDInfo in case of success, Exception otherwise.
    */
   override def verifiedID[B](implicit request: Request[B], ec: ExecutionContext) = Try {
-    OpenID.verifiedId.map(info => OpenIDInfo(info.id, info.attributes))
+    client.verifiedId(request).map(info => OpenIDInfo(info.id, info.attributes))
   } match {
     case Success(f) => f
     case Failure(e) => Future.failed(e)

silhouette/test/com/mohiva/play/silhouette/impl/providers/OpenIDProviderSpec.scala

@@ -15,8 +15,6 @@
  */
 package com.mohiva.play.silhouette.impl.providers
 
-import java.net.URLEncoder
-
 import com.mohiva.play.silhouette.api.util.HTTPLayer
 import com.mohiva.play.silhouette.impl.exceptions.UnexpectedResponseException
 import com.mohiva.play.silhouette.impl.providers.OpenIDProvider._
@@ -71,28 +69,6 @@ abstract class OpenIDProviderSpec extends SocialProviderSpec[OpenIDInfo] {
       }
     }
 
-    "fix bug 3749" in new WithApplication {
-      val e = (v: String) => URLEncoder.encode(v, "UTF-8")
-      implicit val req = FakeRequest()
-      c.openIDService.redirectURL(any, any)(any) returns Future.successful("https://domain.com/openid/login?openid.ns=http://specs.openid.net/auth/2.0"
-        + "&openid.mode=checkid_setup"
-        + "&openid.claimed_id=" + e(c.openIDSettings.providerURL)
-        + "&openid.identity=" + e(c.openIDSettings.providerURL)
-        + "&openid.return_to=http://www.mydomain.com/auth/domain"
-        + "&openid.realm=http://www.mydomain.com")
-
-      result(c.provider.authenticate()) {
-        case result =>
-          status(result) must equalTo(SEE_OTHER)
-          redirectLocation(result) must beSome.which(_ == "https://domain.com/openid/login?openid.ns=http://specs.openid.net/auth/2.0"
-            + "&openid.mode=checkid_setup"
-            + "&openid.claimed_id=" + e("http://specs.openid.net/auth/2.0/identifier_select")
-            + "&openid.identity=" + e("http://specs.openid.net/auth/2.0/identifier_select")
-            + "&openid.return_to=http://www.mydomain.com/auth/domain"
-            + "&openid.realm=http://www.mydomain.com")
-      }
-    }
-
     "resolves relative callbackURLs before starting the flow" in new WithApplication {
       verifyRelativeCallbackURLResolution("/callback-url", secure = false, "http://www.example.com/callback-url")
     }