modify optional clientSecret #36

Author: tsuyoshizawa

scala-oauth2-core/src/main/scala/scalaoauth2/provider/ClientCredentialFetcher.scala

@@ -2,7 +2,7 @@ package scalaoauth2.provider
 
 import org.apache.commons.codec.binary.Base64
 
-case class ClientCredential(clientId: String, clientSecret: String)
+case class ClientCredential(clientId: String, clientSecret: Option[String])
 
 trait ClientCredentialFetcher {
 
@@ -17,15 +17,14 @@ trait ClientCredentialFetcher {
         val decoded = new String(Base64.decodeBase64(authorization.getBytes), "UTF-8")
         if (decoded.indexOf(':') > 0) {
           decoded.split(":", 2) match {
-            case Array(clientId, clientSecret) => Some(ClientCredential(clientId, clientSecret))
-            case Array(clientId) => Some(ClientCredential(clientId, ""))
-            case _ => None
+            case Array(clientId, clientSecret) => Some(ClientCredential(clientId, if (clientSecret == "") None else Some(clientSecret)))
+            case Array(clientId) => Some(ClientCredential(clientId, None))
           }
         } else {
           None
         }
       case _ => request.clientId.map { clientId =>
-        ClientCredential(clientId, request.clientSecret.getOrElse(""))
+        ClientCredential(clientId, request.clientSecret)
       }
     }
   }

scala-oauth2-core/src/main/scala/scalaoauth2/provider/DataHandler.scala

@@ -3,8 +3,6 @@ package scalaoauth2.provider
 import java.util.Date
 import scala.concurrent.Future
 
-case class AccessTokenRequest[U](clientId: String, clientSecret: String, user: U)
-
 /**
  * Access token
  *
@@ -33,7 +31,7 @@ case class AuthInfo[+U](user: U, clientId: Option[String], scope: Option[String]
  * 
  * <h4>Authorization Code Grant</h4>
  * <ul>
- *   <li>validateClient(clientId, clientSecret, grantType)</li>
+ *   <li>validateClient(clientCredential, grantType)</li>
  *   <li>findAuthInfoByCode(code)</li>
  *   <li>getStoredAccessToken(authInfo)</li>
  *   <li>isAccessTokenExpired(token)</li>
@@ -43,14 +41,14 @@ case class AuthInfo[+U](user: U, clientId: Option[String], scope: Option[String]
  * 
  * <h4>Refresh Token Grant</h4>
  * <ul>
- *   <li>validateClient(clientId, clientSecret, grantType)</li>
+ *   <li>validateClient(clientCredential, grantType)</li>
  *   <li>findAuthInfoByRefreshToken(refreshToken)</li>
  *   <li>refreshAccessToken(authInfo, refreshToken)</li>
  * </ul>
  * 
  * <h4>Resource Owner Password Credentials Grant</h4>
  * <ul>
- *   <li>validateClient(clientId, clientSecret, grantType)</li>
+ *   <li>validateClient(clientCredential, grantType)</li>
  *   <li>findUser(username, password)</li>
  *   <li>getStoredAccessToken(authInfo)</li>
  *   <li>isAccessTokenExpired(token)</li>
@@ -60,8 +58,8 @@ case class AuthInfo[+U](user: U, clientId: Option[String], scope: Option[String]
  * 
  * <h4>Client Credentials Grant</h4>
  * <ul>
- *   <li>validateClient(clientId, clientSecret, grantType)</li>
- *   <li>findClientUser(clientId, clientSecret)</li>
+ *   <li>validateClient(clientCredential, grantType)</li>
+ *   <li>findClientUser(clientCredential)</li>
  *   <li>getStoredAccessToken(authInfo)</li>
  *   <li>isAccessTokenExpired(token)</li>
  *   <li>refreshAccessToken(authInfo, token)
@@ -80,19 +78,18 @@ trait DataHandler[U] {
   /**
    * Verify proper client with parameters for issue an access token.
    *
-   * @param clientId Client send this value which is registered by application.
-   * @param clientSecret Client send this value which is registered by application.
-   * @param grantType Client send this value which is registered by application.
+   * @param clientCredential Client sends clientId and clientSecret which are registered by application.
+   * @param grantType Client sends this value which is registered by application.
    * @return true if request is a regular client, false if request is a illegal client.
    */
