Skip to content

Commit

Permalink
Address Ian's feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
spenes committed Aug 9, 2023
1 parent 132999a commit 58e9d0c
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -190,17 +190,12 @@ class CollectorService[F[_]: Sync](
case None =>
cookieConfig.map { config =>
val responseCookie = ResponseCookie(
name = config.name,
content = networkUserId,
expires = Some(HttpDate.unsafeFromEpochSecond((now + config.expiration).toSeconds)),
domain = cookieDomain(headers, config.domains, config.fallbackDomain),
path = Some("/"),
sameSite = config.sameSite.flatMap {
case "Strict" => SameSite.Strict.some
case "Lax" => SameSite.Lax.some
case "None" => SameSite.None.some
case _ => None
},
name = config.name,
content = networkUserId,
expires = Some(HttpDate.unsafeFromEpochSecond((now + config.expiration).toSeconds)),
domain = cookieDomain(headers, config.domains, config.fallbackDomain),
path = Some("/"),
sameSite = config.sameSite,
secure = config.secure,
httpOnly = config.httpOnly
)
Expand All @@ -222,14 +217,15 @@ class CollectorService[F[_]: Sync](
*/
def cookieDomain(
headers: Headers,
domains: Option[List[String]],
domains: List[String],
fallbackDomain: Option[String]
): Option[String] =
(for {
domainList <- domains
originHosts = extractHosts(headers)
domainToUse <- domainList.find(domain => originHosts.exists(validMatch(_, domain)))
} yield domainToUse).orElse(fallbackDomain)
(domains match {
case Nil => None
case _ =>
val originHosts = extractHosts(headers)
domains.find(domain => originHosts.exists(validMatch(_, domain)))
}).orElse(fallbackDomain)

/** Extracts the host names from a list of values in the request's Origin header. */
def extractHosts(headers: Headers): List[String] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package com.snowplowanalytics.snowplow.collectors.scalastream

import scala.concurrent.duration._

import org.http4s.SameSite

import io.circe.Json

object model {
Expand Down Expand Up @@ -33,11 +35,11 @@ object model {
enabled: Boolean,
name: String,
expiration: FiniteDuration,
domains: Option[List[String]],
domains: List[String],
fallbackDomain: Option[String],
secure: Boolean,
httpOnly: Boolean,
sameSite: Option[String]
sameSite: Option[SameSite]
)

final case class CollectorConfig(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ class CollectorServiceSpec extends Specification {
enabled = true,
name = "name",
expiration = 5.seconds,
domains = Some(List("domain")),
domains = List("domain"),
fallbackDomain = None,
secure = false,
httpOnly = false,
Expand Down Expand Up @@ -325,7 +325,7 @@ class CollectorServiceSpec extends Specification {
val conf = testCookieConfig.copy(
secure = true,
httpOnly = true,
sameSite = Some("None")
sameSite = Some(SameSite.None)
)
val Some(`Set-Cookie`(cookie)) =
service.cookieHeader(
Expand Down Expand Up @@ -356,7 +356,7 @@ class CollectorServiceSpec extends Specification {
enabled = true,
name = "name",
expiration = 5.seconds,
domains = None,
domains = List.empty,
fallbackDomain = None,
secure = false,
httpOnly = false,
Expand All @@ -370,7 +370,7 @@ class CollectorServiceSpec extends Specification {
}
"if a list of domains is supplied in the config but the Origin request header is empty and there is no fallback domain" in {
val headers = Headers.empty
val cookieConfig = testCookieConfig.copy(domains = Some(List("domain.com")))
val cookieConfig = testCookieConfig.copy(domains = List("domain.com"))
service.cookieDomain(headers, cookieConfig.domains, cookieConfig.fallbackDomain) shouldEqual None
}
"if none of the domains in the request's Origin header has a match in the list of domains supplied with the config and there is no fallback domain" in {
Expand All @@ -383,7 +383,7 @@ class CollectorServiceSpec extends Specification {
)
val headers = Headers(origin.toRaw1)
val cookieConfig = testCookieConfig.copy(
domains = Some(List("domain.com", "otherdomain.com"))
domains = List("domain.com", "otherdomain.com")
)
service.cookieDomain(headers, cookieConfig.domains, cookieConfig.fallbackDomain) shouldEqual None
}
Expand All @@ -401,7 +401,7 @@ class CollectorServiceSpec extends Specification {
"if the Origin header is empty and a fallback domain is configured" in {
val headers = Headers.empty
val cookieConfig = testCookieConfig.copy(
domains = Some(List("domain.com")),
domains = List("domain.com"),
fallbackDomain = Some("fallbackDomain")
)
service.cookieDomain(headers, cookieConfig.domains, cookieConfig.fallbackDomain) shouldEqual Some(
Expand All @@ -418,7 +418,7 @@ class CollectorServiceSpec extends Specification {
)
val headers = Headers(origin.toRaw1)
val cookieConfig = testCookieConfig.copy(
domains = Some(List("domain.com", "otherdomain.com")),
domains = List("domain.com", "otherdomain.com"),
fallbackDomain = Some("fallbackDomain")
)
service.cookieDomain(headers, cookieConfig.domains, cookieConfig.fallbackDomain) shouldEqual Some(
Expand All @@ -435,7 +435,7 @@ class CollectorServiceSpec extends Specification {
)
val headers = Headers(origin.toRaw1)
val cookieConfig = testCookieConfig.copy(
domains = Some(List("domain.com", "otherdomain.com")),
domains = List("domain.com", "otherdomain.com"),
fallbackDomain = Some("fallbackDomain")
)
service.cookieDomain(headers, cookieConfig.domains, cookieConfig.fallbackDomain) shouldEqual Some(
Expand All @@ -456,7 +456,7 @@ class CollectorServiceSpec extends Specification {
)
val headers = Headers(origin.toRaw1)
val cookieConfig = testCookieConfig.copy(
domains = Some(List("domain.com", "otherdomain.com")),
domains = List("domain.com", "otherdomain.com"),
fallbackDomain = Some("fallbackDomain")
)
service.cookieDomain(headers, cookieConfig.domains, cookieConfig.fallbackDomain) shouldEqual Some(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ object TestUtils {
enabled = true,
name = "sp",
expiration = 365.days,
domains = None,
domains = List.empty,
fallbackDomain = None,
secure = false,
httpOnly = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ object StdoutCollector extends IOApp {
enabled = true,
name = "sp",
expiration = 365.days,
domains = None,
domains = List.empty,
fallbackDomain = None,
secure = false,
httpOnly = false,
Expand Down

0 comments on commit 58e9d0c

Please sign in to comment.