Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AuthorisedAction #13

Merged
merged 5 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions app/auth/AccessScopes.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package auth

import com.gu.identity.auth.AccessScope

object AccessScopes {

case object UserReadSelfSecure extends AccessScope {
val name = "guardian.identity-api.user.read.self.secure"
}
}
40 changes: 40 additions & 0 deletions app/auth/AuthorisedAction.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package auth

import com.gu.identity.auth.*
import play.api.mvc.*
import play.api.mvc.Results.*

import scala.concurrent.{ExecutionContext, Future}

/** An action that requires a valid access token with the given required access scopes. If the token is valid, the
* request is passed to the next action in the chain.
*
* If the request doesn't have an Authorization header or the bearer token in the header isn't a valid access token,
* the action returns a 401 Unauthorized response. If the token is valid but doesn't have the required scopes, the
* action returns a 403 Forbidden response.
*/
class AuthorisedAction(oktaAuthService: OktaAuthService, requiredScopes: List[AccessScope])(implicit
val executionContext: ExecutionContext
) extends ActionBuilder[RequestWithClaims, AnyContent]
with ActionRefiner[Request, RequestWithClaims] {

def parser: BodyParser[AnyContent] = BodyParsers.utils.ignore(AnyContentAsEmpty: AnyContent)

protected def refine[A](request: Request[A]): Future[Either[Result, RequestWithClaims[A]]] = {
Helpers.fetchBearerTokenFromAuthHeader(request.headers.get) match {
case Left(_) => Future.successful(Left(Unauthorized("Request has no Authorization header")))
case Right(token) =>
oktaAuthService
.validateAccessToken(AccessToken(token), requiredScopes)
.redeem(
{
case OktaValidationException(err: ValidationError) =>
Left(new Status(err.suggestedHttpResponseCode)(err.message))
case err => Left(InternalServerError(err.getMessage))
},
claims => Right(RequestWithClaims(claims, request))
)
.unsafeToFuture()
}
}
}
6 changes: 6 additions & 0 deletions app/auth/RequestWithClaims.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package auth

import com.gu.identity.auth.DefaultAccessClaims
import play.api.mvc.{Request, WrappedRequest}

class RequestWithClaims[A](val claims: DefaultAccessClaims, request: Request[A]) extends WrappedRequest[A](request)
15 changes: 15 additions & 0 deletions app/controllers/UserController.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package controllers

import auth.AccessScopes.UserReadSelfSecure
import auth.AuthorisedAction
import com.gu.identity.auth.AccessScope
import play.api.*
import play.api.mvc.*

class UserController(
val controllerComponents: ControllerComponents,
authorisedAction: List[AccessScope] => AuthorisedAction
) extends BaseController {

def me(): Action[AnyContent] = authorisedAction(List(UserReadSelfSecure))(NotImplemented("TODO: implement me"))
}
26 changes: 18 additions & 8 deletions app/load/AppComponents.scala
Original file line number Diff line number Diff line change
@@ -1,21 +1,31 @@
package load

import auth.AuthorisedAction
import com.gu.identity.auth.{OktaAudience, OktaAuthService, OktaIssuerUrl, OktaTokenValidationConfig}
import controllers.{HealthCheckController, UserController}
import logging.RequestLoggingFilter
import play.api.ApplicationLoader.Context
import play.api.BuiltInComponentsFromContext
import play.api.mvc.EssentialFilter
import play.api.{BuiltInComponentsFromContext, Configuration}
import play.filters.HttpFiltersComponents
import router.Routes
import software.amazon.awssdk.regions.Region
import software.amazon.awssdk.services.ssm.SsmClient
import software.amazon.awssdk.services.ssm.model.GetParameterRequest

import scala.util.Using

