WIP: squash

src/main/resources/reference.conf

@@ -233,7 +233,7 @@ omadataoauth2 = {
       ]
     },
     {
-      client_id = "dvvdigilompakkopk"
+      client_id = "oauth2client"
       redirect_uris = [
         "http://localhost:7021/koski/omadata-oauth2/debug-post-response"
         "/koski/omadata-oauth2/debug-post-response"

src/main/scala/fi/oph/koski/koskiuser/MockUsers.scala

@@ -36,6 +36,17 @@ object MockUsers {
     )))
   )
 
+  val rekisteröimätönOmadataOAuth2Palvelukäyttäjä = KoskiMockUser(
+    "oauth2clienteirek",
+    "oauth2clienteirek",
+    "1.2.246.562.24.99999984729",
+    Seq(OrganisaatioJaKäyttöoikeudet(MockOrganisaatiot.dvv, List(
+      PalveluJaOikeus("KOSKI", Rooli.OMADATAOAUTH2_OPISKELUOIKEUDET_SUORITETUT_TUTKINNOT),
+      PalveluJaOikeus("KOSKI", Rooli.OMADATAOAUTH2_HENKILOTIEDOT_NIMI),
+      PalveluJaOikeus("KOSKI", Rooli.OMADATAOAUTH2_HENKILOTIEDOT_SYNTYMAAIKA),
+    )))
+  )
+
   val omniaPalvelukäyttäjä = KoskiMockUser(
     "käyttäjä",
     "omnia-palvelukäyttäjä",
@@ -608,7 +619,8 @@ object MockUsers {
     xssHyökkääjä,
     muuKuinSäänneltyKoulutusYritys,
     pohjoiskalotinKoulutussäätiöKäyttäjä,
-    omadataOAuth2Palvelukäyttäjä
+    omadataOAuth2Palvelukäyttäjä,
+    rekisteröimätönOmadataOAuth2Palvelukäyttäjä
   )
 }
 

src/main/scala/fi/oph/koski/omadataoauth2/OmaDataOAuth2AuthorizationServerServlet.scala

@@ -13,51 +13,118 @@ import org.scalatra.i18n.I18nSupport
 
 class OmaDataOAuth2AuthorizationServerServlet(implicit val application: KoskiApplication)
   extends KoskiSpecificApiServlet
