Return a device sign-in to the approval it came from

c67cf3943fa4 · AtlantisPleb · · parent c303a37ecae9

Return a device sign-in to the approval it came from

Opening the link a terminal prints -- /device?user_code=... -- without a
session bounced the reader to the public root, which said nothing about
why they were there. Signing in put them on the dashboard, and the
approval they had actually come for was a second errand with the code
back in the terminal they had left. One intent, two logins, nothing on
screen connecting them.

The bounce now remembers the code in the browser session, and the OAuth
callback reads it back and returns the reader to /device with the code
already matched, so approving is the next click. Carrying it in the
session rather than in the sign-in form means every sign-in control
returns them -- the landing page's and the command bar's alike --
instead of only the one that was threaded with the code. The parameter
still rides in the URL, but only so the landing page can say what the
sign-in is for; the session is the authority.

The value decides a redirect and is printed onto a page, so exactly one
function admits it. DeviceAuthorizations.cast_user_code/1 returns
{:ok, code} only for the eight-character alphabet this application
mints, anchored so no second line can follow, and every place that puts
it into a URL, a session, or a page casts it rather than trusting an
earlier cast. A host, a scheme-relative URL, a path, a CRLF, markup, or
a query-string list leaves the reader on the bare public root with
nothing remembered -- where the refusal put them before this existed.
The refusal itself is unchanged: no session, no approval, and the code
alone grants nothing.

The approval page also names what is asking. It listed the scopes but
never the grantee, and a grant with no named grantee is a reflex rather
than a decision. It now names the client and says when the request
expires. It does not name the computer: nothing in a device
authorization records one, and inventing an answer would be worse than
omitting it.

Proof is adversarial. DeviceSignInReturnTest walks the whole round trip
-- bounce, landing, sign-in, callback, approval, approve -- and then
enumerates eighteen crafted user_code values, asserting for each that it
reaches neither the session, nor the redirect, nor the page, and that a
sign-in completed after it still lands on the dashboard. IDENTITY-001
records the amendment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SoZMfWRSGnf6FZX2Ar9rQ2
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>

Deploy story

What this commit did to the running system — joined from the forge receipt chain, the part a commit page elsewhere cannot show.

pushed
by user · WAL seq 451 · 2026-08-26T15:51:04.867963Z
built
21 modules in 153.7 s
deployed
live · 21 modules on 3 nodes · push→live —
deployed
needs_rolling_replace · 21 modules on 0 nodes · push→live —

Changed files

  • modified INVARIANTS.md
  • modified lib/openagents/device_authorizations.ex
  • modified lib/openagents_web/controllers/auth_controller.ex
  • modified lib/openagents_web/live/device_authorization_live.ex
  • modified lib/openagents_web/live/home_live.ex
  • modified lib/openagents_web/user_auth.ex
  • added test/openagents_web/device_sign_in_return_test.exs
  • modified test/openagents_web/live/device_authorization_live_test.exs

Diff

8 files changed, +486 -11

INVARIANTS.md modified +26 -2

@@ -279,9 +279,33 @@ session and requires each to refuse. The two OAuth entries are in that class