class AppComponents(context: Context) extends BuiltInComponentsFromContext(context) with HttpFiltersComponents {

override def httpFilters: Seq[EssentialFilter] = super.httpFilters :+ new RequestLoggingFilter(materializer)

lazy val healthCheckController = new controllers.HealthCheckController(controllerComponents)
lazy val router: Routes = new Routes(httpErrorHandler, healthCheckController)
private lazy val authService = OktaAuthService(
OktaTokenValidationConfig(
issuerUrl = OktaIssuerUrl(context.initialConfiguration.get[String]("idProvider.issuer")),
audience = Some(OktaAudience(context.initialConfiguration.get[String]("idProvider.audience"))),
clientId = None,
)
)

private lazy val authorisedAction = new AuthorisedAction(authService, _)

private lazy val healthCheckController = new HealthCheckController(controllerComponents)
private lazy val userController = new UserController(controllerComponents, authorisedAction)

lazy val router: Routes = new Routes(httpErrorHandler, healthCheckController, userController)
}
8 changes: 8 additions & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ lazy val root = (project in file("."))
libraryDependencies ++= Seq(
"net.logstash.logback" % "logstash-logback-encoder" % "7.4",
("com.gu" %% "simple-configuration-ssm" % "1.6.4").cross(CrossVersion.for3Use2_13),
/* Using Scala 2.13 version of identity-auth-play until a Scala 3 version has been released:
* https://trello.com/c/5kOc41kD/4669-release-scala-3-version-of-identity-libraries */
("com.gu.identity" %% "identity-auth-core" % "4.20")
.cross(CrossVersion.for3Use2_13)
exclude ("org.scala-lang.modules", "scala-xml_2.13")
exclude ("org.scala-lang.modules", "scala-parser-combinators_2.13")
exclude ("com.fasterxml.jackson.module", "jackson-module-scala_2.13")
exclude ("com.typesafe", "ssl-config-core_2.13"),
"org.scalatestplus.play" %% "scalatestplus-play" % "7.0.1" % Test,
),
dependencyOverrides ++= Seq(
Expand Down
5 changes: 5 additions & 0 deletions conf/CODE.conf
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
include "application.conf"

idProvider {
issuer = "https://profile.code.dev-theguardian.com/oauth2/aus3v9gla95Toj0EE0x7"
audience = "https://profile.code.dev-theguardian.com/"
}
5 changes: 5 additions & 0 deletions conf/DEV.conf
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
include "application.conf"

idProvider {
issuer = "https://profile.code.dev-theguardian.com/oauth2/aus3v9gla95Toj0EE0x7"
audience = "https://profile.code.dev-theguardian.com/"
}
5 changes: 5 additions & 0 deletions conf/PROD.conf
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
include "application.conf"

idProvider {
issuer = "https://profile.theguardian.com/oauth2/aus3xgj525jYQRowl417"
audience = "https://profile.theguardian.com/"
}
2 changes: 2 additions & 0 deletions conf/routes
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@

+anyhost
GET /healthcheck controllers.HealthCheckController.healthCheck()

GET /user/me controllers.UserController.me()
73 changes: 73 additions & 0 deletions test/auth/AuthorisedActionSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package auth

import cats.effect.IO
import com.gu.identity.auth.*
import org.mockito.Mockito.when
import org.scalatest.matchers.should.Matchers.shouldBe
import org.scalatestplus.mockito.MockitoSugar.mock
import org.scalatestplus.play.*
import play.api.mvc.*
import play.api.mvc.Results.*
import play.api.test.*
import play.api.test.Helpers.*

import scala.concurrent.ExecutionContext.Implicits.global

class AuthorisedActionSpec extends PlaySpec {

"refine" should {

val requiredScopes = List(new AccessScope {
override def name: String = "requiredScope"
})

"return 401 when there is no Authorization header" in {
val authService = mock[OktaAuthService]
val action = new AuthorisedAction(authService, requiredScopes)
val request = FakeRequest(GET, "/")
val result = action(Ok)(request)
status(result) shouldBe UNAUTHORIZED
contentType(result) shouldBe Some("text/plain")
contentAsString(result) shouldBe "Request has no Authorization header"
}

"return 401 when the token is invalid" in {
val authService = mock[OktaAuthService]
when(authService.validateAccessToken(AccessToken("invalidToken"), requiredScopes))
.thenReturn(IO.raiseError(OktaValidationException(InvalidOrExpiredToken)))
val action = new AuthorisedAction(authService, requiredScopes)
val request = FakeRequest(GET, "/").withHeaders("Authorization" -> "Bearer invalidToken")
val result = action(Ok)(request)
status(result) shouldBe UNAUTHORIZED
contentType(result) shouldBe Some("text/plain")
contentAsString(result) shouldBe "Token is invalid or expired"
}

"return 403 when the token is valid but doesn't have the required scopes" in {
val authService = mock[OktaAuthService]
when(authService.validateAccessToken(AccessToken("validToken"), requiredScopes))
.thenReturn(IO.raiseError(OktaValidationException(MissingRequiredScope(requiredScopes))))
val action = new AuthorisedAction(authService, requiredScopes)
val request = FakeRequest(GET, "/").withHeaders("Authorization" -> "Bearer validToken")
val result = action(Ok)(request)
status(result) shouldBe FORBIDDEN
contentType(result) shouldBe Some("text/plain")
contentAsString(result) shouldBe "Token is missing required scope(s): requiredScope"
}

"return 200 when the token is valid and has the required scopes" in {
val requiredScopes = List(new AccessScope {
override def name: String = "requiredScope"
})
val authService = mock[OktaAuthService]
when(authService.validateAccessToken(AccessToken("validToken"), requiredScopes))
.thenReturn(
IO.pure(DefaultAccessClaims(primaryEmailAddress = "a@b.com", identityId = "I43", username = None))
)
val action = new AuthorisedAction(authService, requiredScopes)
val request = FakeRequest(GET, "/").withHeaders("Authorization" -> "Bearer validToken")
val result = action(Ok)(request)
status(result) shouldBe OK
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we missing the case where the token is present but invalid ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 3c943e1

}
13 changes: 7 additions & 6 deletions test/controllers/HealthCheckControllerSpec.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package controllers

import load.AppComponents
import org.scalatest.matchers.should.Matchers.shouldBe
import org.scalatestplus.play.*
import org.scalatestplus.play.components.OneAppPerTestWithComponents
import play.api.BuiltInComponents
Expand All @@ -18,17 +19,17 @@ class HealthCheckControllerSpec extends PlaySpec with OneAppPerTestWithComponent
"run health check from a new instance of controller" in {
val controller = new HealthCheckController(stubControllerComponents())
val healthCheck = controller.healthCheck().apply(FakeRequest(GET, path))
status(healthCheck) mustBe OK
contentType(healthCheck) mustBe Some("text/plain")
contentAsString(healthCheck) must be("OK")
status(healthCheck) shouldBe OK
contentType(healthCheck) shouldBe Some("text/plain")
contentAsString(healthCheck) shouldBe "OK"
}

"run health check from the router" in {
val request = FakeRequest(GET, path)
val healthCheck = route(app, request).get
status(healthCheck) mustBe OK
contentType(healthCheck) mustBe Some("text/plain")
contentAsString(healthCheck) must be("OK")
status(healthCheck) shouldBe OK
contentType(healthCheck) shouldBe Some("text/plain")
contentAsString(healthCheck) shouldBe "OK"
}
}
}
9 changes: 5 additions & 4 deletions test/logging/LogEntrySpec.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package logging

import org.scalatest.matchers.should.Matchers.shouldBe
import org.scalatestplus.play.*
import play.api.mvc.Results.Ok
import play.api.test.*
Expand All @@ -11,10 +12,10 @@ class LogEntrySpec extends PlaySpec {
val request = FakeRequest(GET, "/").withHeaders(REFERER -> "Referrer")
val entry = LogEntry.requestAndResponse(request, response = Ok.withHeaders(CONTENT_LENGTH -> "11"), duration = 7)
"give correct message" in {
entry.message mustBe """127.0.0.1 - "GET / HTTP/1.1" 200 11 "Referrer" 7ms"""
entry.message shouldBe """127.0.0.1 - "GET / HTTP/1.1" 200 11 "Referrer" 7ms"""
}
"give correct fields" in {
entry.otherFields mustBe Map(
entry.otherFields shouldBe Map(
"type" -> "access",
"origin" -> "127.0.0.1",
"referrer" -> "Referrer",
Expand All @@ -32,10 +33,10 @@ class LogEntrySpec extends PlaySpec {
val request = FakeRequest(GET, "/").withHeaders(REFERER -> "Referrer")
val entry = LogEntry.error(request, duration = 17)
"give correct message" in {
entry.message mustBe """127.0.0.1 - "GET / HTTP/1.1" ERROR "Referrer" 17ms"""
entry.message shouldBe """127.0.0.1 - "GET / HTTP/1.1" ERROR "Referrer" 17ms"""
}
"give correct fields" in {
entry.otherFields mustBe Map(
entry.otherFields shouldBe Map(
"type" -> "access",
"origin" -> "127.0.0.1",
"referrer" -> "Referrer",
Expand Down