Skip to content

Commit

Permalink
[MINOR][BugFix] Duplicate cookies being set
Browse files Browse the repository at this point in the history
Description

Handler is setting multiple cookies because session cookies is missing path and domain

Adds cookie samesite to strict
  • Loading branch information
eliasjpr committed Dec 14, 2022
1 parent 3eade52 commit f64cb4e
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 11 deletions.
2 changes: 1 addition & 1 deletion spec/provider_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe Session::Provider do
end

it "creates a session cookie from current session for #{stores[storage]}" do
cookie = provider.cookie
cookie = provider.cookie("localhost")

cookie.name.should eq provider.session_key
cookie.value.should eq provider.session_id
Expand Down
3 changes: 2 additions & 1 deletion src/handler.cr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ module Session
def call(context : HTTP::Server::Context)
@session.load_from context.request.cookies
call_next(context)
@session.set_cookies context.response.cookies
@session.set_cookies context.response.cookies, context.request.hostname.to_s
context
end
end
end
22 changes: 14 additions & 8 deletions src/provider.cr
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ module Session
self.cookies = request_cookies if self.is_a? CookieStore(T)
cookie = request_cookies[session_key]?
return @current_session = create if cookie.nil?
@current_session = if store_session = self[cookie.not_nil!.value]?
store_session
else
create
end
@current_session = if store_session = self[cookie.not_nil!.value]?
store_session
else
create
end
ensure
on(:loaded, session_id, data)
end
Expand All @@ -48,14 +48,17 @@ module Session
on :started, session_id, current_session.data
end

def set_cookies(response_cookies : HTTP::Cookies)
response_cookies << cookie
def set_cookies(response_cookies : HTTP::Cookies, host : String = "")
response_cookies << cookie(host)
if self.is_a? CookieStore(T)
response_cookies << HTTP::Cookie.new(
name: prefixed(self.cookie_name + session_id),
value: encrypt_and_sign(current_session.to_json),
expires: timeout.from_now,
secure: true,
domain: host,
path: "/",
samesite: HTTP::Cookie::SameSite::Strict,
http_only: true,
creation_time: Time.local,
)
Expand All @@ -65,13 +68,16 @@ module Session
end

# Creates the session cookie
def cookie
def cookie(host : String)
HTTP::Cookie.new(
name: session_key,
value: session_id,
expires: timeout.from_now,
secure: true,
http_only: true,
domain: host,
path: "/",
samesite: HTTP::Cookie::SameSite::Strict,
creation_time: Time.local,
)
ensure
Expand Down
2 changes: 1 addition & 1 deletion src/store.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Session

abstract def [](key : String) : SessionId(T)
abstract def []?(key : String) : SessionId(T)?
abstract def []=(key : String, session_id : SessionId(T)) : SessionId(T)
abstract def []=(key : String, session : SessionId(T)) : SessionId(T)
abstract def delete(key : String)
abstract def size : Int64
abstract def clear
Expand Down

0 comments on commit f64cb4e

Please sign in to comment.