279 279
and refuse the same way — an anonymous request reaches them and leaves with an
280 280
`auth_error` and no session.
281 281
282
Amended 2026-08-26 (issue #129): a refusal at `/device` remembers the terminal
283
code in the browser session and returns the reader to the approval after the
284
OAuth round trip, so one intent costs one sign-in rather than two. The refusal
285
itself is unchanged — no session, no approval, and the code alone grants
286
nothing — but two of its outputs now depend on a value the browser sent: where
287
the redirect points, and what the landing page prints.
288
289
That value is admitted by exactly one function.
290
`OpenAgents.DeviceAuthorizations.cast_user_code/1` returns `{:ok, code}` only
291
for the eight-character alphabet this application mints, anchored so no second
292
line can follow, and every place that puts the value into a URL, a session, or
293
a page casts it rather than trusting an earlier cast. Anything else — a host, a
294
scheme-relative URL, a path, a CRLF, markup, a query-string list — leaves the
295
reader on the bare public root with nothing remembered, which is where the
296
refusal put them before this existed. The proof is adversarial and enumerates
297
the crafted forms rather than asserting the happy path.
298
299
What this does not do: name the computer that is asking. Nothing in the device
300
authorization records one, so the approval page names the client and the
301
scopes, which are what the application actually knows.
302
282 303
Evidence: `OpenAgents.GitHubOAuth`, `OpenAgents.Accounts`, `OpenAgentsWeb.AuthController`,
304
`OpenAgentsWeb.UserAuth.require_authenticated_user/2`,
305
`OpenAgents.DeviceAuthorizations.cast_user_code/1`,
283 306
`OpenAgentsWeb.Endpoint.session_options/0`, `OpenAgents.GitHubOAuthTest`,
284
`OpenAgents.AccountsTest`, `OpenAgentsWeb.AuthControllerTest`, and
307
`OpenAgents.AccountsTest`, `OpenAgentsWeb.AuthControllerTest`,
308
`OpenAgentsWeb.DeviceSignInReturnTest`, and
285 309
`OpenAgentsWeb.AuthenticatedRouteGateTest`.
286 310
287 311
### IDENTITY-002 — Conversation lookup never accepts a client database ID

@@ -6272,7 +6296,7 @@ contract; the invariant prose above defines the assertion, not the filename.

6272 6296
| DEGRADE-001 | `test/openagents/program_artifacts_test.exs`, `test/openagents/turn_provenance_test.exs` |
6273 6297
| PROGRAM-002 | `test/openagents/shadow_programs_test.exs` |
6274 6298
| PROGRAM-003 | `test/openagents/program_lifecycle_test.exs` |
6275
| IDENTITY-001 | `test/openagents/github_oauth_test.exs`, `test/openagents_web/auth_controller_test.exs`, `test/openagents_web/authenticated_route_gate_test.exs` |
6299
| IDENTITY-001 | `test/openagents/github_oauth_test.exs`, `test/openagents_web/auth_controller_test.exs`, `test/openagents_web/device_sign_in_return_test.exs`, `test/openagents_web/authenticated_route_gate_test.exs` |
6276 6300
| IDENTITY-002 | `test/openagents_web/auth_gate_test.exs`, `test/openagents_web/authenticated_route_gate_test.exs`, `test/openagents_web/live_view_scope_test.exs` |
6277 6301
| IDENTITY-003 | `test/openagents/memory_portability_test.exs` |
6278 6302
| IDENTITY-004 | `test/openagents/agents_test.exs`, `test/openagents_web/controllers/agent_controller_test.exs` |
lib/openagents/device_authorizations.ex modified +33 -1

@@ -22,6 +22,38 @@ defmodule OpenAgents.DeviceAuthorizations do

22 22
  @interval_seconds 5
23 23
  @maximum_create_attempts 3
24 24
25
  # The alphabet `random_user_code/0` draws from. `I`, `O`, `0`, and `1` are
26
  # absent on purpose: a code is read off one screen and typed into another.
27
  @user_code_alphabet "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"
28
  @user_code_pattern ~r/\A[ABCDEFGHJKLMNPQRSTUVWXYZ23456789]{4}-[ABCDEFGHJKLMNPQRSTUVWXYZ23456789]{4}\z/
29
30
  @doc """
31
  Casts a caller-supplied user code to the exact shape this module mints.
32
33
  `get_pending_by_user_code/1` can be handed anything and answers `nil`, which
34
  is the right answer for a lookup. It is the wrong answer for anything that
35
  puts the value back into a URL, a page, or a session, because "no such
36
  authorization" and "not a code at all" are then indistinguishable.
37
38
  This is that second question, and it is asked wherever a code the browser
39
  sent goes on to build something. The pattern is anchored with `\\A` and
40
  `\\z` rather than `^` and `$`, so a trailing newline cannot smuggle a second
41
  line past it, and it admits only the thirty-two characters and one hyphen
42
  above — never a path, a host, a scheme, a quote, or a tag.
43
44
  Trimming and upcasing come first, so a code retyped in lowercase is the same
45
  code. A letter this alphabet excludes is not silently corrected to one it
46
  admits: `i` upcases to `I`, which is not in the set, and is refused.
47
  """
48
  @spec cast_user_code(term()) :: {:ok, String.t()} | :error
49
  def cast_user_code(code) when is_binary(code) do
50
    normalized = normalize_user_code(code)
51
52
    if Regex.match?(@user_code_pattern, normalized), do: {:ok, normalized}, else: :error
53
  end
54
55
  def cast_user_code(_code), do: :error
56
25 57
  def create(scopes \\ ApiTokens.default_scopes())
26 58
27 59
  def create(scopes) when is_list(scopes), do: create(scopes, @maximum_create_attempts)

@@ -214,7 +246,7 @@ defmodule OpenAgents.DeviceAuthorizations do

214 246
  end
215 247
216 248
  defp random_user_code do
217
    alphabet = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"
249
    alphabet = @user_code_alphabet
218 250
219 251
    8
220 252
    |> :crypto.strong_rand_bytes()
lib/openagents_web/controllers/auth_controller.ex modified +30 -2

@@ -1,7 +1,7 @@

1 1
defmodule OpenAgentsWeb.AuthController do
2 2
  use OpenAgentsWeb, :controller
3 3
4
  alias OpenAgents.{Accounts, Analytics, GitHubOAuth, Repositories}
4
  alias OpenAgents.{Accounts, Analytics, DeviceAuthorizations, GitHubOAuth, Repositories}
5 5
6 6
  @attempt_session_key "github_oauth_attempt"
7 7
  @identity_session_key "posthog_identity"

@@ -26,6 +26,11 @@ defmodule OpenAgentsWeb.AuthController do

26 26
  def callback(conn, %{"code" => code, "state" => state}) do
27 27
    attempt = get_session(conn, @attempt_session_key)
28 28
    verifier = if is_map(attempt), do: attempt["verifier"]
29
30
    # Read before the session is cleared below, because clearing it is what
31
    # takes the remembered device code away.
32
    landing = landing_path(get_session(conn, OpenAgentsWeb.UserAuth.device_session_key()))
33
29 34
    conn = delete_session(conn, @attempt_session_key)
30 35
31 36
    with :ok <- GitHubOAuth.consume_attempt(attempt, state),

@@ -44,7 +49,7 @@ defmodule OpenAgentsWeb.AuthController do

44 49
      |> put_session("user_id", active_user.id)
45 50
      |> put_session(@identity_session_key, identity(active_user))
46 51
      |> put_resp_header("cache-control", "no-store")
47
      |> redirect(to: ~p"/sarah")
52
      |> redirect(to: landing)
48 53
    else
49 54
      {:error, :banned} -> auth_failure(conn, "banned")
50 55
      {:error, _reason} -> auth_failure(conn, "failed")

@@ -93,6 +98,29 @@ defmodule OpenAgentsWeb.AuthController do

93 98
    end
94 99
  end
95 100
101
  # Where a completed sign-in lands.
102
  #
103
  # A reader who came from `/device` is halfway through authorizing a terminal,
104
  # not starting a session at the dashboard. `OpenAgentsWeb.UserAuth` remembers
105
  # the terminal's code when it bounces them here, and this returns them to the
106
  # approval with the code still in hand, so approving is the next click rather
107
  # than a fresh errand.
108
  #
109
  # The code is cast a second time on the way out. The session is signed, so
110
  # this is not defending against a forged cookie; it is keeping one rule —
111
  # only a value this application mints ever becomes part of a URL — true at
112
  # every place that builds one, rather than true here because it happened to
113
  # be checked somewhere else.
114
  #
115
  # Nothing writes the key back. It has done its work, and `clear_session/1`
116
  # above takes it with the rest.
117
  defp landing_path(code) do
118
    case DeviceAuthorizations.cast_user_code(code) do
119
      {:ok, code} -> ~p"/device?user_code=#{code}"
120
      :error -> ~p"/sarah"
121
    end
122
  end
123
96 124
  # A row created and updated in the same write is a first sign-in; anything
97 125
  # else reauthenticated an existing account.
98 126
  defp capture_sign_in(user) do
lib/openagents_web/live/device_authorization_live.ex modified +23 -2

@@ -92,7 +92,16 @@ defmodule OpenAgentsWeb.DeviceAuthorizationLive do

92 92
              <code class="text-2xl font-semibold tracking-widest">{@form[:user_code].value}</code>
93 93
            </div>
94 94
            <div>
95
              <p class="font-medium">Requested access</p>
95
              <%!-- Approving is a grant, and a grant with no named grantee is a
96
              reflex rather than a decision. The CLI is what this application
97
              knows is asking: it is the only client that mints a device
98
              authorization, and `DeviceAuthorizations.claim/3` names the token
99
              it walks away with "OpenAgents CLI". Which computer it is running
100
              on is not recorded, so this does not claim to say. --%>
101
              <p class="font-medium">The OpenAgents CLI is asking to act as you</p>
102
              <p class="mt-1 text-sm text-muted-foreground">
103
                Approving gives it these permissions, and no others:
104
              </p>
96 105
              <ul class="mt-2 space-y-1 text-sm text-muted-foreground">
97 106
                <li :for={scope <- @authorization.scopes} class="flex items-baseline gap-2">
98 107
                  <code>{scope}</code>

@@ -104,7 +113,9 @@ defmodule OpenAgentsWeb.DeviceAuthorizationLive do

104 113
                only if you started this login yourself.
105 114
              </p>
106 115
              <p class="mt-3 text-sm text-muted-foreground">
107
                The CLI never receives your GitHub token.
116
                The CLI never receives your GitHub token. This request expires {expires_in(
117
                  @authorization
118
                )}; after that the terminal has to ask again.
108 119
              </p>
109 120
            </div>
110 121
            <div class="flex flex-wrap justify-end gap-3">

@@ -147,6 +158,16 @@ defmodule OpenAgentsWeb.DeviceAuthorizationLive do

147 158
148 159
  defp code_form(user_code), do: to_form(%{"user_code" => user_code}, as: :device)
149 160
161
  # Rendered once at mount and not counted down. A ticking clock would make the
162
  # page a timer, and the window is ten minutes: what the reader needs is that
163
  # there is one, not the second it lands on.
164
  defp expires_in(%{expires_at: expires_at}) do
165
    case DateTime.diff(expires_at, DateTime.utc_now(), :second) do
166
      seconds when seconds <= 60 -> "in under a minute"
167
      seconds -> "in about #{div(seconds + 30, 60)} minutes"
168
    end
169
  end
170
150 171
  defp scope_description("forge:write"),
151 172
    do: "Create and manage repositories, issues, and pull requests as you."
152 173
lib/openagents_web/live/home_live.ex modified +65 -2

@@ -53,7 +53,7 @@ defmodule OpenAgentsWeb.HomeLive do

53 53
  @changelog_limit 5
54 54
55 55
  @impl true
56
  def mount(_params, _session, socket) do
56
  def mount(params, _session, socket) do
57 57
    if socket.assigns[:current_user] do
58 58
      if connected?(socket), do: subscribe()
59 59

@@ -63,10 +63,28 @@ defmodule OpenAgentsWeb.HomeLive do

63 63
       |> assign(:changed_repositories, MapSet.new())
64 64
       |> assign_dashboard()}
