From 892dc3a1713183a1f48114acdcbd9df11af4b3f8 Mon Sep 17 00:00:00 2001 From: Johanna Larsson Date: Tue, 14 Jul 2026 22:25:38 +0100 Subject: [PATCH] Simplify NonceCache Drops session ID and uses DPoP key thumbprint instead, that should work to uniquely identify a session. Simplifies things a bit --- lib/latch.ex | 13 +++++------- lib/latch/client.ex | 2 +- lib/latch/flow.ex | 43 +++++++++++++++++++------------------- lib/latch/nonce_cache.ex | 8 +++---- lib/latch/request.ex | 6 ++---- lib/latch/session.ex | 6 ++---- lib/latch/xrpc.ex | 32 ++++++++++++++++++++++------ test/latch/client_test.exs | 7 +++---- test/latch/flow_test.exs | 18 +++++----------- test/latch_test.exs | 15 ++++++------- 10 files changed, 75 insertions(+), 75 deletions(-) diff --git a/lib/latch.ex b/lib/latch.ex index e16bf77..fdb5104 100644 --- a/lib/latch.ex +++ b/lib/latch.ex @@ -69,12 +69,11 @@ defmodule Latch do verifier = PKCE.generate_verifier() dpop_key = DPoP.generate_key() state = Base.url_encode64(:crypto.strong_rand_bytes(32), padding: false) - session_id = Base.url_encode64(:crypto.strong_rand_bytes(32), padding: false) with {:ok, identity} <- Identity.resolve_handle(handle), {:ok, server} <- Discovery.discover(identity.pds_endpoint), {:ok, request_uri} <- - Flow.par(config, session_id, server, + Flow.par(config, server, client_id: config.client_id, client_jwk: config.signing_key, redirect_uri: config.redirect_uri, @@ -92,8 +91,7 @@ defmodule Latch do issuer: server.issuer, token_endpoint: server.token_endpoint, pkce_verifier: verifier, - dpop_key: dpop_key, - session_id: session_id + dpop_key: dpop_key }, :ok <- store_request(config, request) do {:ok, Flow.authorization_url(server, config.client_id, request_uri)} @@ -117,7 +115,7 @@ defmodule Latch do %Config{} = config = config(name) with {:ok, server} <- Discovery.discover(session.pds_endpoint) do - Flow.refresh(config, session.session_id, server, session, + Flow.refresh(config, server, session, client_id: config.client_id, client_jwk: config.signing_key ) @@ -147,7 +145,7 @@ defmodule Latch do ) when is_binary(code) do with :ok <- verify_issuer(request, issuer) do - Flow.exchange_code(config, request.session_id, + Flow.exchange_code(config, client_id: config.client_id, client_jwk: config.signing_key, redirect_uri: config.redirect_uri, @@ -157,8 +155,7 @@ defmodule Latch do expected_did: request.did, pds_endpoint: request.pds_endpoint, issuer: request.issuer, - token_endpoint: request.token_endpoint, - session_id: request.session_id + token_endpoint: request.token_endpoint ) end end diff --git a/lib/latch/client.ex b/lib/latch/client.ex index 810c186..a65cb2e 100644 --- a/lib/latch/client.ex +++ b/lib/latch/client.ex @@ -100,7 +100,7 @@ defmodule Latch.Client do defp do_refresh(config, session) do result = with {:ok, server} <- Discovery.discover(session.pds_endpoint) do - Flow.refresh(config, session.session_id, server, session, + Flow.refresh(config, server, session, client_id: config.client_id, client_jwk: config.signing_key ) diff --git a/lib/latch/flow.ex b/lib/latch/flow.ex index 14e4982..d826e13 100644 --- a/lib/latch/flow.ex +++ b/lib/latch/flow.ex @@ -35,10 +35,10 @@ defmodule Latch.Flow do - `:login_hint` - the user's handle or DID """ - @spec par(Config.t(), String.t(), ServerMetadata.t(), keyword()) :: + @spec par(Config.t(), ServerMetadata.t(), keyword()) :: {:ok, String.t()} | {:error, InvalidResponse.t() | MissingDPoPNonce.t() | OAuth.t() | Transport.t()} - def par(%Config{} = config, session_id, %ServerMetadata{} = server, opts) do + def par(%Config{} = config, %ServerMetadata{} = server, opts) do client_id = Keyword.fetch!(opts, :client_id) client_jwk = Keyword.fetch!(opts, :client_jwk) redirect_uri = Keyword.fetch!(opts, :redirect_uri) @@ -67,7 +67,7 @@ defmodule Latch.Flow do end with {:ok, body} <- - dpop_request(config, session_id, server.par_endpoint, build_form, dpop_key) do + dpop_request(config, server.par_endpoint, build_form, dpop_key) do parse_par_response(body) end end @@ -102,7 +102,7 @@ defmodule Latch.Flow do ## Optional - `:now` - base time for `expires_at` (defaults to the current time) """ - @spec exchange_code(Config.t(), String.t(), keyword()) :: + @spec exchange_code(Config.t(), keyword()) :: {:ok, Session.t()} | {:error, InvalidResponse.t() @@ -110,7 +110,7 @@ defmodule Latch.Flow do | OAuth.t() | SecurityViolation.t() | Transport.t()} - def exchange_code(%Config{} = config, session_id, opts) do + def exchange_code(%Config{} = config, opts) do client_id = Keyword.fetch!(opts, :client_id) client_jwk = Keyword.fetch!(opts, :client_jwk) redirect_uri = Keyword.fetch!(opts, :redirect_uri) @@ -135,10 +135,10 @@ defmodule Latch.Flow do ] end - with {:ok, body} <- dpop_request(config, session_id, token_endpoint, build_form, dpop_key), + with {:ok, body} <- dpop_request(config, token_endpoint, build_form, dpop_key), {:ok, tokens} <- parse_token_response(body), :ok <- verify_sub(tokens.sub, expected_did) do - {:ok, build_session(tokens, issuer, pds_endpoint, dpop_key, session_id, now)} + {:ok, build_session(tokens, issuer, pds_endpoint, dpop_key, now)} end end @@ -156,7 +156,7 @@ defmodule Latch.Flow do ## Optional - `:now` - base time for `expires_at` (defaults to the current time) """ - @spec refresh(Config.t(), String.t(), ServerMetadata.t(), Session.t(), keyword()) :: + @spec refresh(Config.t(), ServerMetadata.t(), Session.t(), keyword()) :: {:ok, Session.t()} | {:error, InvalidResponse.t() @@ -164,7 +164,7 @@ defmodule Latch.Flow do | OAuth.t() | SecurityViolation.t() | Transport.t()} - def refresh(config, session_id, %ServerMetadata{} = server, %Session{} = session, opts) do + def refresh(config, %ServerMetadata{} = server, %Session{} = session, opts) do client_id = Keyword.fetch!(opts, :client_id) client_jwk = Keyword.fetch!(opts, :client_jwk) now = Keyword.get_lazy(opts, :now, &DateTime.utc_now/0) @@ -181,7 +181,7 @@ defmodule Latch.Flow do with :ok <- verify_refresh_issuer(server.issuer, session.issuer), {:ok, body} <- - dpop_request(config, session_id, server.token_endpoint, build_form, session.dpop_key), + dpop_request(config, server.token_endpoint, build_form, session.dpop_key), {:ok, tokens} <- parse_token_response(body), :ok <- verify_sub(tokens.sub, session.did) do {:ok, @@ -190,32 +190,32 @@ defmodule Latch.Flow do server.issuer, session.pds_endpoint, session.dpop_key, - session.session_id, now )} end end - defp dpop_request(config, session_id, url, build_form, dpop_key) do + defp dpop_request(config, url, build_form, dpop_key) do origin = origin(url) + thumbprint = JOSE.JWK.thumbprint(dpop_key) nonce = - case Latch.NonceCache.get_nonce(config, session_id, origin) do + case Latch.NonceCache.get_nonce(config, thumbprint, origin) do {:ok, nonce} -> nonce :error -> nil end - send_dpop(config, session_id, url, build_form, dpop_key, origin, nonce) + send_dpop(config, url, build_form, dpop_key, origin, thumbprint, nonce) end - defp send_dpop(config, session_id, url, build_form, dpop_key, origin, nonce) do + defp send_dpop(config, url, build_form, dpop_key, origin, thumbprint, nonce) do proof = DPoP.proof(dpop_key, "POST", url, nonce: nonce) with {:ok, %{status: status, body: raw, headers: headers}} <- HTTP.post_form(url, build_form.(), [{"dpop", proof}]), {:ok, body} <- decode_json(raw) do if fresh = nonce_header(headers) do - Latch.NonceCache.put_nonce(config, session_id, origin, fresh) + Latch.NonceCache.put_nonce(config, thumbprint, origin, fresh) end cond do @@ -223,7 +223,7 @@ defmodule Latch.Flow do {:ok, body} retry_nonce?(body, nonce) -> - retry_with_nonce(config, session_id, url, build_form, dpop_key, origin, headers) + retry_with_nonce(config, url, build_form, dpop_key, origin, thumbprint, headers) is_binary(Map.get(body, "error")) -> {:error, @@ -239,9 +239,9 @@ defmodule Latch.Flow do end end - defp retry_with_nonce(config, session_id, url, build_form, dpop_key, origin, headers) do + defp retry_with_nonce(config, url, build_form, dpop_key, origin, thumbprint, headers) do if nonce = nonce_header(headers) do - send_dpop(config, session_id, url, build_form, dpop_key, origin, nonce) + send_dpop(config, url, build_form, dpop_key, origin, thumbprint, nonce) else {:error, %MissingDPoPNonce{}} end @@ -307,7 +307,7 @@ defmodule Latch.Flow do defp verify_sub(sub, sub), do: :ok defp verify_sub(_sub, _expected), do: {:error, %SecurityViolation{reason: :did_mismatch}} - defp build_session(tokens, issuer, pds_endpoint, dpop_key, session_id, now) do + defp build_session(tokens, issuer, pds_endpoint, dpop_key, now) do %Session{ did: tokens.sub, access_token: tokens.access_token, @@ -316,8 +316,7 @@ defmodule Latch.Flow do scope: tokens.scope, issuer: issuer, pds_endpoint: pds_endpoint, - expires_at: DateTime.add(now, tokens.expires_in, :second), - session_id: session_id + expires_at: DateTime.add(now, tokens.expires_in, :second) } end end diff --git a/lib/latch/nonce_cache.ex b/lib/latch/nonce_cache.ex index d3712cc..dc45656 100644 --- a/lib/latch/nonce_cache.ex +++ b/lib/latch/nonce_cache.ex @@ -14,8 +14,8 @@ defmodule Latch.NonceCache do @default_sweep_interval :timer.minutes(2) @default_ttl_ms :timer.minutes(5) - def get_nonce(config, session_id, origin) do - key = {session_id, origin} + def get_nonce(config, dpop_thumbprint, origin) do + key = {dpop_thumbprint, origin} case :ets.lookup(table(config), key) do [{^key, nonce, expires_at}] -> @@ -30,9 +30,9 @@ defmodule Latch.NonceCache do end end - def put_nonce(config, session_id, origin, nonce, ttl_ms \\ @default_ttl_ms) do + def put_nonce(config, dpop_thumbprint, origin, nonce, ttl_ms \\ @default_ttl_ms) do expires_at = System.monotonic_time(:millisecond) + ttl_ms - :ets.insert(table(config), {{session_id, origin}, nonce, expires_at}) + :ets.insert(table(config), {{dpop_thumbprint, origin}, nonce, expires_at}) :ok end diff --git a/lib/latch/request.ex b/lib/latch/request.ex index 27b74b7..3d1ae58 100644 --- a/lib/latch/request.ex +++ b/lib/latch/request.ex @@ -9,8 +9,7 @@ defmodule Latch.Request do :issuer, :token_endpoint, :pkce_verifier, - :dpop_key, - :session_id + :dpop_key ] defstruct @enforce_keys @@ -22,7 +21,6 @@ defmodule Latch.Request do issuer: String.t(), token_endpoint: String.t(), pkce_verifier: String.t(), - dpop_key: JOSE.JWK.t(), - session_id: String.t() + dpop_key: JOSE.JWK.t() } end diff --git a/lib/latch/session.ex b/lib/latch/session.ex index bbe27d9..4bb72bd 100644 --- a/lib/latch/session.ex +++ b/lib/latch/session.ex @@ -17,8 +17,7 @@ defmodule Latch.Session do :scope, :issuer, :pds_endpoint, - :expires_at, - :session_id + :expires_at ] defstruct @enforce_keys @@ -30,7 +29,6 @@ defmodule Latch.Session do scope: String.t(), issuer: String.t(), pds_endpoint: String.t(), - expires_at: DateTime.t(), - session_id: String.t() + expires_at: DateTime.t() } end diff --git a/lib/latch/xrpc.ex b/lib/latch/xrpc.ex index 9c3c864..7f6727e 100644 --- a/lib/latch/xrpc.ex +++ b/lib/latch/xrpc.ex @@ -60,17 +60,27 @@ defmodule Latch.XRPC do defp request(%Config{} = config, %Session{} = session, http_method, url, body) do origin = origin(url) + thumbprint = JOSE.JWK.thumbprint(session.dpop_key) nonce = - case Latch.NonceCache.get_nonce(config, session.session_id, origin) do + case Latch.NonceCache.get_nonce(config, thumbprint, origin) do {:ok, nonce} -> nonce :error -> nil end - send_dpop(config, session, http_method, url, body, origin, nonce) + send_dpop(config, session, http_method, url, body, origin, thumbprint, nonce) end - defp send_dpop(%Config{} = config, %Session{} = session, http_method, url, body, origin, nonce) do + defp send_dpop( + %Config{} = config, + %Session{} = session, + http_method, + url, + body, + origin, + thumbprint, + nonce + ) do proof = DPoP.proof(session.dpop_key, http_method, url, nonce: nonce, @@ -82,11 +92,20 @@ defmodule Latch.XRPC do with {:ok, %{status: status, body: raw, headers: resp_headers}} <- HTTP.request(http_method, url, headers, body) do if fresh = nonce_header(resp_headers) do - Latch.NonceCache.put_nonce(config, session.session_id, origin, fresh) + Latch.NonceCache.put_nonce(config, thumbprint, origin, fresh) end if needs_nonce?(status, resp_headers) do - retry_with_nonce(config, session, http_method, url, body, origin, resp_headers) + retry_with_nonce( + config, + session, + http_method, + url, + body, + origin, + thumbprint, + resp_headers + ) else handle_response(status, raw) end @@ -110,10 +129,11 @@ defmodule Latch.XRPC do url, body, origin, + thumbprint, headers ) do if nonce = nonce_header(headers) do - send_dpop(config, session, http_method, url, body, origin, nonce) + send_dpop(config, session, http_method, url, body, origin, thumbprint, nonce) else {:error, %MissingDPoPNonce{}} end diff --git a/test/latch/client_test.exs b/test/latch/client_test.exs index 1112f3f..2975298 100644 --- a/test/latch/client_test.exs +++ b/test/latch/client_test.exs @@ -35,7 +35,7 @@ defmodule Latch.ClientTest do expect(Discovery, :discover, fn @pds -> {:ok, server} end) - expect(Flow, :refresh, fn _config, _session_id, ^server, ^stale_session, opts -> + expect(Flow, :refresh, fn _config, ^server, ^stale_session, opts -> assert opts[:client_id] == config.client_id assert opts[:client_jwk] == config.signing_key {:ok, refreshed_session} @@ -79,7 +79,7 @@ defmodule Latch.ClientTest do expect(Discovery, :discover, fn @pds -> {:ok, server} end) - expect(Flow, :refresh, fn _config, _session_id, ^server, ^stale_session, _opts -> + expect(Flow, :refresh, fn _config, ^server, ^stale_session, _opts -> {:ok, refreshed_session} end) @@ -99,8 +99,7 @@ defmodule Latch.ClientTest do scope: "atproto", issuer: @issuer, pds_endpoint: @pds, - expires_at: expires_at, - session_id: "random 32 chars" + expires_at: expires_at } end diff --git a/test/latch/flow_test.exs b/test/latch/flow_test.exs index 037737d..a6f1b8a 100644 --- a/test/latch/flow_test.exs +++ b/test/latch/flow_test.exs @@ -27,8 +27,6 @@ defmodule Latch.FlowTest do {Latch.NonceCache, config: config, name: config.name, sweep_disabled: true} ) - session_id = "session-id" - client_jwk = JOSE.JWK.generate_key({:ec, "P-256"}) dpop_key = JOSE.JWK.generate_key({:ec, "P-256"}) @@ -58,7 +56,7 @@ defmodule Latch.FlowTest do end) assert {:ok, session} = - Flow.exchange_code(config, session_id, + Flow.exchange_code(config, client_id: "https://client.example.com/oauth-client-metadata.json", client_jwk: client_jwk, redirect_uri: "https://client.example.com/oauth/callback", @@ -69,8 +67,7 @@ defmodule Latch.FlowTest do pds_endpoint: "https://pds.example.com", issuer: "https://issuer.example.com", token_endpoint: "https://issuer.example.com/oauth/token", - now: ~U[2026-01-01 00:00:00Z], - session_id: "random 32 chars" + now: ~U[2026-01-01 00:00:00Z] ) assert session.did == did @@ -99,8 +96,6 @@ defmodule Latch.FlowTest do {Latch.NonceCache, config: config, name: config.name, sweep_disabled: true} ) - session_id = "session-id" - server = %ServerMetadata{ issuer: "https://issuer.example.com", authorization_endpoint: "https://issuer.example.com/oauth/authorize", @@ -126,7 +121,7 @@ defmodule Latch.FlowTest do end) assert {:ok, "urn:ietf:params:oauth:request_uri:request"} = - Flow.par(config, session_id, server, + Flow.par(config, server, client_id: "https://client.example.com/oauth-client-metadata.json", client_jwk: client_jwk, redirect_uri: "https://client.example.com/oauth/callback", @@ -156,8 +151,6 @@ defmodule Latch.FlowTest do {Latch.NonceCache, config: config, name: config.name, sweep_disabled: true} ) - session_id = "session-id" - server = %ServerMetadata{ issuer: "https://other.example.com", authorization_endpoint: "https://other.example.com/oauth/authorize", @@ -174,12 +167,11 @@ defmodule Latch.FlowTest do scope: "atproto", issuer: "https://issuer.example.com", pds_endpoint: "https://pds.example.com", - expires_at: ~U[2026-01-01 00:00:00Z], - session_id: "random 32 chars" + expires_at: ~U[2026-01-01 00:00:00Z] } assert {:error, %SecurityViolation{reason: :issuer_mismatch}} = - Flow.refresh(config, session_id, server, session, + Flow.refresh(config, server, session, client_id: "client-id", client_jwk: nil ) diff --git a/test/latch_test.exs b/test/latch_test.exs index b4f1b60..535a7c3 100644 --- a/test/latch_test.exs +++ b/test/latch_test.exs @@ -37,7 +37,7 @@ defmodule LatchTest do expect(Identity, :resolve_handle, fn @handle -> {:ok, identity} end) expect(Discovery, :discover, fn @pds -> {:ok, server} end) - expect(Flow, :par, fn _config, _session_id, ^server, opts -> + expect(Flow, :par, fn _config, ^server, opts -> assert opts[:client_id] == config.client_id assert opts[:redirect_uri] == config.redirect_uri assert opts[:scope] == "atproto" @@ -100,8 +100,7 @@ defmodule LatchTest do issuer: @issuer, token_endpoint: @issuer <> "/oauth/token", pkce_verifier: "pkce-verifier", - dpop_key: dpop_key, - session_id: "32-random-characters" + dpop_key: dpop_key } session = %Session{ @@ -112,13 +111,12 @@ defmodule LatchTest do scope: "atproto", issuer: @issuer, pds_endpoint: @pds, - expires_at: ~U[2026-01-01 01:00:00Z], - session_id: "32 random chars" + expires_at: ~U[2026-01-01 01:00:00Z] } :ok = Latch.TestStore.put_request(state, request, 600) - expect(Flow, :exchange_code, fn _config, _session_id, opts -> + expect(Flow, :exchange_code, fn _config, opts -> assert opts[:code] == code assert opts[:code_verifier] == "pkce-verifier" assert opts[:dpop_key] == dpop_key @@ -164,8 +162,7 @@ defmodule LatchTest do scope: "atproto", issuer: @issuer, pds_endpoint: @pds, - expires_at: ~U[2026-01-01 00:00:00Z], - session_id: "32 random chars" + expires_at: ~U[2026-01-01 00:00:00Z] } refreshed_session = %{session | access_token: "refreshed-access-token"} @@ -173,7 +170,7 @@ defmodule LatchTest do expect(Discovery, :discover, fn @pds -> {:ok, server} end) - expect(Flow, :refresh, fn _config, _session_id, ^server, ^session, opts -> + expect(Flow, :refresh, fn _config, ^server, ^session, opts -> assert opts[:client_id] == config.client_id assert opts[:client_jwk] == config.signing_key {:ok, refreshed_session} -- 2.51.2