fix(auth): take the member email from the UserInfo endpoint

lib/omniauth/strategies/codebar.rb

@@ -83,12 +83,19 @@ 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 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'])
+        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('id_token has no email claim'))
+          return fail!(:missing_email, StandardError.new('UserInfo response has no email'))
         end
 
         # Build omniauth.auth hash
@@ -97,7 +104,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 +178,33 @@ 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
+          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}"
+        end
+        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
+
       # Verify JWT signature using auth app's JWKS.
       def verify_jwt(token)
         jwks = fetch_jwks

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,169 @@ 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.error.type']).to eq(:missing_email)
+        expect(callback_env['omniauth.auth'][:info][:name]).to eq(name)
+      end
+    end
+
+    describe 'when the userinfo request fails' 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(: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 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(: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 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(: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
+
+      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