65 65
    else
66
      {:ok, socket}
66
      {:ok, assign(socket, :device_user_code, device_user_code(params))}
67 67
    end
68 68
  end
69 69
70
  # A reader who arrives here from `/device` was sent by their terminal, not by
71
  # a link to a product. `OpenAgentsWeb.UserAuth` puts the terminal's code in
72
  # the URL on the way past so this page can say what the sign-in is for; the
73
  # session it also wrote is what actually returns them afterwards, so this
74
  # value decides what is rendered and nothing else.
75
  #
76
  # It is cast rather than read. The parameter is printed on the page, so
77
  # admitting only the shape this application mints is what keeps a crafted
78
  # link from putting its own words in OpenAgents's mouth.
79
  defp device_user_code(params) when is_map(params) do
80
    case OpenAgents.DeviceAuthorizations.cast_user_code(params["user_code"]) do
81
      {:ok, code} -> code
82
      :error -> nil
83
    end
84
  end
85
86
  defp device_user_code(_not_mounted_at_router), do: nil
87
70 88
  # Every panel here already had a publisher or has one now, so the dashboard
71 89
  # can stop being a snapshot of the moment it was opened. The messages carry
72 90
  # ids and nothing else: each panel re-reads through the same authorized read

@@ -452,6 +470,51 @@ defmodule OpenAgentsWeb.HomeLive do

