Skip to content

Commit 4ebc086

Browse files
fix(rbac): Fix cross domain cookie for redirect_to, to work with saml login
## 📝 Description <!-- Describe your changes in detail --> ## ✅ Checklist - [ ] I have tested this change - [ ] This change requires documentation update
1 parent 755c06a commit 4ebc086

File tree

3 files changed

+23
-37
lines changed

3 files changed

+23
-37
lines changed

ee/rbac/lib/rbac/utils/http.ex

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,12 @@ defmodule Rbac.Utils.Http do
55
@state_cookie_options [
66
encrypt: true,
77
max_age: 30 * 60,
8-
# If `same_site` is set to `Strict` then the cookie will not be sent on
9-
# IdP callback redirects, which will break the auth flow.
10-
same_site: "Lax",
8+
same_site: "Strict",
9+
path: "/",
1110
secure: true,
1211
http_only: true
1312
]
1413

15-
def store_redirect_info(conn) do
16-
[
17-
conn.query_params["redirect_path"],
18-
conn.query_params["redirect_to"]
19-
]
20-
|> Enum.filter(fn item -> item not in [nil, ""] end)
21-
|> List.last("")
22-
|> URI.decode()
23-
|> validate_url("")
24-
|> case do
25-
"" ->
26-
conn
27-
28-
url ->
29-
conn |> put_state_value(@redirect_cookie_key, url)
30-
end
31-
end
32-
3314
def fetch_redirect_value(conn, default) do
3415
case conn |> fetch_state_value(@redirect_cookie_key) do
3516
{:ok, redirect_to, _conn} ->
@@ -44,14 +25,6 @@ defmodule Rbac.Utils.Http do
4425
delete_state_value(conn, @redirect_cookie_key)
4526
end
4627

47-
def put_state_value(conn, key, value) do
48-
Logger.debug("Putting state value into cookie for key: #{key}")
49-
50-
value = :erlang.term_to_binary(value)
51-
52-
Plug.Conn.put_resp_cookie(conn, key, value, @state_cookie_options)
53-
end
54-
5528
def delete_state_value(conn, key) do
5629
Logger.debug("Deleting state value from cookie for key: #{key}")
5730

guard/lib/guard/utils.ex

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,8 @@ defmodule Guard.Utils.Http do
137137
@state_cookie_options [
138138
encrypt: true,
139139
max_age: 30 * 60,
140-
# If `same_site` is set to `Strict` then the cookie will not be sent on
141-
# IdP callback redirects, which will break the auth flow.
142-
same_site: "Lax",
140+
same_site: "Strict",
141+
path: "/",
143142
secure: true,
144143
http_only: true
145144
]
@@ -159,9 +158,7 @@ defmodule Guard.Utils.Http do
159158
end
160159
end
161160

162-
def store_redirect_info(conn, url) do
163-
conn |> put_state_value(@redirect_cookie_key, url)
164-
end
161+
def store_redirect_info(conn, url), do: conn |> put_state_value(@redirect_cookie_key, url)
165162

166163
def fetch_redirect_value(conn, default) do
167164
case conn |> fetch_state_value(@redirect_cookie_key) do
@@ -182,7 +179,19 @@ defmodule Guard.Utils.Http do
182179

183180
value = :erlang.term_to_binary(value)
184181

185-
Plug.Conn.put_resp_cookie(conn, key, value, @state_cookie_options)
182+
opts =
183+
if key == @redirect_cookie_key do
184+
# If `same_site` is set to `Strict` then the cookie will not be sent on
185+
# IdP callback redirects, which will break the auth flow.
186+
Keyword.merge(@state_cookie_options,
187+
same_site: "None",
188+
domain: "." <> domain()
189+
)
190+
else
191+
@state_cookie_options
192+
end
193+
194+
Plug.Conn.put_resp_cookie(conn, key, value, opts)
186195
end
187196

188197
def delete_state_value(conn, key) do

guard/test/guard/id/api_test.exs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,11 @@ defmodule Guard.Id.Api.Test do
329329
assert response.status_code == 200
330330

331331
{_, cookie} = List.keyfind(response.headers, "set-cookie", 0)
332+
332333
assert cookie =~ "semaphore_redirect_to="
334+
assert cookie =~ "domain=.localhost"
335+
assert cookie =~ "path=/"
336+
assert cookie =~ "SameSite=None"
333337
end
334338

335339
test "redirect_to query param present, redirects to valid domain only" do
@@ -472,7 +476,7 @@ defmodule Guard.Id.Api.Test do
472476
{_, cookie} = Enum.find(response.headers, fn h -> elem(h, 0) == "set-cookie" end)
473477

474478
assert cookie =~ "semaphore_auth_state="
475-
assert cookie =~ "secure; HttpOnly; SameSite=Lax"
479+
assert cookie =~ "secure; HttpOnly; SameSite=Strict"
476480

477481
assert response.body =~ "/protocol/openid-connect/auth"
478482
assert response.body =~ "localhost"

0 commit comments

Comments
 (0)