-    with Logging with ContentEncodingSupport with NoCache with FormSupport with I18nSupport with RequiresOmaDataOAuth2
-{
+    with Logging with ContentEncodingSupport with NoCache with FormSupport with I18nSupport with RequiresOmaDataOAuth2 with OmaDataOAuth2Support {
 
   // in: auth code yms. OAuth2 headerit
   // out: access token, jos käyttäjällä oikeudet kyseiseen authorization codeen.
-  //      TAI OAuth2-protokollan mukainen virheilmoitus (joka luotetaan nginx:n välittävän sellaisenaan, jos pyyntö on tänne asti tullut?)
-  // TODO: Lisävarmistus, että hyväksytään vain "application/x-www-form-urlencoded"-tyylinen input?
+  //      TAI OAuth2-protokollan mukainen virheilmoitus (
+  //      TODO: TOR-2210 joka luotetaan nginx:n välittävän sellaisenaan, jos pyyntö on tänne asti tullut?)
+  // TODO: TOR-2210 Lisävarmistus, että hyväksytään vain "application/x-www-form-urlencoded"-tyylinen input?
   post("/") {
-    val result = validate(AccessTokenRequest.formAccessTokenRequest)(
+    val result: AccessTokenResponse = validate(AccessTokenRequest.formAccessTokenRequest)(
       (errors: Seq[(String, String)]) => {
-        // TODO: OAuth2-standardin mukaiset virheet
-        Left(KoskiErrorCategory.badRequest(errors.map { case (a, b) => s"${a}: ${b}" }.mkString(";")))
+        val validationError = ValidationError(ValidationErrorType.invalid_request, errors.map { case (a, b) => s"${a}: ${b}" }.mkString(";"))
+
+        logger.warn(validationError.getLoggedErrorMessage)
+        AccessTokenErrorResponse(validationError)
       },
       (accessTokenRequest: AccessTokenRequest) => {
-        // TODO: Oikea toteutus ja yksikkötestit
-        // Bearer-token spec: https://www.rfc-editor.org/rfc/rfc6750
-        Right(AccessTokenResponse(access_token = "dummy-access-token", token_type = "Bearer", expires_in = 86400))
+        validateAccessTokenRequest(accessTokenRequest) match {
+          case Left(validationError) =>
+
+            logger.warn(validationError.getLoggedErrorMessage)
+            AccessTokenErrorResponse(validationError)
+          case _ =>
+            // TODO: Oikea toteutus ja yksikkötestit
+            // Bearer-token spec: https://www.rfc-editor.org/rfc/rfc6750
+
+            AccessTokenSuccessResponse(
+              access_token = "dummy-access-token",
+              token_type = "Bearer",
+              expires_in = 86400
+            )
+        }
       }
     )
 
-    renderEither(result)
+    response.setStatus(result.httpStatus)
+    renderObject(result)
+  }
+
+  protected def validateAccessTokenRequest(request: AccessTokenRequest): Either[ValidationError, Unit] = {
+    for {
+      _ <- validateClientId(request.client_id)
+    } yield ()
+  }
+
+  protected def validateClientId(clientIdParam: Option[String]): Either[ValidationError, String] = {
+    clientIdParam match {
+      case Some(clientId) =>
+        // clientId annettu parametrina, tarkista, että vastaa mtls:n kautta saatua
+        for {
+          clientId <- validateClientIdRekisteröity(clientId)
+          _ <- validateClientIdSamaKuinKäyttäjätunnus(clientId)
+        } yield clientId
+      case _ =>
+        // käytä mtls:n kautta saatua client_id:tä
+        for {
+          clientId <- validateClientIdRekisteröity(koskiSession.user.username)
+        } yield clientId
+    }
   }
-}
 
+  protected def validateClientIdSamaKuinKäyttäjätunnus(clientId: String): Either[ValidationError, String] = {
+    if (koskiSession.user.username == clientId) {
+      Right(clientId)
+    } else {
+      Left(ValidationError(ValidationErrorType.invalid_client_data, s"Annettu client_id ${clientId} on eri kuin mTLS-käyttäjä ${koskiSession.user.username}"))
+    }
+  }
+}
 object AccessTokenRequest {
-  // TODO: Voiko tehdä omia dynaaamisia constrainteja, jotka tsekkaa muitakin asioita, esim. tietokannasta asti?
   val formAccessTokenRequest: MappingValueType[AccessTokenRequest] = mapping(
     "grant_type" -> label("grant_type", text(required, oneOf(Seq("authorization_code")))),
-    "code" -> label("code", text(required, oneOf(Seq("foobar")))), // TODO: Oikea koodi-patternin validointi?
-    "redirect_uri" -> label("redirect_uri", optional(text())), // TODO: URI pattern matching? Oikeasti tarve on verrata, että tämä täsmälleen sama, millä on luotu
-    "client_id" -> label("client_id", optional(text())) // TODO: Tsekkaa, että vastaa käyttäjää, jolla tultiin sisään, jos on annettu?
+    "code" -> label("code", text(required, oneOf(Seq("foobar")))), // TODO: TOR-2210 Oikea koodi-patternin validointi?
+    "redirect_uri" -> label("redirect_uri", optional(text())), // TODO: TOR-2210 Vertaa, että tämän sisältö on täsmälleen sama kuin alkuperäisessä pyynnössä, jos siellä on redirect_uri annettu
+    "client_id" -> label("client_id", optional(text()))
   )(AccessTokenRequest.apply)
 }
 
 case class AccessTokenRequest(
   grant_type: String,
   code: String,
-  redirect_uri: Option[String],
+  redirect_uri: Option[String], // Pitää olla annettu ja sama, jos on autorisointiöpyynnössä annettu
   client_id: Option[String]
 )
 
-case class AccessTokenResponse(
+trait AccessTokenResponse {
+  def httpStatus: Int
+}
+
+case class AccessTokenSuccessResponse(
   access_token: String,
   token_type: String,
   expires_in: Long,
   // refresh_token: Option[String] // jos tarvitaan refresh token
   // scope: Option[String] // jos aletaan joskus tukea scopen vaihtoa tässä vaiheessa
-)
+) extends AccessTokenResponse {
+  val httpStatus = 200
+}
+
+object AccessTokenErrorResponse {
+  def apply(validationError: ValidationError): AccessTokenErrorResponse = {
+    AccessTokenErrorResponse(
+      "invalid_client",
+      Some(validationError.getLoggedErrorMessage),
+      None
+    )
+  }
+}
+
+case class AccessTokenErrorResponse(
+  error: String,
+  error_description: Option[String],
+  error_uri: Option[String]
+) extends AccessTokenResponse {
+  val httpStatus = 400
+}

src/main/scala/fi/oph/koski/omadataoauth2/OmaDataOAuth2Support.scala

@@ -29,10 +29,10 @@ trait OmaDataOAuth2Support extends ScalatraServlet with OmaDataOAuth2Config {
     }
   }
 