452 470
    """
453 471
  end
454 472
473
  # A reader mid-way through authorizing a terminal is not here to be sold the
474
  # product; they are here because a sign-in stands between them and one
475
  # approval. Showing them the landing page would be the third screen in a row
476
  # that does not mention what they are actually doing, so this says it: the
477
  # terminal, its code, and where the sign-in puts them next.
478
  #
479
  # None of the sign-in controls need the code threaded through them. It is in
480
  # the session already, so the command bar's control returns the reader to the
481
  # approval exactly as this one does.
482
  def render(%{device_user_code: code} = assigns) when is_binary(code) do
483
    ~H"""
484
    <Layouts.app
485
      flash={@flash}
486
      sidebar_sections={assigns[:sidebar_sections]}
487
      current_scope={@current_scope}
488
    >
489
      <main id="device-sign-in" class="mx-auto w-full max-w-lg space-y-8 px-4 py-16">
490
        <.header>
491
          Sign in to authorize your terminal
492
          <:subtitle>
493
            Your terminal is waiting on this code. Signing in brings you straight back to
494
            the approval — you will not need to enter it again.
495
          </:subtitle>
496
        </.header>
497
498
        <.card>
499
          <div class="space-y-6">
500
            <div>
501
              <p class="text-sm text-muted-foreground">Code shown in your terminal</p>
