diff --git a/app/com/mohiva/play/silhouette/core/providers/OAuth2Provider.scala b/app/com/mohiva/play/silhouette/core/providers/OAuth2Provider.scala index 4fda93e56..d973c9598 100644 --- a/app/com/mohiva/play/silhouette/core/providers/OAuth2Provider.scala +++ b/app/com/mohiva/play/silhouette/core/providers/OAuth2Provider.scala @@ -46,6 +46,11 @@ abstract class OAuth2Provider( extends SocialProvider[OAuth2Info] with Logger { + /** + * A list with headers to send to the API. + */ + protected val headers: Seq[(String, String)] = Seq() + /** * Converts the JSON into a [[com.mohiva.play.silhouette.core.providers.OAuth2Info]] object. */ @@ -102,7 +107,7 @@ abstract class OAuth2Provider( * @return The info containing the access token. */ protected def getAccessToken(code: String): Future[OAuth2Info] = { - httpLayer.url(settings.accessTokenURL).post(Map( + httpLayer.url(settings.accessTokenURL).withHeaders(headers: _*).post(Map( ClientID -> Seq(settings.clientID), ClientSecret -> Seq(settings.clientSecret), GrantType -> Seq(AuthorizationCode), diff --git a/app/com/mohiva/play/silhouette/core/providers/oauth2/GitHubProvider.scala b/app/com/mohiva/play/silhouette/core/providers/oauth2/GitHubProvider.scala index f6ceabb45..4fefe718c 100644 --- a/app/com/mohiva/play/silhouette/core/providers/oauth2/GitHubProvider.scala +++ b/app/com/mohiva/play/silhouette/core/providers/oauth2/GitHubProvider.scala @@ -19,7 +19,7 @@ */ package com.mohiva.play.silhouette.core.providers.oauth2 -import play.api.libs.ws.Response +import play.api.http.HeaderNames import scala.concurrent.Future import scala.concurrent.ExecutionContext.Implicits.global import com.mohiva.play.silhouette.core._ @@ -27,7 +27,6 @@ import com.mohiva.play.silhouette.core.utils.{ HTTPLayer, CacheLayer } import com.mohiva.play.silhouette.core.providers.{ SocialProfile, OAuth2Info, OAuth2Settings, OAuth2Provider } import com.mohiva.play.silhouette.core.services.AuthInfoService import GitHubProvider._ -import OAuth2Provider._ /** * A GitHub OAuth2 Provider. @@ -36,6 +35,7 @@ import OAuth2Provider._ * @param cacheLayer The cache layer implementation. * @param httpLayer The HTTP layer implementation. * @param settings The provider settings. + * @see https://developer.github.com/v3/oauth/ */ class GitHubProvider( protected val authInfoService: AuthInfoService, @@ -44,6 +44,16 @@ class GitHubProvider( settings: OAuth2Settings) extends OAuth2Provider(settings, cacheLayer, httpLayer) { + /** + * A list with headers to send to the API. + * + * Without defining the accept header, the response will take the following form: + * access_token=e72e16c7e42f292c6912e7710c838347ae178b4a&scope=user%2Cgist&token_type=bearer + * + * @see https://developer.github.com/v3/oauth/#response + */ + override protected val headers = Seq(HeaderNames.ACCEPT -> "application/json") + /** * Gets the provider ID. * @@ -61,7 +71,10 @@ class GitHubProvider( httpLayer.url(API.format(authInfo.accessToken)).get().map { response => val json = response.json (json \ Message).asOpt[String] match { - case Some(msg) => throw new AuthenticationException(SpecifiedProfileError.format(id, msg)) + case Some(msg) => + val docURL = (json \ DocURL).asOpt[String] + + throw new AuthenticationException(SpecifiedProfileError.format(id, msg, docURL)) case _ => val userID = (json \ ID).as[Int] val fullName = (json \ Name).asOpt[String] @@ -74,28 +87,9 @@ class GitHubProvider( avatarURL = avatarUrl, email = email) } - }.recover { case e => throw new AuthenticationException(UnspecifiedProfileError.format(id), e) } - } - - /** - * Builds the OAuth2 info. - * - * @param response The response from the provider. - * @return The OAuth2 info. - */ - override protected def buildInfo(response: Response): OAuth2Info = { - val values: Map[String, String] = response.body.split("&").toList - .map(_.split("=")).withFilter(_.size == 2) - .map(r => (r(0), r(1)))(collection.breakOut) - - values.get(AccessToken) match { - case Some(accessToken) => OAuth2Info( - accessToken, - values.get(TokenType), - values.get(ExpiresIn).map(_.toInt), - values.get(RefreshToken) - ) - case _ => throw new AuthenticationException(MissingAccessToken.format(id)) + }.recover { + case e if !e.isInstanceOf[AuthenticationException] => + throw new AuthenticationException(UnspecifiedProfileError.format(id), e) } } } @@ -109,8 +103,7 @@ object GitHubProvider { * The error messages. */ val UnspecifiedProfileError = "[Silhouette][%s] Error retrieving profile information" - val SpecifiedProfileError = "[Silhouette][%s] Error retrieving profile information. Error message: %s" - val MissingAccessToken = "[Silhouette][%s] Did not get access token" + val SpecifiedProfileError = "[Silhouette][%s] Error retrieving profile information. Error message: %s, doc URL: %s" /** * The Foursquare constants. @@ -118,6 +111,7 @@ object GitHubProvider { val GitHub = "github" val API = "https://api.github.com/user?access_token=%s" val Message = "message" + val DocURL = "documentation_url" val ID = "id" val Name = "name" val AvatarURL = "avatar_url" diff --git a/test/com/mohiva/play/silhouette/core/providers/OAuth2ProviderSpec.scala b/test/com/mohiva/play/silhouette/core/providers/OAuth2ProviderSpec.scala index e9dd45284..046d0d9aa 100644 --- a/test/com/mohiva/play/silhouette/core/providers/OAuth2ProviderSpec.scala +++ b/test/com/mohiva/play/silhouette/core/providers/OAuth2ProviderSpec.scala @@ -142,6 +142,8 @@ abstract class OAuth2ProviderSpec extends PlaySpecification with Mockito with Js RedirectURI -> Seq(c.oAuthSettings.redirectURL)) ++ c.oAuthSettings.accessTokenParams.mapValues(Seq(_)) implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) + requestHolder.withHeaders(any) returns requestHolder + // We must use this neat trick here because it isn't possible to check the post call with a verification, // because of the implicit params needed for the post call. On the other hand we can test it in the abstract // spec, because we throw an exception in both cases which stops the test once the post method was called. diff --git a/test/com/mohiva/play/silhouette/core/providers/oauth2/FacebookProviderSpec.scala b/test/com/mohiva/play/silhouette/core/providers/oauth2/FacebookProviderSpec.scala index 1c20c7908..ff7afd869 100644 --- a/test/com/mohiva/play/silhouette/core/providers/oauth2/FacebookProviderSpec.scala +++ b/test/com/mohiva/play/silhouette/core/providers/oauth2/FacebookProviderSpec.scala @@ -38,6 +38,7 @@ class FacebookProviderSpec extends OAuth2ProviderSpec { val response = mock[Response] implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.body returns "" + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) httpLayer.url(oAuthSettings.accessTokenURL) returns requestHolder @@ -55,6 +56,7 @@ class FacebookProviderSpec extends OAuth2ProviderSpec { implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.body returns AccessToken + "=my.access.token&" + Expires + "=1" response.json returns Helper.loadJson("providers/oauth2/facebook.error.json") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) @@ -78,6 +80,7 @@ class FacebookProviderSpec extends OAuth2ProviderSpec { implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.body returns AccessToken + "=my.access.token&" + Expires + "=1" response.json throws new RuntimeException("") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) @@ -97,6 +100,7 @@ class FacebookProviderSpec extends OAuth2ProviderSpec { implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.body returns AccessToken + "=my.access.token&" + Expires + "=1" response.json returns Helper.loadJson("providers/oauth2/facebook.success.json") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) @@ -124,6 +128,7 @@ class FacebookProviderSpec extends OAuth2ProviderSpec { implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.body returns AccessToken + "=my.access.token&" + Expires + "=1" response.json returns Helper.loadJson("providers/oauth2/facebook.success.json") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) @@ -146,6 +151,7 @@ class FacebookProviderSpec extends OAuth2ProviderSpec { implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.body returns AccessToken + "=my.access.token" response.json returns Helper.loadJson("providers/oauth2/facebook.success.json") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) diff --git a/test/com/mohiva/play/silhouette/core/providers/oauth2/FoursquareProviderSpec.scala b/test/com/mohiva/play/silhouette/core/providers/oauth2/FoursquareProviderSpec.scala index 2b65f3f06..597f9b275 100644 --- a/test/com/mohiva/play/silhouette/core/providers/oauth2/FoursquareProviderSpec.scala +++ b/test/com/mohiva/play/silhouette/core/providers/oauth2/FoursquareProviderSpec.scala @@ -39,6 +39,7 @@ class FoursquareProviderSpec extends OAuth2ProviderSpec { val response = mock[Response] implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.json returns Json.obj() + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) httpLayer.url(oAuthSettings.accessTokenURL) returns requestHolder @@ -55,6 +56,7 @@ class FoursquareProviderSpec extends OAuth2ProviderSpec { val response = mock[Response] implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.json returns oAuthInfo thenReturns Helper.loadJson("providers/oauth2/foursquare.error.json") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) @@ -77,6 +79,7 @@ class FoursquareProviderSpec extends OAuth2ProviderSpec { val response = mock[Response] implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.json returns oAuthInfo thenThrows new RuntimeException("") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) @@ -95,6 +98,7 @@ class FoursquareProviderSpec extends OAuth2ProviderSpec { val response = mock[Response] implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.json returns oAuthInfo thenReturns Helper.loadJson("providers/oauth2/foursquare.success.json") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) @@ -120,6 +124,7 @@ class FoursquareProviderSpec extends OAuth2ProviderSpec { val response = mock[Response] implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) response.json returns oAuthInfo thenReturns Helper.loadJson("providers/oauth2/foursquare.deprecated.json") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) @@ -153,6 +158,7 @@ class FoursquareProviderSpec extends OAuth2ProviderSpec { ) response.json returns oAuthInfo thenReturns Helper.loadJson("providers/oauth2/foursquare.success.json") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) @@ -186,6 +192,7 @@ class FoursquareProviderSpec extends OAuth2ProviderSpec { ) response.json returns oAuthInfo thenReturns Helper.loadJson("providers/oauth2/foursquare.success.json") + requestHolder.withHeaders(any) returns requestHolder requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) requestHolder.get() returns Future.successful(response) cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) diff --git a/test/com/mohiva/play/silhouette/core/providers/oauth2/GitHubProviderSpec.scala b/test/com/mohiva/play/silhouette/core/providers/oauth2/GitHubProviderSpec.scala new file mode 100644 index 000000000..239776f7e --- /dev/null +++ b/test/com/mohiva/play/silhouette/core/providers/oauth2/GitHubProviderSpec.scala @@ -0,0 +1,158 @@ +/** + * Copyright 2014 Mohiva Organisation (license at mohiva dot com) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.mohiva.play.silhouette.core.providers.oauth2 + +import test.Helper +import java.util.UUID +import play.api.libs.ws.{ Response, WS } +import play.api.test.{ FakeRequest, WithApplication } +import scala.concurrent.Future +import com.mohiva.play.silhouette.core.providers._ +import com.mohiva.play.silhouette.core.{ LoginInfo, AuthenticationException } +import GitHubProvider._ +import OAuth2Provider._ +import play.api.libs.json.{ JsValue, Json } +import play.api.http.HeaderNames + +/** + * Test case for the [[com.mohiva.play.silhouette.core.providers.oauth2.GitHubProvider]] class. + */ +class GitHubProviderSpec extends OAuth2ProviderSpec { + + "The authenticate method" should { + "throw AuthenticationException if OAuth2Info can be build because of an unexpected response" in new WithApplication with Context { + val cacheID = UUID.randomUUID().toString + val state = UUID.randomUUID().toString + val requestHolder = mock[WS.WSRequestHolder] + val response = mock[Response] + implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) + response.json returns Json.obj() + requestHolder.withHeaders(HeaderNames.ACCEPT -> "application/json") returns requestHolder + requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) + cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) + httpLayer.url(oAuthSettings.accessTokenURL) returns requestHolder + + await(provider.authenticate()) must throwAn[AuthenticationException].like { + case e => e.getMessage must startWith(InvalidResponseFormat.format(provider.id, "")) + } + } + + "throw AuthenticationException if API returns error" in new WithApplication with Context { + val cacheID = UUID.randomUUID().toString + val state = UUID.randomUUID().toString + val requestHolder = mock[WS.WSRequestHolder] + val response = mock[Response] + implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) + response.json returns oAuthInfo thenReturns Helper.loadJson("providers/oauth2/github.error.json") + requestHolder.withHeaders(HeaderNames.ACCEPT -> "application/json") returns requestHolder + requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) + requestHolder.get() returns Future.successful(response) + cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) + httpLayer.url(oAuthSettings.accessTokenURL) returns requestHolder + httpLayer.url(API.format("my.access.token")) returns requestHolder + + await(provider.authenticate()) must throwAn[AuthenticationException].like { + case e => e.getMessage must equalTo(SpecifiedProfileError.format( + provider.id, + "Bad credentials", + Some("http://developer.github.com/v3"))) + } + } + + "throw AuthenticationException if an unexpected error occurred" in new WithApplication with Context { + val cacheID = UUID.randomUUID().toString + val state = UUID.randomUUID().toString + val requestHolder = mock[WS.WSRequestHolder] + val response = mock[Response] + implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) + response.json returns oAuthInfo thenThrows new RuntimeException("") + requestHolder.withHeaders(HeaderNames.ACCEPT -> "application/json") returns requestHolder + requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) + requestHolder.get() returns Future.successful(response) + cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) + httpLayer.url(oAuthSettings.accessTokenURL) returns requestHolder + httpLayer.url(API.format("my.access.token")) returns requestHolder + + await(provider.authenticate()) must throwAn[AuthenticationException].like { + case e => e.getMessage must equalTo(UnspecifiedProfileError.format(provider.id)) + } + } + + "return the social profile" in new WithApplication with Context { + val cacheID = UUID.randomUUID().toString + val state = UUID.randomUUID().toString + val requestHolder = mock[WS.WSRequestHolder] + val response = mock[Response] + implicit val req = FakeRequest(GET, "?" + Code + "=my.code&" + State + "=" + state).withSession(CacheKey -> cacheID) + response.json returns oAuthInfo thenReturns Helper.loadJson("providers/oauth2/github.success.json") + requestHolder.withHeaders(HeaderNames.ACCEPT -> "application/json") returns requestHolder + requestHolder.post[Map[String, Seq[String]]](any)(any, any) returns Future.successful(response) + requestHolder.get() returns Future.successful(response) + cacheLayer.get[String](cacheID) returns Future.successful(Some(state)) + httpLayer.url(oAuthSettings.accessTokenURL) returns requestHolder + httpLayer.url(API.format("my.access.token")) returns requestHolder + + await(provider.authenticate()) must beRight.like { + case p => + p must be equalTo new SocialProfile( + loginInfo = LoginInfo(provider.id, "1"), + fullName = Some("Apollonia Vanova"), + email = Some("apollonia.vanova@watchmen.com"), + avatarURL = Some("https://github.com/images/error/apollonia_vanova.gif") + ) + } + } + } + + /** + * Defines the context for the abstract OAuth2 provider spec. + * + * @return The Context to use for the abstract OAuth2 provider spec. + */ + override protected def context: OAuth2ProviderSpecContext = new Context {} + + /** + * The context. + */ + trait Context extends OAuth2ProviderSpecContext { + + /** + * The OAuth2 settings. + */ + lazy val oAuthSettings = OAuth2Settings( + authorizationURL = "https://github.com/login/oauth/authorize", + accessTokenURL = "https://github.com/login/oauth/access_token", + redirectURL = "https://www.mohiva.com", + clientID = "my.client.id", + clientSecret = "my.client.secret", + scope = Some("repo,gist")) + + /** + * The OAuth2 info returned by GutHub + * + * @see https://developer.github.com/v3/oauth/#response + */ + override lazy val oAuthInfo: JsValue = Json.obj( + "access_token" -> "my.access.token", + "scope" -> "repo,gist", + "token_type" -> "bearer") + + /** + * The provider to test. + */ + lazy val provider = new GitHubProvider(authInfoService, cacheLayer, httpLayer, oAuthSettings) + } +} diff --git a/test/resources/providers/oauth2/github.error.json b/test/resources/providers/oauth2/github.error.json new file mode 100644 index 000000000..cd0fd7a7b --- /dev/null +++ b/test/resources/providers/oauth2/github.error.json @@ -0,0 +1,4 @@ +{ + "message": "Bad credentials", + "documentation_url": "http://developer.github.com/v3" +} diff --git a/test/resources/providers/oauth2/github.success.json b/test/resources/providers/oauth2/github.success.json new file mode 100644 index 000000000..50bb86520 --- /dev/null +++ b/test/resources/providers/oauth2/github.success.json @@ -0,0 +1,6 @@ +{ + "id": 1, + "avatar_url": "https://github.com/images/error/apollonia_vanova.gif", + "name": "Apollonia Vanova", + "email": "apollonia.vanova@watchmen.com" +}