-  def validateClient(clientId: String, clientSecret: String, grantType: String): Future[Boolean]
+  def validateClient(clientCredential: ClientCredential, grantType: String): Future[Boolean]
 
   /**
    * Find userId with username and password these are used on your system.
    * If you don't support Resource Owner Password Credentials Grant then doesn't need implementing.
    *
-   * @param username Client send this value which is used on your system.
-   * @param password Client send this value which is used on your system.
+   * @param username Client sends this value which is used on your system.
+   * @param password Client sends this value which is used on your system.
    * @return Including UserId to Option if could find the user, None if couldn't find.
    */
   def findUser(username: String, password: String): Future[Option[U]]
@@ -128,7 +125,7 @@ trait DataHandler[U] {
    *
    * If you don't support Authorization Code Grant then doesn't need implementing.
    *
-   * @param code Client send authorization code which is registered by system.
+   * @param code Client sends authorization code which is registered by system.
    * @return Return authorized information that matched the code.
    */
   def findAuthInfoByCode(code: String): Future[Option[AuthInfo[U]]]
@@ -138,7 +135,7 @@ trait DataHandler[U] {
    *
    * If you don't support Refresh Token Grant then doesn't need implementing.
    *
-   * @param refreshToken Client send refresh token which is created by system.
+   * @param refreshToken Client sends refresh token which is created by system.
    * @return Return authorized information that matched the refresh token.
    */
   def findAuthInfoByRefreshToken(refreshToken: String): Future[Option[AuthInfo[U]]]
@@ -148,16 +145,15 @@ trait DataHandler[U] {
    *
    * If you don't support Client Credentials Grant then doesn't need implementing.
    *
-   * @param clientId Client send this value which is registered by application.
-   * @param clientSecret Client send this value which is registered by application.
+   * @param clientCredential Client sends clientId and clientSecret which are registered by application.
    * @return Return user that matched both values.
    */
-  def findClientUser(clientId: String, clientSecret: String, scope: Option[String]): Future[Option[U]]
+  def findClientUser(clientCredential: ClientCredential, scope: Option[String]): Future[Option[U]]
 
   /**
    * Find AccessToken object by access token code.
    *
-   * @param token Client send access token which is created by system.
+   * @param token Client sends access token which is created by system.
    * @return Return access token that matched the token.
    */
   def findAccessToken(token: String): Future[Option[AccessToken]]

scala-oauth2-core/src/main/scala/scalaoauth2/provider/GrantHandler.scala

@@ -95,7 +95,7 @@ class ClientCredentials extends GrantHandler {
     val clientCredential = optionalClientCredential.getOrElse(throw new InvalidRequest("Client credential is required"))
     val scope = request.scope
 
-    dataHandler.findClientUser(clientCredential.clientId, clientCredential.clientSecret, scope).flatMap { optionalUser =>
+    dataHandler.findClientUser(clientCredential, scope).flatMap { optionalUser =>
       val user = optionalUser.getOrElse(throw new InvalidGrant("client_id or client_secret or scope is incorrect"))
       val authInfo = AuthInfo(user, Some(clientCredential.clientId), scope, None)
 

scala-oauth2-core/src/main/scala/scalaoauth2/provider/TokenEndpoint.scala

@@ -18,7 +18,7 @@ trait TokenEndpoint {
     val handler = handlers.get(grantType).getOrElse(throw new UnsupportedGrantType("The grant_type is not supported"))
 
     fetcher.fetch(request).map { clientCredential =>
-      dataHandler.validateClient(clientCredential.clientId, clientCredential.clientSecret, grantType).flatMap { validClient =>
+      dataHandler.validateClient(clientCredential, grantType).flatMap { validClient =>
         if (!validClient) {
           Future.successful(Left(throw new InvalidClient()))
         } else {

scala-oauth2-core/src/test/scala/scalaoauth2/provider/AuthorizationCodeSpec.scala

@@ -11,7 +11,7 @@ class AuthorizationCodeSpec extends FlatSpec with ScalaFutures {
   it should "handle request" in {
     val authorizationCode = new AuthorizationCode()
     val request = AuthorizationRequest(Map(), Map("code" -> Seq("code1"), "redirect_uri" -> Seq("http://example.com/")))
-    val f = authorizationCode.handleRequest(request, Some(ClientCredential("clientId1", "clientSecret1")), new MockDataHandler() {
+    val f = authorizationCode.handleRequest(request, Some(ClientCredential("clientId1", Some("clientSecret1"))), new MockDataHandler() {
 
       override def findAuthInfoByCode(code: String): Future[Option[AuthInfo[User]]] = Future.successful(Some(
         AuthInfo(user = MockUser(10000, "username"), clientId = Some("clientId1"), scope = Some("all"), redirectUri = Some("http://example.com/"))
@@ -32,7 +32,7 @@ class AuthorizationCodeSpec extends FlatSpec with ScalaFutures {
   it should "handle request if redirectUrl is none" in {
     val authorizationCode = new AuthorizationCode()
     val request = AuthorizationRequest(Map(), Map("code" -> Seq("code1"), "redirect_uri" -> Seq("http://example.com/")))
-    val f = authorizationCode.handleRequest(request, Some(ClientCredential("clientId1", "clientSecret1")), new MockDataHandler() {
+    val f = authorizationCode.handleRequest(request, Some(ClientCredential("clientId1", Some("clientSecret1"))), new MockDataHandler() {
 
       override def findAuthInfoByCode(code: String): Future[Option[AuthInfo[MockUser]]] = Future.successful(Some(
         AuthInfo(user = MockUser(10000, "username"), clientId = Some("clientId1"), scope = Some("all"), redirectUri = None)

scala-oauth2-core/src/test/scala/scalaoauth2/provider/ClientCredentialFetcherSpec.scala

@@ -9,24 +9,29 @@ class ClientCredentialFetcherSpec extends FlatSpec {
     val request = AuthorizationRequest(Map("Authorization" -> Seq("Basic Y2xpZW50X2lkX3ZhbHVlOmNsaWVudF9zZWNyZXRfdmFsdWU=")), Map())
     val Some(c) = ClientCredentialFetcher.fetch(request)
     c.clientId should be ("client_id_value")
-    c.clientSecret should be ("client_secret_value")
+    c.clientSecret should be (Some("client_secret_value"))
   }
 
   it should "fetch Basic64 by case insensitive" in {
     val request = AuthorizationRequest(Map("authorization" -> Seq("Basic Y2xpZW50X2lkX3ZhbHVlOmNsaWVudF9zZWNyZXRfdmFsdWU=")), Map())
     val Some(c) = ClientCredentialFetcher.fetch(request)
     c.clientId should be ("client_id_value")
-    c.clientSecret should be ("client_secret_value")
+    c.clientSecret should be (Some("client_secret_value"))
   }
 
-  it should "fetch empty client_secret" in {
+  it should "fetch authorization header without colon" in {
+    val request = AuthorizationRequest(Map("Authorization" -> Seq("Basic Y2xpZW50X2lkX3ZhbHVl")), Map())
+    ClientCredentialFetcher.fetch(request) should be (None)
+  }
+
+  it should "fetch empty client_secret with colon" in {
     val request = AuthorizationRequest(Map("Authorization" -> Seq("Basic Y2xpZW50X2lkX3ZhbHVlOg==")), Map())
     val Some(c) = ClientCredentialFetcher.fetch(request)
     c.clientId should be ("client_id_value")
-    c.clientSecret should be ("")
+    c.clientSecret should be (None)
   }
 
-  it should "not fetch no Authorization key in header" in {
+  it should "not fetch not Authorization key in header" in {
     val request = AuthorizationRequest(Map("authorizatio" -> Seq("Basic Y2xpZW50X2lkX3ZhbHVlOmNsaWVudF9zZWNyZXRfdmFsdWU=")), Map())
     ClientCredentialFetcher.fetch(request) should be (None)
   }
@@ -40,13 +45,13 @@ class ClientCredentialFetcherSpec extends FlatSpec {
     val request = AuthorizationRequest(Map(), Map("client_id" -> Seq("client_id_value"), "client_secret" -> Seq("client_secret_value")))
     val Some(c) = ClientCredentialFetcher.fetch(request)
     c.clientId should be ("client_id_value")
-    c.clientSecret should be ("client_secret_value")
+    c.clientSecret should be (Some("client_secret_value"))
   }
 
   it should "omit client_secret" in {
     val Some(c) = ClientCredentialFetcher.fetch(AuthorizationRequest(Map(), Map("client_id" -> Seq("client_id_value"))))
     c.clientId should be ("client_id_value")
-    c.clientSecret should be ("")
+    c.clientSecret should be (None)
   }
 
   it should "not fetch missing parameter" in {
@@ -65,6 +70,6 @@ class ClientCredentialFetcherSpec extends FlatSpec {
     )
     val Some(c) = ClientCredentialFetcher.fetch(request)
     c.clientId should be ("client_id_value")
-    c.clientSecret should be ("client_secret_value")
+    c.clientSecret should be (Some("client_secret_value"))
   }
 }

scala-oauth2-core/src/test/scala/scalaoauth2/provider/ClientCredentialsSpec.scala

@@ -11,9 +11,9 @@ class ClientCredentialsSpec extends FlatSpec with ScalaFutures {
   it should "handle request" in {
     val clientCredentials = new ClientCredentials()
     val request = AuthorizationRequest(Map(), Map("scope" -> Seq("all")))
-    val f = clientCredentials.handleRequest(request, Some(ClientCredential("clientId1", "clientSecret1")), new MockDataHandler() {
+    val f = clientCredentials.handleRequest(request, Some(ClientCredential("clientId1", Some("clientSecret1"))), new MockDataHandler() {
 
-      override def findClientUser(clientId: String, clientSecret: String, scope: Option[String]): Future[Option[User]] = Future.successful(Some(MockUser(10000, "username")))
+      override def findClientUser(clientCredential: ClientCredential, scope: Option[String]): Future[Option[User]] = Future.successful(Some(MockUser(10000, "username")))
 
       override def createAccessToken(authInfo: AuthInfo[User]): Future[AccessToken] = Future.successful(AccessToken("token1", None, Some("all"), Some(3600), new java.util.Date()))
     })

scala-oauth2-core/src/test/scala/scalaoauth2/provider/MockDataHandler.scala

@@ -8,7 +8,7 @@ case class MockUser(id: Long, name: String) extends User
 
 class MockDataHandler extends DataHandler[User] {
 
-  def validateClient(clientId: String, clientSecret: String, grantType: String): Future[Boolean] = Future.successful(false)
+  def validateClient(clientCredential: ClientCredential, grantType: String): Future[Boolean] = Future.successful(false)
 
   def findUser(username: String, password: String): Future[Option[User]] = Future.successful(None)
 
@@ -18,7 +18,7 @@ class MockDataHandler extends DataHandler[User] {
 
   def findAuthInfoByRefreshToken(refreshToken: String): Future[Option[AuthInfo[User]]] = Future.successful(None)
 
-  def findClientUser(clientId: String, clientSecret: String, scope: Option[String]): Future[Option[User]] = Future.successful(None)
+  def findClientUser(clientCredential: ClientCredential, scope: Option[String]): Future[Option[User]] = Future.successful(None)
 
   def findAccessToken(token: String): Future[Option[AccessToken]] = Future.successful(None)
 

scala-oauth2-core/src/test/scala/scalaoauth2/provider/PasswordSpec.scala

@@ -13,7 +13,7 @@ class PasswordSpec extends FlatSpec with ScalaFutures {
     override def clientCredentialRequired = false
   }
 
-  "Password when client credential required" should "handle request" in handlesRequest(passwordClientCredReq, Some(ClientCredential("clientId1", "clientSecret1")))
+  "Password when client credential required" should "handle request" in handlesRequest(passwordClientCredReq, Some(ClientCredential("clientId1", Some("clientSecret1"))))
   "Password when client credential not required" should "handle request" in handlesRequest(passwordNoClientCredReq, None)
 
   def handlesRequest(password: Password, clientCredential: Option[ClientCredential]) = {

scala-oauth2-core/src/test/scala/scalaoauth2/provider/RefreshTokenSpec.scala

@@ -11,7 +11,7 @@ class RefreshTokenSpec extends FlatSpec with ScalaFutures {
   it should "handle request" in {
     val refreshToken = new RefreshToken()
     val request = AuthorizationRequest(Map(), Map("refresh_token" -> Seq("refreshToken1")))
-    val f = refreshToken.handleRequest(request, Some(ClientCredential("clientId1", "clientSecret1")), new MockDataHandler() {
+    val f = refreshToken.handleRequest(request, Some(ClientCredential("clientId1", Some("clientSecret1"))), new MockDataHandler() {
 
       override def findAuthInfoByRefreshToken(refreshToken: String): Future[Option[AuthInfo[User]]] =
         Future.successful(Some(AuthInfo(user = MockUser(10000, "username"), clientId = Some("clientId1"), scope = None, redirectUri = None)))