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.

Sign up for GitHub

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

Fix matcher handling Play 2.8.x backport #730

Merged
merged 2 commits into from
Dec 3, 2024
Merged

Fix matcher handling Play 2.8.x backport #730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a17d96
Fix matcher handling
reardonj Nov 30, 2024
f7cf2c6
fix scala 2.12 compat issues
reardonj Dec 3, 2024
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
82 changes: 51 additions & 31 deletions shared/src/main/scala/org/pac4j/play/filters/SecurityFilter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import javax.inject.{Inject, Singleton}
import scala.compat.java8.FutureConverters.CompletionStageOps
import scala.concurrent.{ExecutionContext, Future}
import scala.util.Failure
import scala.util.matching.Regex

/**
* Filter on all requests to apply security by the Pac4J framework.
Expand Down Expand Up @@ -67,55 +68,62 @@ class SecurityFilter @Inject()(configuration: Configuration, config: Config)

override def apply(nextFilter: RequestHeader => Future[play.api.mvc.Result])
(request: RequestHeader): Future[play.api.mvc.Result] = {
findRule(request).flatMap(_.data) match {
case Some(rule) =>
findRule(request).map(_.data) match {
case Some(rule :: remainingRules) =>
log.debug(s"Authentication needed for ${request.uri}")
proceedRuleLogic(nextFilter, request, rule)
proceedRuleLogic(nextFilter, request, rule, remainingRules)

case None =>
case _ =>
log.debug(s"No authentication needed for ${request.uri}")
nextFilter(request)
}
}

private def proceedRuleLogic(nextFilter: RequestHeader => Future[Result], request: RequestHeader, rule: RuleData): Future[Result] = {
private def proceedRuleLogic(
nextFilter: RequestHeader => Future[Result],
request: RequestHeader,
rule: RuleData,
remainingRules: Seq[RuleData]
): Future[Result] = {

FrameworkAdapter.INSTANCE.applyDefaultSettingsIfUndefined(config)

val parameters = new PlayFrameworkParameters(request)
val webContext = config.getWebContextFactory().newContext(parameters).asInstanceOf[PlayWebContext]
val securityAction = new SecureAction(config)

def calculateResult(secureActionResult: mvc.Result): Future[Result] = {
if (secureActionResult.isInstanceOf[PlayWebContextResultHolder]) {
val newCtx = secureActionResult.asInstanceOf[PlayWebContextResultHolder].getPlayWebContext
val newRequest = newCtx.supplementRequest(request.asJava).asScala
nextFilter(newRequest)
} else {
// When the user is not authenticated, the result is one of the following:
// - forbidden
// - redirect to IDP
// - unauthorized
// Or the future results in an exception
Future {
log.info(s"Authentication failed for ${request.uri} with clients ${rule.clients} and authorizers ${rule.authorizers} and matchers ${rule.matchers}. Authentication response code ${secureActionResult.status}.")
secureActionResult.asScala
}
}
}

val futureResult: Future[Result] =
def checkSecurity(request: RequestHeader, rule: RuleData, remainingRules: Seq[RuleData]): Future[Result] =
securityAction
.call(parameters, rule.clients, rule.authorizers, rule.matchers)
.call(parameters, rule.clients, rule.authorizers, rule.matchers)
.toScala
.flatMap[Result](calculateResult)
.flatMap { secureActionResult =>
if (secureActionResult.isInstanceOf[PlayWebContextResultHolder]) {
val newCtx = secureActionResult.asInstanceOf[PlayWebContextResultHolder].getPlayWebContext
val newRequest = newCtx.supplementRequest(request.asJava).asScala

remainingRules match {
case Nil => nextFilter(newRequest)
case head :: tail => checkSecurity(newRequest, head, tail)
}
} else {
// When the user is not authenticated, the result is one of the following:
// - forbidden
// - redirect to IDP
// - unauthorized
// Or the future results in an exception
Future.successful {
log.info(s"Authentication failed for ${request.uri} with clients ${rule.clients} and authorizers ${rule.authorizers} and matchers ${rule.matchers}. Authentication response code ${secureActionResult.status}.")
secureActionResult.asScala
}
}
}

futureResult.andThen { case Failure(ex) => log.error("Exception during authentication procedure", ex) }
checkSecurity(request, rule, remainingRules).andThen { case Failure(ex) => log.error("Exception during authentication procedure", ex) }
}

private def findRule(request: RequestHeader): Option[Rule] = {
val pathNormalized = getNormalizedPath(request)
rules.find(rule => pathNormalized.matches(rule.pathRegex))
rules.find(rule => rule.compiledRegex.pattern.matcher(pathNormalized).matches)
}

private def getNormalizedPath(request: RequestHeader): String = {
Expand All @@ -132,13 +140,25 @@ class SecurityFilter @Inject()(configuration: Configuration, config: Config)
}

object SecurityFilter {
private[filters] case class Rule(pathRegex: String, data: Option[RuleData])
private[filters] case class Rule(pathRegex: String, data: List[RuleData]) {
val compiledRegex = pathRegex.r

def mergeData(other: Rule) = this.copy(data = this.data ++ other.data)
}
private[filters] case class RuleData(clients: String, authorizers: String, matchers: String)

private[filters]
def loadRules(configuration: Configuration): Seq[Rule] = {
val ruleConfigs = configuration.getOptional[Seq[Configuration]]("pac4j.security.rules").getOrElse(Seq())
ruleConfigs.map(convertConfToRule)
ruleConfigs
.map(convertConfToRule)
// coalesce adjacent rules with the exact same path
.foldLeft(List.empty[Rule]) {
case (Nil, rule) => List(rule)
case (head :: tail, rule) if head.pathRegex == rule.pathRegex => head.mergeData(rule) :: tail
case (list, rule) => rule :: list
}
.reverse
}

private def convertConfToRule(conf: Configuration): Rule = {
Expand All @@ -158,6 +178,6 @@ object SecurityFilter {
}
}

Rule(path.replace("\"", ""), ruleData)
Rule(path.replace("\"", ""), ruleData.toList)
}
}
72 changes: 59 additions & 13 deletions shared/src/test/java/org/pac4j/play/filters/SecurityFilterTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ class SecurityFilterTests extends ScalaFutures with Results {
val config: Configuration = new Configuration(ConfigFactory.load("config/security_filter.conf"))

SecurityFilter.loadRules(config) shouldBe Seq(
Rule("/path_anonymous", Some(RuleData("AnonymousClient", null, null))),
Rule("/path_secure_1", Some(RuleData("client1,client2", null, null))),
Rule("/path_secure_3", Some(RuleData(null, "authorizer1,authorizer2", null))),
Rule("/path_secure_4", Some(RuleData("client1,client2", "authorizer1,authorizer2", "matcher1,matcher2")))
Rule("/path_anonymous", List(RuleData("AnonymousClient", null, null))),
Rule("/path_secure_1", List(RuleData("client1,client2", null, null))),
Rule("/path_secure_3", List(RuleData(null, "authorizer1,authorizer2", null))),
Rule("/path_secure_4", List(RuleData("client1,client2", "authorizer1,authorizer2", "matcher1,matcher2")))
)
}

Expand Down Expand Up @@ -69,19 +69,65 @@ class SecurityFilterTests extends ScalaFutures with Results {
""".stripMargin
)

def tryFilterApply(path: String): Future[Result] = {
val nextFilter = (_: RequestHeader) => Future.successful(Ok("ok"))
val testRequest: RequestHeader = FakeRequest(POST, path)
securityFilter.apply(nextFilter)(testRequest)
}
status(tryFilterApply(securityFilter, "/path_secure", POST)) shouldBe 401
status(tryFilterApply(securityFilter, "/path_secure_2/any_path_", POST)) shouldBe 401

status(tryFilterApply("/path_secure")) shouldBe 401
status(tryFilterApply("/path_secure_2/any_path_")) shouldBe 401
status(tryFilterApply(securityFilter, "/path_anonymous", POST)) shouldBe 200
status(tryFilterApply(securityFilter, "any/other/path", POST)) shouldBe 200
}

status(tryFilterApply("/path_anonymous")) shouldBe 200
status(tryFilterApply("any/other/path")) shouldBe 200
@Test
def testThatSecurityFilterBlocksUnauthorizedRequestsWithMatchers(): Unit = {
implicit val ec = scala.concurrent.ExecutionContext.global
implicit val as = ActorSystem("text-actor-system")
implicit val mat: ActorMaterializer = ActorMaterializer()

val securityFilter = prepareSecurityFilter(
"""
|pac4j.security.rules = [
| {
| "/path_secure" = {
| "clients" = "AnonymousClient"
| "authorizers" = "none"
| "matchers" = "put"
| }
| },
| {
| "/path_secure" = {
| "clients" = "AnonymousClient"
| "authorizers" = "none"
| "matchers" = "get"
| }
| }, {
| "/path_secure" = {
| clients = "client1"
| "matchers" = "post"
| }
| }, {
| "/path_secure" = {
| "clients" = "AnonymousClient"
| }
| }, {
| "/path_secure/deeper" = {
| clients = "client1"
| }
| }
|]
""".stripMargin
)

status(tryFilterApply(securityFilter, "/path_secure", GET)) shouldBe 200
status(tryFilterApply(securityFilter, "/path_secure", POST)) shouldBe 401
status(tryFilterApply(securityFilter, "/path_secure/deeper", GET)) shouldBe 401
status(tryFilterApply(securityFilter, "/path_secure/deeper", POST)) shouldBe 401
}

private def tryFilterApply(securityFilter: SecurityFilter, path: String, method: String): Future[Result] = {
val nextFilter = (_: RequestHeader) => Future.successful(Ok("ok"))
val testRequest: RequestHeader = FakeRequest(method, path)
securityFilter.apply(nextFilter)(testRequest)
}

private def prepareSecurityFilter(configString: String)
(implicit ec: ExecutionContext, mat: Materializer): SecurityFilter = {
val pac4jConfig = new Config
Expand Down
Loading