-  private def validateClientIdRekisteröity(clientId: String): Either[ValidationError, Unit] = {
+  protected def validateClientIdRekisteröity(clientId: String): Either[ValidationError, String] = {
     // TODO: TOR-2210: esim. koodisto voisi olla parempi source kuin konffitiedosto clientien tiedoille
     if (hasConfigForClient(clientId)) {
-      Right(Unit)
+      Right(clientId)
     } else {
       Left(ValidationError(ValidationErrorType.invalid_client_data, s"${clientId} tuntematon client_id, ei rekisteröity"))
     }
@@ -65,5 +65,6 @@ sealed abstract class ValidationErrorType(val errorType: String)
 
 object ValidationErrorType {
   final case object invalid_client_data extends ValidationErrorType("invalid_client_data")
+  final case object invalid_request extends ValidationErrorType("invalid_request")
 }
 

src/test/scala/fi/oph/koski/omadataoauth2/OmaDataOAuth2Spec.scala

@@ -15,7 +15,7 @@ class OmaDataOAuth2Spec extends AnyFreeSpec with KoskiHttpSpec with Matchers {
   val app = KoskiApplicationForTests
 
   "client-details route" - {
-    val clientDetailsUri = "api/omadata-oauth2/resource-owner/client-details/dvvdigilompakkopk"
+    val clientDetailsUri = "api/omadata-oauth2/resource-owner/client-details/oauth2client"
 
     val hetu = KoskiSpecificMockOppijat.eero.hetu.get
 
@@ -47,7 +47,7 @@ class OmaDataOAuth2Spec extends AnyFreeSpec with KoskiHttpSpec with Matchers {
     val vääräRedirectUri = "/koski/omadata-oauth2/EI-OLE/debug-post-response"
 
     val validParams = Seq(
-      ("client_id", "dvvdigilompakkopk"),
+      ("client_id", "oauth2client"),
       ("response_type", "code"),
       ("response_mode", "form_post"),
       ("redirect_uri", "/koski/omadata-oauth2/debug-post-response"),
@@ -296,6 +296,41 @@ class OmaDataOAuth2Spec extends AnyFreeSpec with KoskiHttpSpec with Matchers {
         verifyResponseStatus(403, KoskiErrorCategory.forbidden.vainOmaDataOAuth2())
       }
     }
+
+    "redirect_uri" - {
+      "vaaditaan, että on sama, jos oli annettu myös autorisointikutsussa" in {
+        // TODO: TOR-2210
+      }
+
+      "ei vaadita, jos ei ollut mukana autorisointikutsussa" in {
+        // TODO: TOR-2210, jos tätä polkua tuetaan
+      }
+    }
+
+    "kun optionaalinen client_id on annettu" - {
+      "voi kutsua, kun client on rekisteröity" in {
+        val user = MockUsers.omadataOAuth2Palvelukäyttäjä
+        postAuthorizationServer(user, clientId = Some(user.username)) {
+          verifyResponseStatusOk()
+        }
+      }
+      "ei voi kutsua, kun clientia ei ole rekisteröity" in {
+        val user = MockUsers.rekisteröimätönOmadataOAuth2Palvelukäyttäjä
+        postAuthorizationServer(user, clientId = Some(user.username)) {
+          // TODO: TOR-2210: oikeat error-koodit ja sisällöt, https://www.rfc-editor.org/rfc/rfc6749#section-5.2
+          verifyResponseStatus(400)
+        }
+      }
+
+      "ei voi kutsua, jos rekisteröity client_id ei vastaa käyttäjätunnusta" in {
+        val user = MockUsers.omadataOAuth2Palvelukäyttäjä
+        val vääräUser = MockUsers.rekisteröimätönOmadataOAuth2Palvelukäyttäjä
+        postAuthorizationServer(user, clientId = Some(vääräUser.username)) {
+          // TODO: TOR-2210: oikeat error-koodit ja sisällöt, https://www.rfc-editor.org/rfc/rfc6749#section-5.2
+          verifyResponseStatus(400)
+        }
+      }
+    }
   }
 
   "resource-server rajapinta" - {
@@ -321,14 +356,19 @@ class OmaDataOAuth2Spec extends AnyFreeSpec with KoskiHttpSpec with Matchers {
   }.mkString("&")
 
 
-  private def postAuthorizationServer[T](user: KoskiMockUser, grantType: String = "authorization_code", code: String = "foobar")(f: => T): T = {
+  private def postAuthorizationServer[T](user: KoskiMockUser, grantType: String = "authorization_code", code: String = "foobar", clientId: Option[String] = None)(f: => T): T = {
     post(uri = "api/omadata-oauth2/authorization-server",
-      body = createFormParametersBody(grantType, code),
+      body = createFormParametersBody(grantType, code, clientId),
       headers = authHeaders(user) ++ formContent)(f)
   }
 
-  private def createFormParametersBody(grantType: String, code: String): Array[Byte] = {
-    s"grant_type=${grantType}&code=${code}".getBytes(StandardCharsets.UTF_8)
+  private def createFormParametersBody(grantType: String, code: String, client_id: Option[String]): Array[Byte] = {
+    val params = Seq(
+      ("grant_type", grantType),
+      ("code", code),
+    ) ++ client_id.toSeq.map(clientId => ("client_id", clientId))
+
+    createParamsString(params).getBytes(StandardCharsets.UTF_8)
   }
 
   private def postResourceServer[T](user: KoskiMockUser, token: String = "dummy-access-token")(f: => T): T = {