From 16e4692644b498cda79c435ad3f4409fab9083cc Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 7 Oct 2026 08:24:29 +0200 Subject: [PATCH] fix(auth): fail the codebar callback when the id_token has no email claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the codebar OmniAuth strategy received an id_token without an `email` claim, it substituted `payload['sub']` (the better-auth user id) for the member email, so AuthServicesController created a member keyed on an opaque user id with no subscriptions or roles — the duplicate-member mechanism behind the 2026-10-02 incident (members 31450 and 31452). The strategy now fails the callback with `:missing_email` before building the auth hash. No Member, AuthService, or activity row is created; the standard OmniAuth failure redirect to `/auth/failure` and the generic "Authentication failed" flash surface the failure. The guard also covers a present-but-blank email claim. The successful-callback spec's token now carries a distinct `sub` and an `email` claim, so `uid` proves to come from the email, not the sub fallback — that spec previously passed only because of the fallback. New specs pin the guard, the middleware 302 to `/auth/failure?`, and the blank-claim boundary. Related: codebar/auth#83 restores the claims provider-side. This is the planner-side integrity guard; the sub-keyed cleanup tooling ships in #2987 and its production run waits for this deploy. --- lib/omniauth/strategies/codebar.rb | 9 +- spec/lib/omniauth/strategies/codebar_spec.rb | 96 +++++++++++++++++--- 2 files changed, 92 insertions(+), 13 deletions(-) diff --git a/lib/omniauth/strategies/codebar.rb b/lib/omniauth/strategies/codebar.rb index f8af0f171..6a344fa5e 100644 --- a/lib/omniauth/strategies/codebar.rb +++ b/lib/omniauth/strategies/codebar.rb @@ -83,8 +83,15 @@ def callback_phase return fail!(:invalid_jwt, StandardError.new('JWT verification failed')) end + # The planner resolves members by email; a token without an email claim + # must not fall back to `sub` (the better-auth user id) — keying a member + # on it creates an account with no subscriptions or roles. + email = payload['email'] + if email.blank? + return fail!(:missing_email, StandardError.new('id_token has no email claim')) + end + # Build omniauth.auth hash - email = payload['email'] || payload['sub'] @env['omniauth.auth'] = AuthHash.new({ provider: name, uid: email, diff --git a/spec/lib/omniauth/strategies/codebar_spec.rb b/spec/lib/omniauth/strategies/codebar_spec.rb index c0e614409..a15ab3e68 100644 --- a/spec/lib/omniauth/strategies/codebar_spec.rb +++ b/spec/lib/omniauth/strategies/codebar_spec.rb @@ -190,13 +190,25 @@ def build_env(path, query: '', session: {}) describe 'successful callback' do let(:rsa_key) { OpenSSL::PKey::RSA.generate(2048) } let(:jwk) { JWT::JWK.new(rsa_key, { kid: 'test-key-1' }) } + let(:token_payload) do + { + 'sub' => 'better-auth-user-id', + 'email' => email, + 'name' => name, + 'iss' => auth_url, + 'aud' => 'planner', + 'iat' => Time.now.to_i, + 'exp' => Time.now.to_i + 3600 + } + end let(:id_token) do - JWT.encode( - { 'sub' => email, 'name' => name, 'iss' => auth_url, 'aud' => 'planner', 'iat' => Time.now.to_i, 'exp' => Time.now.to_i + 3600 }, - rsa_key, - 'RS256', - { kid: 'test-key-1' } - ) + JWT.encode(token_payload, rsa_key, 'RS256', { kid: 'test-key-1' }) + end + + let(:callback_env) do + build_env('/auth/codebar/callback', + query: 'code=abc&state=some-state', + session: { 'omniauth.codebar.state' => 'some-state', 'omniauth.codebar.code_verifier' => 'verifier', 'omniauth.codebar.redirect_uri' => 'http://localhost:3000/auth/codebar/callback' }) end before do @@ -215,19 +227,79 @@ def build_env(path, query: '', session: {}) end it 'builds the auth hash with correct data' do - env = build_env('/auth/codebar/callback', - query: 'code=abc&state=some-state', - session: { 'omniauth.codebar.state' => 'some-state', 'omniauth.codebar.code_verifier' => 'verifier', 'omniauth.codebar.redirect_uri' => 'http://localhost:3000/auth/codebar/callback' }) - strategy.call!(env) + strategy.call!(callback_env) - auth_hash = env['omniauth.auth'] + auth_hash = callback_env['omniauth.auth'] expect(auth_hash).to be_present expect(auth_hash[:provider]).to eq('codebar') expect(auth_hash[:uid]).to eq(email) expect(auth_hash[:info][:email]).to eq(email) expect(auth_hash[:info][:name]).to eq(name) expect(auth_hash[:credentials][:token]).to eq('test-access-token') - expect(auth_hash[:extra][:raw_info]).to include('sub' => email, 'name' => name) + expect(auth_hash[:extra][:raw_info]).to include('sub' => 'better-auth-user-id', 'email' => email, 'name' => name) + end + + describe 'with no name claim' do + let(:token_payload) do + { + 'sub' => 'better-auth-user-id', + 'email' => email, + 'iss' => auth_url, + 'aud' => 'planner', + 'iat' => Time.now.to_i, + 'exp' => Time.now.to_i + 3600 + } + end + + it 'falls back to the email for name' do + strategy.call!(callback_env) + + expect(callback_env['omniauth.error.type']).to be_nil + expect(callback_env['omniauth.auth'][:info][:name]).to eq(email) + end + end + + describe 'with no email claim' do + let(:token_payload) do + { + 'sub' => 'better-auth-user-id', + 'name' => name, + 'iss' => auth_url, + 'aud' => 'planner', + 'iat' => Time.now.to_i, + 'exp' => Time.now.to_i + 3600 + } + end + + it 'fails with missing_email and builds no auth hash' do + strategy.call!(callback_env) + + expect(callback_env['omniauth.error.type']).to eq(:missing_email) + expect(callback_env['omniauth.auth']).to be_nil + end + + it 'returns the failure response from the middleware instead of a nil Rack response' do + response = strategy.call(callback_env) + + expect(response).to be_a(Array) + expect(response[0]).to eq(302) + expect(response[1]['Location']).to start_with('/auth/failure?') + end + + it 'also fails when the email claim is present but blank' do + stub_request(:post, token_url) + .to_return(status: 200, body: { + access_token: 'test-access-token', + id_token: JWT.encode(token_payload.merge('email' => ''), rsa_key, 'RS256', { kid: 'test-key-1' }), + token_type: 'Bearer', + expires_in: 900 + }.to_json, headers: { 'Content-Type' => 'application/json' }) + + strategy.call!(callback_env) + + expect(callback_env['omniauth.error.type']).to eq(:missing_email) + expect(callback_env['omniauth.auth']).to be_nil + end end end