|
1
|
+ |
defmodule OpenAgentsWeb.DeviceSignInReturnTest do
|
|
2
|
+ |
@moduledoc """
|
|
3
|
+ |
Issue #129: signing in from the terminal was two logins wearing one name.
|
|
4
|
+ |
|
|
5
|
+ |
A reader who is not signed in and opens the link their terminal printed —
|
|
6
|
+ |
`/device?user_code=…` — used to be bounced to the public root, where nothing
|
|
7
|
+ |
said why they were there. Signing in put them on the dashboard, and the
|
|
8
|
+ |
approval they had actually come for was an errand still to run, with the code
|
|
9
|
+ |
back in the terminal they had left.
|
|
10
|
+ |
|
|
11
|
+ |
This proves the return path: the bounce remembers the code, the sign-in
|
|
12
|
+ |
carries it across the OAuth round trip, and the reader lands back on the
|
|
13
|
+ |
approval with the code already in hand.
|
|
14
|
+ |
|
|
15
|
+ |
The adversarial half matters as much as the working one. The value that
|
|
16
|
+ |
decides where a sign-in lands arrives in a URL that anyone can write, and it
|
|
17
|
+ |
is printed back onto a page. So the assertions here are not "a code survives"
|
|
18
|
+ |
but "only a code survives": every crafted `?user_code=` must leave the reader
|
|
19
|
+ |
exactly where the old behavior left them, on the public root with nothing
|
|
20
|
+ |
remembered, and must never reach the session, the redirect, or the page.
|
|
21
|
+ |
"""
|
|
22
|
+ |
|
|
23
|
+ |
use OpenAgentsWeb.ConnCase, async: false
|
|
24
|
+ |
|
|
25
|
+ |
import Phoenix.LiveViewTest
|
|
26
|
+ |
|
|
27
|
+ |
alias OpenAgents.DeviceAuthorizations
|
|
28
|
+ |
alias OpenAgentsWeb.UserAuth
|
|
29
|
+ |
|
|
30
|
+ |
setup {Req.Test, :verify_on_exit!}
|
|
31
|
+ |
|
|
32
|
+ |
setup do
|
|
33
|
+ |
original = Application.fetch_env!(:openagents, :github_oauth)
|
|
34
|
+ |
|
|
35
|
+ |
Application.put_env(
|
|
36
|
+ |
:openagents,
|
|
37
|
+ |
:github_oauth,
|
|
38
|
+ |
Keyword.put(original, :request_options, plug: {Req.Test, __MODULE__})
|
|
39
|
+ |
)
|
|
40
|
+ |
|
|
41
|
+ |
on_exit(fn -> Application.put_env(:openagents, :github_oauth, original) end)
|
|
42
|
+ |
:ok
|
|
43
|
+ |
end
|
|
44
|
+ |
|
|
45
|
+ |
test "the sign-in a device code sends a reader through returns them to the approval", %{
|
|
46
|
+ |
conn: conn
|
|
47
|
+ |
} do
|
|
48
|
+ |
{:ok, _authorization, _device_code, user_code} = DeviceAuthorizations.create()
|
|
49
|
+ |
|
|
50
|
+ |
# 1. The terminal's link, opened by a browser with no session.
|
|
51
|
+ |
bounced = get(conn, ~p"/device?user_code=#{user_code}")
|
|
52
|
+ |
|
|
53
|
+ |
assert redirected_to(bounced) == "/?user_code=#{user_code}"
|
|
54
|
+ |
assert get_session(bounced, UserAuth.device_session_key()) == user_code
|
|
55
|
+ |
|
|
56
|
+ |
# 2. The page they land on says what the sign-in is for and shows the code,
|
|
57
|
+ |
# rather than presenting itself as a homepage.
|
|
58
|
+ |
landing = bounced |> recycle() |> get(~p"/?user_code=#{user_code}")
|
|
59
|
+ |
landing_html = html_response(landing, 200)
|
|
60
|
+ |
|
|
61
|
+ |
assert landing_html =~ ~s(id="device-sign-in")
|
|
62
|
+ |
assert landing_html =~ user_code
|
|
63
|
+ |
assert landing_html =~ "authorize your terminal"
|
|
64
|
+ |
|
|
65
|
+ |
# 3. One ordinary sign-in. Nothing about it names the device: the code
|
|
66
|
+ |
# rides in the session, so any sign-in control on the page returns them.
|
|
67
|
+ |
started =
|
|
68
|
+ |
landing
|
|
69
|
+ |
|> recycle()
|
|
70
|
+ |
|> put_req_header("x-csrf-token", Plug.CSRFProtection.get_csrf_token())
|
|
71
|
+ |
|> post(~p"/auth/github?github_tools=enabled")
|
|
72
|
+ |
|
|
73
|
+ |
state = oauth_state(started)
|
|
74
|
+ |
expect_github(4_129, "device-return-person")
|
|
75
|
+ |
|
|
76
|
+ |
authenticated =
|
|
77
|
+ |
started
|
|
78
|
+ |
|> recycle()
|
|
79
|
+ |
|> get(~p"/auth/github/callback?code=valid-code&state=#{state}")
|
|
80
|
+ |
|
|
81
|
+ |
# 4. The sign-in returns them to the approval, code in hand -- not to the
|
|
82
|
+ |
# dashboard with the errand still to run.
|
|
83
|
+ |
assert redirected_to(authenticated) == "/device?user_code=#{user_code}"
|
|
84
|
+ |
assert get_session(authenticated, "user_id")
|
|
85
|
+ |
|
|
86
|
+ |
# The code has done its work and does not linger to redirect a later
|
|
87
|
+ |
# sign-in somewhere the reader did not ask to go.
|
|
88
|
+ |
assert get_session(authenticated, UserAuth.device_session_key()) == nil
|
|
89
|
+ |
|
|
90
|
+ |
# 5. Approving is the next click. The code is already matched, so the
|
|
91
|
+ |
# review is on screen without anything being retyped.
|
|
92
|
+ |
signed_in = recycle(authenticated)
|
|
93
|
+ |
|
|
94
|
+ |
{:ok, view, _html} = live(signed_in, "/device?user_code=#{user_code}")
|
|
95
|
+ |
|
|
96
|
+ |
assert has_element?(view, "#device-authorization-review")
|
|
97
|
+ |
assert has_element?(view, "#approve-device")
|
|
98
|
+ |
refute has_element?(view, "#device-code-invalid")
|
|
99
|
+ |
|
|
100
|
+ |
view |> element("#approve-device") |> render_click()
|
|
101
|
+ |
|
|
102
|
+ |
assert has_element?(view, "#device-approved")
|
|
103
|
+ |
end
|
|
104
|
+ |
|
|
105
|
+ |
test "a sign-in that did not start at the device page still lands on the dashboard", %{
|
|
106
|
+ |
conn: conn
|
|
107
|
+ |
} do
|
|
108
|
+ |
started =
|
|
109
|
+ |
conn
|
|
110
|
+ |
|> init_test_session(%{})
|
|
111
|
+ |
|> put_req_header("x-csrf-token", Plug.CSRFProtection.get_csrf_token())
|
|
112
|
+ |
|> post(~p"/auth/github?github_tools=enabled")
|
|
113
|
+ |
|
|
114
|
+ |
state = oauth_state(started)
|
|
115
|
+ |
expect_github(4_130, "ordinary-sign-in-person")
|
|
116
|
+ |
|
|
117
|
+ |
authenticated =
|
|
118
|
+ |
started
|
|
119
|
+ |
|> recycle()
|
|
120
|
+ |
|> get(~p"/auth/github/callback?code=valid-code&state=#{state}")
|
|
121
|
+ |
|
|
122
|
+ |
assert redirected_to(authenticated) == ~p"/sarah"
|
|
123
|
+ |
end
|
|
124
|
+ |
|
|
125
|
+ |
test "the device page without a code refuses exactly as it always did", %{conn: conn} do
|
|
126
|
+ |
bounced = get(conn, ~p"/device")
|
|
127
|
+ |
|
|
128
|
+ |
assert redirected_to(bounced) == "/"
|
|
129
|
+ |
assert get_session(bounced, UserAuth.device_session_key()) == nil
|
|
130
|
+ |
end
|
|
131
|
+ |
|
|
132
|
+ |
# Everything a link can carry that is not a code this application minted. The
|
|
133
|
+ |
# value is a redirect target and page content, so each one has to be refused
|
|
134
|
+ |
# before it becomes either.
|
|
135
|
+ |
#
|
|
136
|
+ |
# `user_code[]` is here because a query string can produce a list rather than
|
|
137
|
+ |
# a string, and a cast that only guarded binaries would raise on it.
|
|
138
|
+ |
@crafted [
|
|
139
|
+ |
{"an absolute URL", "https://evil.example/steal"},
|
|
140
|
+ |
{"a scheme-relative URL", "//evil.example/steal"},
|
|
141
|
+ |
{"a path traversal", "/../../admin"},
|
|
142
|
+ |
{"another path on this host", "/settings/api-tokens"},
|
|
143
|
+ |
{"markup", "<script>alert(1)</script>"},
|
|
144
|
+ |
{"a quote breaking an attribute", ~s(ABCD-EFGH" onload=")},
|
|
145
|
+ |
{"a header injection", "ABCD-EFGH\r\nSet-Cookie: user_id=1"},
|
|
146
|
+ |
{"a second line", "ABCD-EFGH\nADCD-EFGH"},
|
|
147
|
+ |
{"an appended query", "ABCD-EFGH?next=/admin"},
|
|
148
|
+ |
{"an appended fragment", "ABCD-EFGH#/admin"},
|
|
149
|
+ |
{"a code too long", "ABCD-EFGHJ"},
|
|
150
|
+ |
{"a code too short", "ABC-EFGH"},
|
|
151
|
+ |
{"the wrong separator", "ABCD_EFGH"},
|
|
152
|
+ |
{"no separator", "ABCDEFGH"},
|
|
153
|
+ |
{"characters the alphabet excludes", "IOL1-0OI1"},
|
|
154
|
+ |
{"an empty value", ""},
|
|
155
|
+ |
{"only whitespace", " "},
|
|
156
|
+ |
{"a list rather than a string", ["ABCD-EFGH"]}
|
|
157
|
+ |
]
|
|
158
|
+ |
|
|
159
|
+ |
for {what, crafted} <- @crafted do
|
|
160
|
+ |
test "a crafted user_code -- #{what} -- carries nothing across the sign-in", %{conn: conn} do
|
|
161
|
+ |
crafted = unquote(Macro.escape(crafted))
|
|
162
|
+ |
|
|
163
|
+ |
bounced = get(conn, device_path(crafted))
|
|
164
|
+ |
|
|
165
|
+ |
assert redirected_to(bounced) == "/",
|
|
166
|
+ |
"`#{inspect(crafted)}` decided where the reader went."
|
|
167
|
+ |
|
|
168
|
+ |
assert get_session(bounced, UserAuth.device_session_key()) == nil,
|
|
169
|
+ |
"`#{inspect(crafted)}` was remembered across the sign-in."
|
|
170
|
+ |
|
|
171
|
+ |
# And it cannot get in the back way either: even carried all the way to a
|
|
172
|
+ |
# completed sign-in, it does not become a landing path.
|
|
173
|
+ |
started =
|
|
174
|
+ |
bounced
|
|
175
|
+ |
|> recycle()
|
|
176
|
+ |
|> put_req_header("x-csrf-token", Plug.CSRFProtection.get_csrf_token())
|
|
177
|
+ |
|> post(~p"/auth/github?github_tools=enabled")
|
|
178
|
+ |
|
|
179
|
+ |
state = oauth_state(started)
|
|
180
|
+ |
expect_github(4_131, "crafted-code-person")
|
|
181
|
+ |
|
|
182
|
+ |
authenticated =
|
|
183
|
+ |
started
|
|
184
|
+ |
|> recycle()
|
|
185
|
+ |
|> get(~p"/auth/github/callback?code=valid-code&state=#{state}")
|
|
186
|
+ |
|
|
187
|
+ |
assert redirected_to(authenticated) == ~p"/sarah"
|
|
188
|
+ |
end
|
|
189
|
+ |
end
|
|
190
|
+ |
|
|
191
|
+ |
# The bounce and the landing both go through `cast_user_code/1`, so this pins
|
|
192
|
+ |
# what it admits directly rather than only through the routes that use it.
|
|
193
|
+ |
test "only the shape this application mints casts" do
|
|
194
|
+ |
{:ok, _authorization, _device_code, minted} = DeviceAuthorizations.create()
|
|
195
|
+ |
|
|
196
|
+ |
assert DeviceAuthorizations.cast_user_code(minted) == {:ok, minted}
|
|
197
|
+ |
|
|
198
|
+ |
# A code read off one screen and typed into another arrives however the
|
|
199
|
+ |
# reader typed it.
|
|
200
|
+ |
assert DeviceAuthorizations.cast_user_code(String.downcase(minted)) == {:ok, minted}
|
|
201
|
+ |
assert DeviceAuthorizations.cast_user_code(" " <> minted <> " ") == {:ok, minted}
|
|
202
|
+ |
|
|
203
|
+ |
for {_what, crafted} <- @crafted do
|
|
204
|
+ |
assert DeviceAuthorizations.cast_user_code(crafted) == :error,
|
|
205
|
+ |
"`#{inspect(crafted)}` cast as a user code."
|
|
206
|
+ |
end
|
|
207
|
+ |
|
|
208
|
+ |
assert DeviceAuthorizations.cast_user_code(nil) == :error
|
|
209
|
+ |
assert DeviceAuthorizations.cast_user_code(%{"user_code" => "ABCD-EFGH"}) == :error
|
|
210
|
+ |
end
|
|
211
|
+ |
|
|
212
|
+ |
# Built by hand rather than with `~p`, because half the point is that these
|
|
213
|
+ |
# are query strings a verified route would refuse to construct. Percent
|
|
214
|
+ |
# encoding is what a browser does, and Plug decodes it back before anything
|
|
215
|
+ |
# here sees it, so `%0D%0A` reaches the cast as a real CRLF.
|
|
216
|
+ |
defp device_path(crafted) when is_list(crafted),
|
|
217
|
+ |
do: "/device?" <> URI.encode_query(Enum.map(crafted, &{"user_code[]", &1}))
|
|
218
|
+ |
|
|
219
|
+ |
defp device_path(crafted), do: "/device?" <> URI.encode_query(%{"user_code" => crafted})
|
|
220
|
+ |
|
|
221
|
+ |
defp oauth_state(conn) do
|
|
222
|
+ |
conn
|
|
223
|
+ |
|> redirected_to()
|
|
224
|
+ |
|> URI.parse()
|
|
225
|
+ |
|> Map.fetch!(:query)
|
|
226
|
+ |
|> URI.decode_query()
|
|
227
|
+ |
|> Map.fetch!("state")
|
|
228
|
+ |
end
|
|
229
|
+ |
|
|
230
|
+ |
defp expect_github(github_id, login) do
|
|
231
|
+ |
Req.Test.expect(__MODULE__, fn conn ->
|
|
232
|
+ |
Req.Test.json(conn, %{
|
|
233
|
+ |
"access_token" => "ephemeral-github-token",
|
|
234
|
+ |
"scope" => "repo,read:org"
|
|
235
|
+ |
})
|
|
236
|
+ |
end)
|
|
237
|
+ |
|
|
238
|
+ |
Req.Test.expect(__MODULE__, fn conn ->
|
|
239
|
+ |
Req.Test.json(conn, %{
|
|
240
|
+ |
"id" => github_id,
|
|
241
|
+ |
"login" => login,
|
|
242
|
+ |
"avatar_url" => "https://avatars.githubusercontent.com/u/#{github_id}?v=4"
|
|
243
|
+ |
})
|
|
244
|
+ |
end)
|
|
245
|
+ |
end
|
|
246
|
+ |
end
|