From 6a88d4651be8deeb06ed25e8b755cb86d1a70e4d Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 7 Oct 2026 12:10:14 +0200 Subject: [PATCH 1/2] fix(auth): take the member email from the UserInfo endpoint better-auth 1.7 stopped putting user-record claims in the id_token and serves them from the UserInfo endpoint instead (OIDC Core section 5.4), so the callback now fetches GET /api/auth/oauth2/userinfo with the access token after the code exchange and resolves identity from that response. Email comes from userinfo only; a blank one fails the callback with :missing_email, keeping the previous commit's guard as the integrity floor so no member is ever keyed on `sub`. Name falls back from userinfo to the id_token name, then to the email address. The id_token remains the source of `sub` and `github_id` in `extra.raw_info`. --- lib/omniauth/strategies/codebar.rb | 36 ++++- spec/lib/omniauth/strategies/codebar_spec.rb | 151 ++++++++++++++----- 2 files changed, 144 insertions(+), 43 deletions(-) diff --git a/lib/omniauth/strategies/codebar.rb b/lib/omniauth/strategies/codebar.rb index 6a344fa5e..5ed9aabb3 100644 --- a/lib/omniauth/strategies/codebar.rb +++ b/lib/omniauth/strategies/codebar.rb @@ -83,12 +83,14 @@ 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'] + # The planner resolves members by email, and since better-auth 1.7 the + # id_token is sparse: only the UserInfo response carries the email. A + # blank one must fail the callback — keying a member on `sub` (the + # better-auth user id) creates an account with no subscriptions or roles. + userinfo = fetch_userinfo(tokens['access_token']) + email = userinfo&.dig('email') if email.blank? - return fail!(:missing_email, StandardError.new('id_token has no email claim')) + return fail!(:missing_email, StandardError.new('UserInfo response has no email')) end # Build omniauth.auth hash @@ -97,7 +99,7 @@ def callback_phase uid: email, info: { email:, - name: payload['name'] || email + name: userinfo['name'].presence || payload['name'].presence || email }, credentials: { token: tokens['access_token'], @@ -171,6 +173,28 @@ def exchange_code(code, code_verifier) nil end + # Fetch the OIDC UserInfo response for an access token. The scope-gated + # claims there (`email`, `name`) are the planner's identity source because + # the id_token no longer carries them. + def fetch_userinfo(access_token) + uri = URI("#{options.auth_url}/api/auth/oauth2/userinfo") + request = Net::HTTP::Get.new(uri.path) + request['Authorization'] = "Bearer #{access_token}" + request['User-Agent'] = 'Codebar Planner/1.0' + + response = http_for(uri).request(request) + + if response.code.to_i == 200 + JSON.parse(response.body) + else + Rails.logger.warn "Codebar auth: userinfo fetch returned HTTP #{response.code}" + nil + end + rescue Net::OpenTimeout, Net::ReadTimeout, SocketError, Errno::ECONNREFUSED, JSON::ParserError => e + Rails.logger.warn "Codebar auth: userinfo fetch failed: #{e.class}: #{e.message}" + nil + end + # Verify JWT signature using auth app's JWKS. def verify_jwt(token) jwks = fetch_jwks diff --git a/spec/lib/omniauth/strategies/codebar_spec.rb b/spec/lib/omniauth/strategies/codebar_spec.rb index a15ab3e68..462fc0516 100644 --- a/spec/lib/omniauth/strategies/codebar_spec.rb +++ b/spec/lib/omniauth/strategies/codebar_spec.rb @@ -190,17 +190,27 @@ 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(:userinfo_url) { "#{auth_url}/api/auth/oauth2/userinfo" } + # Sparse id_token: since better-auth 1.7 the email and name live in the + # UserInfo response, not in the token. let(:token_payload) do { 'sub' => 'better-auth-user-id', - 'email' => email, - 'name' => name, + 'github_id' => '4242', 'iss' => auth_url, 'aud' => 'planner', 'iat' => Time.now.to_i, 'exp' => Time.now.to_i + 3600 } end + let(:userinfo_body) do + { + 'sub' => 'better-auth-user-id', + 'email' => email, + 'email_verified' => true, + 'name' => name + } + end let(:id_token) do JWT.encode(token_payload, rsa_key, 'RS256', { kid: 'test-key-1' }) end @@ -224,9 +234,13 @@ def build_env(path, query: '', session: {}) stub_request(:get, jwks_url) .with(headers: { 'User-Agent' => 'Codebar Planner/1.0' }) .to_return(status: 200, body: { keys: [jwk.export] }.to_json, headers: { 'Content-Type' => 'application/json' }) + + stub_request(:get, userinfo_url) + .with(headers: { 'Authorization' => 'Bearer test-access-token', 'User-Agent' => 'Codebar Planner/1.0' }) + .to_return(status: 200, body: userinfo_body.to_json, headers: { 'Content-Type' => 'application/json' }) end - it 'builds the auth hash with correct data' do + it 'builds the auth hash from the userinfo response' do strategy.call!(callback_env) auth_hash = callback_env['omniauth.auth'] @@ -236,70 +250,133 @@ def build_env(path, query: '', session: {}) 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' => 'better-auth-user-id', 'email' => email, 'name' => name) + expect(auth_hash[:extra][:raw_info]).to include('sub' => 'better-auth-user-id', 'github_id' => '4242') 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 + describe 'email source' do + it 'takes the email from userinfo even when the id_token carries a different one' do + stub_request(:post, token_url) + .to_return(status: 200, body: { + access_token: 'test-access-token', + id_token: JWT.encode(token_payload.merge('email' => 'token-level@example.com'), rsa_key, 'RS256', { kid: 'test-key-1' }), + token_type: 'Bearer', + expires_in: 900 + }.to_json, headers: { 'Content-Type' => 'application/json' }) - 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) + expect(callback_env['omniauth.auth'][:uid]).to eq(email) + expect(callback_env['omniauth.auth'][:info][:email]).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 - } + it 'fails with missing_email when userinfo has no email, ignoring the id_token claim' do + stub_request(:post, token_url) + .to_return(status: 200, body: { + access_token: 'test-access-token', + id_token: JWT.encode(token_payload.merge('email' => 'token-level@example.com'), rsa_key, 'RS256', { kid: 'test-key-1' }), + token_type: 'Bearer', + expires_in: 900 + }.to_json, headers: { 'Content-Type' => 'application/json' }) + stub_request(:get, userinfo_url) + .to_return(status: 200, body: { 'sub' => 'better-auth-user-id' }.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 - it 'fails with missing_email and builds no auth hash' do + it 'fails with missing_email when the userinfo email is blank' do + stub_request(:get, userinfo_url) + .to_return(status: 200, body: userinfo_body.merge('email' => '').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 - it 'returns the failure response from the middleware instead of a nil Rack response' do - response = strategy.call(callback_env) + describe 'name source' do + it 'falls back to the id_token name when userinfo has none' do + stub_request(:post, token_url) + .to_return(status: 200, body: { + access_token: 'test-access-token', + id_token: JWT.encode(token_payload.merge('name' => name), rsa_key, 'RS256', { kid: 'test-key-1' }), + token_type: 'Bearer', + expires_in: 900 + }.to_json, headers: { 'Content-Type' => 'application/json' }) + stub_request(:get, userinfo_url) + .to_return(status: 200, body: userinfo_body.except('name').to_json, headers: { 'Content-Type' => 'application/json' }) - expect(response).to be_a(Array) - expect(response[0]).to eq(302) - expect(response[1]['Location']).to start_with('/auth/failure?') + strategy.call!(callback_env) + + expect(callback_env['omniauth.auth'][:info][:name]).to eq(name) + end + + it 'falls back to the email when neither carries a name' do + stub_request(:get, userinfo_url) + .to_return(status: 200, body: userinfo_body.except('name').to_json, headers: { 'Content-Type' => 'application/json' }) + + strategy.call!(callback_env) + + expect(callback_env['omniauth.auth'][:info][:name]).to eq(email) end - it 'also fails when the email claim is present but blank' do + it 'treats a blank userinfo name as missing' 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' }), + id_token: JWT.encode(token_payload.merge('name' => name), rsa_key, 'RS256', { kid: 'test-key-1' }), token_type: 'Bearer', expires_in: 900 }.to_json, headers: { 'Content-Type' => 'application/json' }) + stub_request(:get, userinfo_url) + .to_return(status: 200, body: userinfo_body.merge('name' => '').to_json, headers: { 'Content-Type' => 'application/json' }) + + strategy.call!(callback_env) + + expect(callback_env['omniauth.auth'][:info][:name]).to eq(name) + end + end + + describe 'when the userinfo request fails' do + it 'fails with missing_email when userinfo returns 500' do + stub_request(:get, userinfo_url).to_return(status: 500) + + strategy.call!(callback_env) + + expect(callback_env['omniauth.error.type']).to eq(:missing_email) + expect(callback_env['omniauth.auth']).to be_nil + end + + it 'fails with missing_email when userinfo times out' do + stub_request(:get, userinfo_url).to_timeout + + strategy.call!(callback_env) + + expect(callback_env['omniauth.error.type']).to eq(:missing_email) + expect(callback_env['omniauth.auth']).to be_nil + end + + it 'fails with missing_email when userinfo returns invalid JSON' do + stub_request(:get, userinfo_url).to_return(status: 200, body: 'not json') 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 + stub_request(:get, userinfo_url).to_return(status: 500) + + 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 end end From 49dcefd32073e14592bf00905c6d8e7b33ce7436 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Wed, 7 Oct 2026 13:21:58 +0200 Subject: [PATCH 2/2] fix(auth): distinguish userinfo request failures from a missing email A userinfo transport failure, non-200 response, or malformed body now fails the callback with :userinfo_failed instead of :missing_email, so a degraded auth app is distinguishable from a member without an email. The rescue list gains the common Net::HTTP and OpenSSL transport errors (connection reset, unreachable host, SSL failure) that previously escaped to :unknown_error, and a valid-JSON non-object body counts as a request failure instead of raising. --- lib/omniauth/strategies/codebar.rb | 26 +++++++---- spec/lib/omniauth/strategies/codebar_spec.rb | 48 +++++++++++++++++--- 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/lib/omniauth/strategies/codebar.rb b/lib/omniauth/strategies/codebar.rb index 5ed9aabb3..4a52bd314 100644 --- a/lib/omniauth/strategies/codebar.rb +++ b/lib/omniauth/strategies/codebar.rb @@ -83,12 +83,17 @@ def callback_phase return fail!(:invalid_jwt, StandardError.new('JWT verification failed')) end - # The planner resolves members by email, and since better-auth 1.7 the - # id_token is sparse: only the UserInfo response carries the email. A - # blank one must fail the callback — keying a member on `sub` (the - # better-auth user id) creates an account with no subscriptions or roles. + # The planner resolves members by email, and since better-auth 1.7 only + # the UserInfo response carries it. A failed request fails the callback + # as :userinfo_failed (provider trouble) and a blank email as + # :missing_email: keying a member on `sub` (the better-auth user id) + # creates an account with no subscriptions or roles. userinfo = fetch_userinfo(tokens['access_token']) - email = userinfo&.dig('email') + if userinfo.nil? + return fail!(:userinfo_failed, StandardError.new('UserInfo request failed')) + end + + email = userinfo['email'] if email.blank? return fail!(:missing_email, StandardError.new('UserInfo response has no email')) end @@ -185,12 +190,17 @@ def fetch_userinfo(access_token) response = http_for(uri).request(request) if response.code.to_i == 200 - JSON.parse(response.body) + parsed = JSON.parse(response.body) + return parsed if parsed.is_a?(Hash) + + Rails.logger.warn 'Codebar auth: userinfo response is not a JSON object' else Rails.logger.warn "Codebar auth: userinfo fetch returned HTTP #{response.code}" - nil end - rescue Net::OpenTimeout, Net::ReadTimeout, SocketError, Errno::ECONNREFUSED, JSON::ParserError => e + nil + rescue Net::OpenTimeout, Net::ReadTimeout, SocketError, Errno::ECONNREFUSED, + Errno::ECONNRESET, Errno::EHOSTUNREACH, Errno::ENETUNREACH, Errno::ETIMEDOUT, + Errno::EPIPE, EOFError, OpenSSL::SSL::SSLError, Net::HTTPBadResponse, JSON::ParserError => e Rails.logger.warn "Codebar auth: userinfo fetch failed: #{e.class}: #{e.message}" nil end diff --git a/spec/lib/omniauth/strategies/codebar_spec.rb b/spec/lib/omniauth/strategies/codebar_spec.rb index 462fc0516..4ab271192 100644 --- a/spec/lib/omniauth/strategies/codebar_spec.rb +++ b/spec/lib/omniauth/strategies/codebar_spec.rb @@ -341,30 +341,66 @@ def build_env(path, query: '', session: {}) end describe 'when the userinfo request fails' do - it 'fails with missing_email when userinfo returns 500' do + it 'fails with userinfo_failed when userinfo returns 500' do stub_request(:get, userinfo_url).to_return(status: 500) strategy.call!(callback_env) - expect(callback_env['omniauth.error.type']).to eq(:missing_email) + expect(callback_env['omniauth.error.type']).to eq(:userinfo_failed) + expect(callback_env['omniauth.auth']).to be_nil + end + + it 'fails with userinfo_failed when userinfo rejects the access token' do + stub_request(:get, userinfo_url).to_return(status: 401) + + strategy.call!(callback_env) + + expect(callback_env['omniauth.error.type']).to eq(:userinfo_failed) expect(callback_env['omniauth.auth']).to be_nil end - it 'fails with missing_email when userinfo times out' do + it 'fails with userinfo_failed when userinfo times out' do stub_request(:get, userinfo_url).to_timeout strategy.call!(callback_env) - expect(callback_env['omniauth.error.type']).to eq(:missing_email) + expect(callback_env['omniauth.error.type']).to eq(:userinfo_failed) + expect(callback_env['omniauth.auth']).to be_nil + end + + it 'fails with userinfo_failed when the connection resets' do + stub_request(:get, userinfo_url).to_raise(Errno::ECONNRESET) + + strategy.call!(callback_env) + + expect(callback_env['omniauth.error.type']).to eq(:userinfo_failed) expect(callback_env['omniauth.auth']).to be_nil end - it 'fails with missing_email when userinfo returns invalid JSON' do + it 'fails with userinfo_failed when userinfo returns invalid JSON' do stub_request(:get, userinfo_url).to_return(status: 200, body: 'not json') strategy.call!(callback_env) - expect(callback_env['omniauth.error.type']).to eq(:missing_email) + expect(callback_env['omniauth.error.type']).to eq(:userinfo_failed) + expect(callback_env['omniauth.auth']).to be_nil + end + + it 'fails with userinfo_failed when the userinfo body is an array instead of an object' do + stub_request(:get, userinfo_url).to_return(status: 200, body: '[]', headers: { 'Content-Type' => 'application/json' }) + + strategy.call!(callback_env) + + expect(callback_env['omniauth.error.type']).to eq(:userinfo_failed) + expect(callback_env['omniauth.auth']).to be_nil + end + + it 'fails with userinfo_failed when the userinfo body is null instead of an object' do + stub_request(:get, userinfo_url).to_return(status: 200, body: 'null', headers: { 'Content-Type' => 'application/json' }) + + strategy.call!(callback_env) + + expect(callback_env['omniauth.error.type']).to eq(:userinfo_failed) expect(callback_env['omniauth.auth']).to be_nil end