Fix Microsoft OAuth callback for Graph users whose mail field is null

lib/omniauth/microsoft_graph/domain_verifier.rb

@@ -19,8 +19,11 @@ def self.verify!(auth_hash, access_token, options)
       end
 
       def initialize(auth_hash, access_token, options)
-        @email_domain = auth_hash['info']['email']&.split('@')&.last
-        @upn_domain = auth_hash['extra']['raw_info']['userPrincipalName']&.split('@')&.last
+        raw_info = auth_hash['extra']['raw_info'] || {}
+        effective_email = auth_hash['info']['email'] || raw_info['userPrincipalName']
+
+        @email_domain = effective_email&.split('@')&.last
+        @upn_domain = raw_info['userPrincipalName']&.split('@')&.last
         @access_token = access_token
         @id_token = access_token.params['id_token']
         @skip_verification = options[:skip_domain_verification]
@@ -37,9 +40,10 @@ def verify!
         # This means while it's not suitable for consistently identifying a user
         # (the domain might change), it is suitable for verifying membership in
         # a given domain.
-        return true if email_domain.casecmp?(upn_domain) ||
-          skip_verification == true ||
-          (skip_verification.is_a?(Array) && skip_verification.include?(email_domain)) ||
+        return true if skip_verification == true ||
+          email_domain&.casecmp?(upn_domain) ||
+          (email_domain && skip_verification.is_a?(Array) &&
+            skip_verification.include?(email_domain)) ||
           domain_verified_jwt_claim
         raise DomainVerificationError, verification_error_message
       end

lib/omniauth/strategies/microsoft_graph.rb

@@ -30,7 +30,7 @@ class MicrosoftGraph < OmniAuth::Strategies::OAuth2
 
       info do
         {
-          'email' => raw_info["mail"],
+          'email' => raw_info["mail"] || raw_info["userPrincipalName"],
           'first_name' => raw_info["givenName"],
           'last_name' => raw_info["surname"],
           'name' => [raw_info["givenName"], raw_info["surname"]].join(' '),

spec/omniauth/microsoft_graph/domain_verifier_spec.rb

@@ -35,12 +35,27 @@
       it { is_expected.to be_truthy }
     end
 
+    context 'when email is missing and userPrincipalName is present' do
+      let(:email) { nil }
+      let(:upn) { 'bar@example.com' }
+
+      it { is_expected.to be_truthy }
+    end
+
     context 'when domain validation is disabled' do
       let(:options) { super().merge(skip_domain_verification: true) }
 
       it { is_expected.to be_truthy }
     end
 
+    context 'when domain validation is disabled and email is missing' do
+      let(:email) { nil }
+      let(:upn) { nil }
+      let(:options) { super().merge(skip_domain_verification: true) }
+
+      it { is_expected.to be_truthy }
+    end
+
     context 'when the email domain is explicitly permitted' do
       let(:options) { super().merge(skip_domain_verification: ['example.com']) }
 
@@ -116,5 +131,16 @@
         expect { result }.to raise_error OmniAuth::MicrosoftGraph::DomainVerificationError
       end
     end
+
+    context 'when email and userPrincipalName are missing' do
+      let(:email) { nil }
+      let(:upn) { nil }
+
+      before { allow(access_token).to receive(:get).and_raise(::OAuth2::Error.new('whoops')) }
+
+      it 'raises a DomainVerificationError' do
+        expect { result }.to raise_error OmniAuth::MicrosoftGraph::DomainVerificationError
+      end
+    end
   end
 end

spec/omniauth/strategies/microsoft_graph_oauth2_spec.rb

@@ -280,6 +280,16 @@
       end
     end
 
+    context 'when mail is missing and userPrincipalName is present' do
+      let(:response_hash) do
+        { mail: nil, userPrincipalName: 'something@domain.invalid' }
+      end
+
+      it 'falls back to the userPrincipalName' do
+        expect(subject.info['email']).to eq('something@domain.invalid')
+      end
+    end
+
     context 'when email verification fails' do
       let(:response_hash) { { mail: 'something@domain.invalid' } }
       let(:error) { OmniAuth::MicrosoftGraph::DomainVerificationError.new }