502
              <code id="device-sign-in-code" class="text-2xl font-semibold tracking-widest">
503
                {@device_user_code}
504
              </code>
505
            </div>
506
            <p class="text-sm text-muted-foreground">
507
              Check that it matches before you approve. OpenAgents signs you in through GitHub;
508
              your GitHub token stays here and never reaches the terminal.
509
            </p>
510
            <.github_login id="device-signin" size={:lg} />
511
          </div>
512
        </.card>
513
      </main>
514
    </Layouts.app>
515
    """
516
  end
517
455 518
  def render(assigns) do
456 519
    ~H"""
457 520
    <Layouts.app
lib/openagents_web/user_auth.ex modified +56 -1

@@ -6,9 +6,22 @@ defmodule OpenAgentsWeb.UserAuth do

6 6
  import Plug.Conn
7 7
8 8
  alias OpenAgents.Accounts
9
  alias OpenAgents.DeviceAuthorizations
9 10
10 11
  @session_key "user_id"
11 12
13
  # Read back by `OpenAgentsWeb.AuthController.callback/2`. Both ends go
14
  # through `DeviceAuthorizations.cast_user_code/1`, so nothing but a code this
15
  # application mints ever reaches it or leaves it.
16
  @device_session_key "device_user_code"
17
18
  # The path is written out because a route pattern cannot be a `~p` sigil.
19
  # `OpenAgentsWeb.RouteAuthority` classifies the same literal.
20
  @device_path "/device"
21
22
  @doc "The session key carrying a device authorization across the sign-in."
23
  def device_session_key, do: @device_session_key
24
12 25
  def put_no_store(conn, _options), do: put_resp_header(conn, "cache-control", "no-store")
13 26
14 27
  def fetch_current_user(conn, _options) do

@@ -30,12 +43,54 @@ defmodule OpenAgentsWeb.UserAuth do

