Skip to content

Commit

Permalink
STAC-17136. Fix problem sessionId was not always validated if it was …
Browse files Browse the repository at this point in the history
…in the store. Swap the order of when the CSRF cookie was added in login callback to gurantee a valid sessionId is returned during login process.
  • Loading branch information
CMGRocha committed Aug 3, 2022
1 parent 69ead0b commit 32cdd42
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/main/scala/com/stackstate/pac4j/AkkaHttpSecurity.scala
Expand Up @@ -180,8 +180,8 @@ class AkkaHttpSecurity(config: Config, sessionStorage: SessionStorage, val sessi
_ =>
callbackLogic.perform(akkaWebContext, config, actionAdapter, defaultUrl, saveInSession, multiProfile, true, defaultClient.orNull).map {
result =>
akkaWebContext.addResponseSessionCookie()
if (setCsrfCookie) akkaWebContext.addResponseCsrfCookie()
akkaWebContext.addResponseSessionCookie()
result
}
}
Expand Down
Expand Up @@ -40,6 +40,7 @@ class AkkaHttpWebContext(val request: HttpRequest,
.find(_.name == sessionCookieName)
.map(_.value)
.filter(_.nonEmpty)
.find(session => sessionStorage.sessionExists(session))

def getOrCreateSessionId(): String = {
val newSession =
Expand Down
66 changes: 66 additions & 0 deletions src/test/scala/com/stackstate/pac4j/AkkaHttpSecurityTest.scala
Expand Up @@ -264,6 +264,72 @@ class AkkaHttpSecurityTest extends AnyWordSpecLike with Matchers with ScalatestR
responseAs[String] shouldBe "called!"
}
}


"run the callbackLogic should not send back a sessionId if the set csrf cookie is false" in {
val config = new Config()
val existingContext = AkkaHttpWebContext(HttpRequest(), Seq.empty, new ForgetfulSessionStorage, AkkaHttpWebContext.DEFAULT_COOKIE_NAME)

val actionAdapter = new HttpActionAdapter[HttpResponse, AkkaHttpWebContext] {
override def adapt(code: HttpAction, context: AkkaHttpWebContext): HttpResponse = ???
}
config.setHttpActionAdapter(actionAdapter)
config.setCallbackLogic(new AkkaHttpCallbackLogic {
override def perform(context: AkkaHttpWebContext,
config: Config,
httpActionAdapter: HttpActionAdapter[Future[RouteResult], AkkaHttpWebContext],
defaultUrl: String,
saveInSession: lang.Boolean,
multiProfile: lang.Boolean,
renewSession: lang.Boolean,
client: String): Future[RouteResult] = {
Future.successful(Complete(HttpResponse(StatusCodes.OK, entity = "called!")))
}
})

val akkaHttpSecurity = new AkkaHttpSecurity(config, new ForgetfulSessionStorage)

Get("/") ~> akkaHttpSecurity
.callback("/blaat", saveInSession = false, multiProfile = false, Some("Yooo"), existingContext = Some(existingContext), setCsrfCookie = false) ~> check {
// Session Store is empty so `addResponseSessionCookie` will create a token that will expire immediately
header("Set-Cookie").get.value().contains("AkkaHttpPac4jSession=; Max-Age=0;") shouldBe true

}
}

"run the callbackLogic should send back a sessionId if the csrf cookie is true" in {
val config = new Config()
val existingContext = AkkaHttpWebContext(HttpRequest(), Seq.empty, new InMemorySessionStorage(3.minutes), AkkaHttpWebContext.DEFAULT_COOKIE_NAME)

val actionAdapter = new HttpActionAdapter[HttpResponse, AkkaHttpWebContext] {
override def adapt(code: HttpAction, context: AkkaHttpWebContext): HttpResponse = ???
}
config.setHttpActionAdapter(actionAdapter)
config.setCallbackLogic(new AkkaHttpCallbackLogic {
override def perform(context: AkkaHttpWebContext,
config: Config,
httpActionAdapter: HttpActionAdapter[Future[RouteResult], AkkaHttpWebContext],
defaultUrl: String,
saveInSession: lang.Boolean,
multiProfile: lang.Boolean,
renewSession: lang.Boolean,
client: String): Future[RouteResult] = {
Future.successful(Complete(HttpResponse(StatusCodes.OK, entity = "called!")))
}
})

val akkaHttpSecurity = new AkkaHttpSecurity(config, new InMemorySessionStorage(3.minutes))
Get("/") ~> akkaHttpSecurity
.callback("/blaat", saveInSession = false, multiProfile = false, Some("Yooo"), existingContext = Some(existingContext), setCsrfCookie = true) ~> check {
headers.size shouldBe 2
val localHeaders: Seq[HttpHeader] = headers
val threeMinutesInSeconds = 180
// When `addResponseCsrfCookie` is called the method `getOrCreateSessionId` is called which creates a Session
// when `addResponseSessionCookie` is called there is already a session so a cookie with value is added.
localHeaders.find(_.value().contains("pac4jCsrfToken")).get.value().contains(s"Max-Age=$threeMinutesInSeconds;") shouldBe true
localHeaders.find(_.value().contains("AkkaHttpPac4jSession")).get.value().contains(s"Max-Age=$threeMinutesInSeconds;") shouldBe true
}
}
}

"AkkaHttpSecurity.authorize" should {
Expand Down
22 changes: 22 additions & 0 deletions src/test/scala/com/stackstate/pac4j/AkkaHttpWebContextTest.scala
Expand Up @@ -266,6 +266,28 @@ class AkkaHttpWebContextTest extends AnyWordSpecLike with Matchers {

webContext.getChanges.cookies shouldBe List(validCookie)
}

"when getSessionId is called and the session exists in the store should return it" in withContext(
cookies = List(Cookie(AkkaHttpWebContext.DEFAULT_COOKIE_NAME, "validId")),
sessionStorage = new ForgetfulSessionStorage {
override val sessionLifetime = 3.seconds

override def sessionExists(sessionKey: SessionKey): Boolean = true
}
) { webContext =>
webContext.getSessionId shouldBe Some("validId")
}

"when getSessionId is called and the session doesn't exists in the store should return None" in withContext(
cookies = List(Cookie(AkkaHttpWebContext.DEFAULT_COOKIE_NAME, "notValidAnyMore")),
sessionStorage = new ForgetfulSessionStorage {
override val sessionLifetime = 3.seconds

override def sessionExists(sessionKey: SessionKey): Boolean = false
}
) { webContext =>
webContext.getSessionId shouldBe None
}
}

def withContext(requestHeaders: List[(String, String)] = List.empty,
Expand Down

0 comments on commit 32cdd42

Please sign in to comment.