From 1376147666eb7c30b78cd616ec78edf07b179d82 Mon Sep 17 00:00:00 2001 From: Johanna Larsson Date: Sun, 25 Oct 2026 08:08:44 +0000 Subject: [PATCH] On refresh, handle changing pds endpoint and issuer Always look up did doc before refresh. If the issuer has changed, the refresh is rejected. The user must log in again because there is no grant anymore. If the pds endpoint has changed but issuer is the same, we can try to refresh. --- lib/latch/client.ex | 8 ++- lib/latch/flow.ex | 13 ++++- test/latch/client_test.exs | 24 +++++++- test/latch/flow_test.exs | 105 +++++++++++++++++++++++++++++----- test/latch/store/ets_test.exs | 13 ++++- 5 files changed, 138 insertions(+), 25 deletions(-) diff --git a/lib/latch/client.ex b/lib/latch/client.ex index 893655e..8590116 100644 --- a/lib/latch/client.ex +++ b/lib/latch/client.ex @@ -15,6 +15,7 @@ defmodule Latch.Client do alias Latch.Error.Store, as: StoreError alias Latch.Error.XRPC, as: XRPCError alias Latch.Flow + alias Latch.Identity alias Latch.XRPC @refresh_buffer_seconds 60 @@ -115,11 +116,12 @@ defmodule Latch.Client do defp do_refresh(config, session) do result = - with {:ok, server} <- - Discovery.discover(config, session.pds_endpoint, + with {:ok, %Identity{} = identity} <- Identity.resolve_did(config, session.did), + {:ok, server} <- + Discovery.discover(config, identity.pds_endpoint, allow_http: Config.localhost?(config) ) do - Flow.refresh(config, server, session, + Flow.refresh(config, server, session, identity, client_id: Config.client_id(config), client_jwk: config.signing_key ) diff --git a/lib/latch/flow.ex b/lib/latch/flow.ex index f4fe4d6..0327cfb 100644 --- a/lib/latch/flow.ex +++ b/lib/latch/flow.ex @@ -19,6 +19,7 @@ defmodule Latch.Flow do alias Latch.Error.SecurityViolation alias Latch.Error.Transport alias Latch.HTTP + alias Latch.Identity alias Latch.ServerMetadata alias Latch.Session alias Latch.TokenResponse @@ -150,7 +151,7 @@ defmodule Latch.Flow do ## Optional - `:now` - base time for `expires_at` (defaults to the current time) """ - @spec refresh(Config.t(), ServerMetadata.t(), Session.t(), keyword()) :: + @spec refresh(Config.t(), ServerMetadata.t(), Session.t(), Identity.t(), keyword()) :: {:ok, Session.t()} | {:error, InvalidResponse.t() @@ -158,7 +159,13 @@ defmodule Latch.Flow do | OAuth.t() | SecurityViolation.t() | Transport.t()} - def refresh(config, %ServerMetadata{} = server, %Session{} = session, opts) do + def refresh( + config, + %ServerMetadata{} = server, + %Session{} = session, + %Identity{} = identity, + opts + ) do client_id = Keyword.fetch!(opts, :client_id) now = Keyword.get_lazy(opts, :now, &DateTime.utc_now/0) @@ -179,7 +186,7 @@ defmodule Latch.Flow do build_session( tokens, server.issuer, - session.pds_endpoint, + identity.pds_endpoint, session.dpop_key, now )} diff --git a/test/latch/client_test.exs b/test/latch/client_test.exs index 9c8676d..db24bb9 100644 --- a/test/latch/client_test.exs +++ b/test/latch/client_test.exs @@ -7,12 +7,14 @@ defmodule Latch.ClientTest do alias Latch.Discovery alias Latch.Error.XRPC, as: XRPCError alias Latch.Flow + alias Latch.Identity alias Latch.ServerMetadata alias Latch.Session alias Latch.XRPC @did "did:plc:bvraa6gajy4tfr3eh2sisdkr" @pds "https://pds.example.com" + @handle "handle" @issuer "https://issuer.example.com" @client_id_path "/oauth-client-metadata.json" @redirect_uri_path "/oauth/callback" @@ -23,16 +25,21 @@ defmodule Latch.ClientTest do stale_session = session("stale-access-token", ~U[2020-01-01 00:00:00Z]) refreshed_session = session("fresh-access-token", ~U[2030-01-01 00:00:00Z]) server = server() + identity = make_identity() :ok = Latch.TestStore.put_session(@did, stale_session) + expect(Identity, :resolve_did, fn _config, _did -> + {:ok, identity} + end) + expect(Discovery, :discover, fn _pool, @pds, _opts -> {:ok, server} end) expect(Task.Supervisor, :async_nolink, fn _name, fun -> Task.completed(fun.()) end) - expect(Flow, :refresh, fn _config, ^server, ^stale_session, opts -> + expect(Flow, :refresh, fn _config, ^server, ^stale_session, _identity, opts -> assert opts[:client_id] == Latch.Config.client_id(config) assert opts[:client_jwk] == config.signing_key {:ok, refreshed_session} @@ -56,6 +63,7 @@ defmodule Latch.ClientTest do stale_session = session("stale-access-token", ~U[2030-01-01 00:00:00Z]) refreshed_session = session("fresh-access-token", ~U[2030-01-01 00:00:00Z]) server = server() + identity = make_identity() :ok = Latch.TestStore.put_session(@did, stale_session) @@ -67,13 +75,17 @@ defmodule Latch.ClientTest do {:ok, %{"did" => @did}} end) + expect(Identity, :resolve_did, fn _config, _did -> + {:ok, identity} + end) + expect(Discovery, :discover, fn _pool, @pds, _opts -> {:ok, server} end) expect(Task.Supervisor, :async_nolink, fn _name, fun -> Task.completed(fun.()) end) - expect(Flow, :refresh, fn _config, ^server, ^stale_session, _opts -> + expect(Flow, :refresh, fn _config, ^server, ^stale_session, _identity, _opts -> {:ok, refreshed_session} end) @@ -156,4 +168,12 @@ defmodule Latch.ClientTest do base_url_fun: fn -> "https://client.example.com" end } end + + defp make_identity do + %Identity{ + did: @did, + pds_endpoint: @pds, + handle: @handle + } + end end diff --git a/test/latch/flow_test.exs b/test/latch/flow_test.exs index 4d54c3c..467854b 100644 --- a/test/latch/flow_test.exs +++ b/test/latch/flow_test.exs @@ -6,12 +6,17 @@ defmodule Latch.FlowTest do alias Latch.Error.SecurityViolation alias Latch.Flow alias Latch.HTTP + alias Latch.Identity alias Latch.ServerMetadata alias Latch.Session + @did "did:plc:id" + @handle "handle" + @pds_endpoint "https://pds.example.com" + @issuer "https://issuer.example.com" + describe "exchange_code/1" do test "exchanges an authorization code for a session" do - did = "did:plc:bvraa6gajy4tfr3eh2sisdkr" access_token = "access-token" refresh_token = "refresh-token" config = make_config() @@ -44,7 +49,7 @@ defmodule Latch.FlowTest do "token_type" => "DPoP", "expires_in" => 3600, "scope" => "atproto", - "sub" => did + "sub" => @did }) }} end) @@ -57,14 +62,14 @@ defmodule Latch.FlowTest do code: "authorization-code", code_verifier: "pkce-verifier", dpop_key: dpop_key, - expected_did: did, - pds_endpoint: "https://pds.example.com", - issuer: "https://issuer.example.com", - token_endpoint: "https://issuer.example.com/oauth/token", + expected_did: @did, + pds_endpoint: @pds_endpoint, + issuer: @issuer, + token_endpoint: "#{@issuer}/oauth/token", now: ~U[2026-01-01 00:00:00Z] ) - assert session.did == did + assert session.did == @did assert session.access_token == access_token assert session.refresh_token == refresh_token assert session.dpop_key == dpop_key @@ -255,6 +260,49 @@ defmodule Latch.FlowTest do end describe "refresh/3" do + test "refreshes successfully even if pds_endpoint changes" do + config = make_config() + + start_link_supervised!( + {Latch.NonceCache, config: config, name: config.name, sweep_disabled: true} + ) + + server = make_server_metadata() + # simulate an account that has migrated to a new pds endpoint + # but is still behind the same issuer + session = make_session(pds_endpoint: "https://pds.example.com") + identity = make_identity(pds_endpoint: "https://pds.other.com") + + expect(HTTP, :post_form, fn _config, _url, _form, _headers -> + {:ok, + %{ + status: 201, + headers: %{}, + body: + Jason.encode!(%{ + "access_token" => "new-access-token", + "refresh_token" => "new-refresh-token", + "token_type" => "DPoP", + "expires_in" => 3600, + "scope" => "atproto", + "sub" => session.did + }) + }} + end) + + assert {:ok, %Session{} = new_session} = + Flow.refresh(config, server, session, identity, + client_id: "client-id", + client_jwk: nil + ) + + assert new_session.did == @did + assert new_session.access_token == "new-access-token" + assert new_session.refresh_token == "new-refresh-token" + # uses the pds_endpoint from identity, not the old one on session + assert new_session.pds_endpoint == identity.pds_endpoint + end + test "rejects a refresh when discovery returns a different issuer" do reject(HTTP, :post_form, 4) config = make_config() @@ -272,18 +320,20 @@ defmodule Latch.FlowTest do } session = %Session{ - did: "did:plc:bvraa6gajy4tfr3eh2sisdkr", + did: @did, access_token: "access-token", refresh_token: "refresh-token", dpop_key: nil, scope: "atproto", - issuer: "https://issuer.example.com", - pds_endpoint: "https://pds.example.com", + issuer: @issuer, + pds_endpoint: @pds_endpoint, expires_at: ~U[2026-01-01 00:00:00Z] } + identity = make_identity() + assert {:error, %SecurityViolation{reason: :issuer_mismatch}} = - Flow.refresh(config, server, session, + Flow.refresh(config, server, session, identity, client_id: "client-id", client_jwk: nil ) @@ -308,11 +358,36 @@ defmodule Latch.FlowTest do defp make_server_metadata do %ServerMetadata{ - issuer: "https://issuer.example.com", - authorization_endpoint: "https://issuer.example.com/oauth/authorize", - token_endpoint: "https://issuer.example.com/oauth/token", - par_endpoint: "https://issuer.example.com/oauth/par", + issuer: @issuer, + authorization_endpoint: "#{@issuer}/oauth/authorize", + token_endpoint: "#{@issuer}/oauth/token", + par_endpoint: "#{@issuer}/oauth/par", scopes_supported: ["atproto"] } end + + defp make_identity(overrides \\ []) do + attrs = Keyword.merge([did: @did, pds_endpoint: @pds_endpoint, handle: @handle], overrides) + + struct(Identity, attrs) + end + + defp make_session(overrides) do + attrs = + Keyword.merge( + [ + did: @did, + access_token: "access-token", + refresh_token: "refresh-token", + dpop_key: DPoP.generate_key(), + scope: "atproto", + issuer: "https://issuer.example.com", + pds_endpoint: @pds_endpoint, + expires_at: ~U[2026-01-01 00:00:00Z] + ], + overrides + ) + + struct(Session, attrs) + end end diff --git a/test/latch/store/ets_test.exs b/test/latch/store/ets_test.exs index 6f610fb..da3d919 100644 --- a/test/latch/store/ets_test.exs +++ b/test/latch/store/ets_test.exs @@ -213,9 +213,13 @@ defmodule Latch.Store.ETSTest do {:ok, session} end) + expect(Identity, :resolve_did, fn _config, _did -> + {:ok, identity} + end) + expect(Discovery, :discover, fn _config, _pds, _opts -> {:ok, server} end) - expect(Flow, :refresh, fn _config, _server, old_session, _opts -> + expect(Flow, :refresh, fn _config, _server, old_session, _identity, _opts -> assert session == old_session {:ok, @@ -287,11 +291,15 @@ defmodule Latch.Store.ETSTest do {:ok, session} end) + expect(Identity, :resolve_did, fn _config, _did -> + {:ok, identity} + end) + expect(Discovery, :discover, fn _config, _pds, _opts -> {:ok, server} end) # Refresh only happens once even when we trigger refresh 10 times # by using XPRC in a `for` below. - expect(Flow, :refresh, fn _config, _server, old_session, _opts -> + expect(Flow, :refresh, fn _config, _server, old_session, _identity, _opts -> assert session == old_session {:ok, @@ -338,6 +346,7 @@ defmodule Latch.Store.ETSTest do store_pid = start_link_supervised!({Latch.ETSStore, []}) Mimic.allow(Latch.Flow, self(), store_pid) Mimic.allow(Latch.Discovery, self(), store_pid) + Mimic.allow(Latch.Identity, self(), store_pid) opts = Keyword.merge( -- knotmirror 0.0.1