30 43
      do: conn
31 44
32 45
  def require_authenticated_user(conn, _options) do
46
    {conn, sign_in_path} = remember_device_authorization(conn)
47
33 48
    conn
34 49
    |> put_resp_header("cache-control", "no-store")
35
    |> Phoenix.Controller.redirect(to: ~p"/")
50
    |> Phoenix.Controller.redirect(to: sign_in_path)
36 51
    |> halt()
37 52
  end
38 53
54
  # `/device` is the one authenticated route a reader reaches before they have
55
  # a session here: a terminal sent them, and the code it printed is in the
56
  # link. Bouncing them to the public root and forgetting the code turns one
57
  # intent into two errands — sign in, then go and find the code again — which
58
  # is what issue #129 is about.
59
  #
60
  # So the code is remembered in the session on the way out.
61
  # `OpenAgentsWeb.AuthController.callback/2` reads it back after the OAuth
62
  # round trip and returns the reader to the approval with the code already in
63
  # hand, so approving is the next click. Carrying it in the session rather
64
  # than in the sign-in form means every sign-in control returns them —
65
  # the landing page's and the command bar's alike — instead of only the one
66
  # that was threaded with the code.
67
  #
68
  # It rides in the URL as well, but only so the landing page can say what the
69
  # sign-in is for. The session is the authority; the parameter is display.
70
  #
71
  # Only a code travels, never a path. `cast_user_code/1` admits exactly the
72
  # shape this application mints, so a crafted `?user_code=` carrying a host, a
73
  # path, a newline, or markup is refused here and the reader lands on the
74
  # public root exactly as they did before. This is the only place in the
75
  # sign-out path that builds a redirect from anything the browser sent, and it
76
  # builds one of two constants either way.
77
  defp remember_device_authorization(%{request_path: @device_path} = conn) do
78
    conn = Plug.Conn.fetch_query_params(conn)
79
80
    case DeviceAuthorizations.cast_user_code(conn.query_params["user_code"]) do
81
      {:ok, code} ->
82
        {put_session(conn, @device_session_key, code), ~p"/?user_code=#{code}"}
83
84
      # Reaching the device page without a usable code says the reader is not
85
      # mid-flow with the one we may have remembered earlier. Drop it rather
86
      # than let it decide where a later sign-in lands.
87
      :error ->
88
        {delete_session(conn, @device_session_key), ~p"/"}
89
    end
90
  end
91
92
  defp remember_device_authorization(conn), do: {conn, ~p"/"}
93
39 94
  def require_admin_user(conn, _options) do
40 95
    if Accounts.admin?(conn.assigns[:current_user]) do
41 96
      conn
test/openagents_web/device_sign_in_return_test.exs added +246

@@ -0,0 +1,246 @@

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
test/openagents_web/live/device_authorization_live_test.exs modified +7 -1

@@ -20,9 +20,15 @@ defmodule OpenAgentsWeb.DeviceAuthorizationLiveTest do

20 20
    refute has_element?(view, "#device-authorization-review")
21 21
  end
22 22
23
  # Still a refusal: no session, no approval, and the code alone grants nothing.
24
  # What changed with issue #129 is where the refusal sends them. It used to be
25
  # the bare public root, which forgot why they were there; it is now the
26
  # sign-in that returns them here with the code still in hand.
27
  # `OpenAgentsWeb.DeviceSignInReturnTest` proves the whole round trip.
23 28
  test "a signed-out browser cannot approve a device code", %{conn: conn} do
24 29
    {:ok, _authorization, _device_code, user_code} = OpenAgents.DeviceAuthorizations.create()
25 30
26
    assert {:error, {:redirect, %{to: "/"}}} = live(conn, ~p"/device?user_code=#{user_code}")
31
    assert {:error, {:redirect, %{to: "/?user_code=" <> ^user_code}}} =
32
             live(conn, ~p"/device?user_code=#{user_code}")
27 33
  end
28 34
end

This page updates live while a promote is in flight · changelog