Fix domain verification error handling using CallbackError

lib/omniauth/microsoft_graph/domain_verifier.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
+
 require 'jwt' # for token signature validation
-require 'omniauth' # to inherit from OmniAuth::Error
+require 'omniauth-oauth2' # to use CallbackError
 require 'oauth2' # to rescue OAuth2::Error
 
 module OmniAuth
@@ -11,8 +12,6 @@ module MicrosoftGraph
     OIDC_CONFIG_URL = 'https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-configuration'
     COMMON_JWKS_URL = 'https://login.microsoftonline.com/common/discovery/v2.0/keys'
 
-    class DomainVerificationError < OmniAuth::Error; end
-
     class DomainVerifier
       def self.verify!(auth_hash, access_token, options)
         new(auth_hash, access_token, options).verify!
@@ -37,11 +36,21 @@ 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 == upn_domain ||
-          skip_verification == true ||
-          (skip_verification.is_a?(Array) && skip_verification.include?(email_domain)) ||
-          domain_verified_jwt_claim
-        raise DomainVerificationError, verification_error_message
+        # return true if email_domain == upn_domain ||
+        #   skip_verification == true ||
+        #   (skip_verification.is_a?(Array) && skip_verification.include?(email_domain))
+
+        # Try domain_verified_jwt_claim check
+        xms_edov_result = check_xms_edov
+        return true if xms_edov_result == :valid
+
+        # If we get here, verification failed - determine why
+        error_key = determine_error_key(xms_edov_result)
+        error_message = build_error_message(xms_edov_result)
+
+        # Use CallbackError to ensure the error is properly caught by the callback_phase
+        # rescue clause and converted to an OmniAuth failure instead of bubbling up as a 500 error.
+        raise OmniAuth::Strategies::OAuth2::CallbackError.new(error_key, error_message)
       end
 
       private
@@ -60,20 +69,81 @@ def verify!
       #
       # To get to it, we need to decode the ID token with the key material from Microsoft's
       # OIDC configuration endpoint, and inspect it for the claim in question.
-      def domain_verified_jwt_claim
+      def check_xms_edov
         oidc_config = access_token.get(OIDC_CONFIG_URL).parsed
         algorithms = oidc_config['id_token_signing_alg_values_supported']
         jwks = get_jwks(oidc_config)
         decoded_token = JWT.decode(id_token, nil, true, algorithms: algorithms, jwks: jwks)
-        xms_edov_valid?(decoded_token)
-      rescue JWT::VerificationError, ::OAuth2::Error
-        false
+
+        xms_edov_value = decoded_token.first['xms_edov']
+
+        if xms_edov_value.nil?
+          :missing
+        elsif ['1', 1, 'true', true].include?(xms_edov_value)
+          :valid
+        else
+          :false
+        end
+      rescue JWT::VerificationError, ::OAuth2::Error => e
+        :error
+      end
+
+      def determine_error_key(xms_edov_result)
+        case xms_edov_result
+        when :missing
+          :domain_verification_xms_edov_missing
+        when :false
+          :domain_verification_xms_edov_false
+        when :error
+          :domain_verification_xms_edov_error
+        else
+          :domain_verification_failed
+        end
       end
 
-      def xms_edov_valid?(decoded_token)
-        # https://github.com/MicrosoftDocs/azure-docs/issues/111425#issuecomment-1761043378
-        # Comments seemed to indicate the value is not consistent
-        ['1', 1, 'true', true].include?(decoded_token.first['xms_edov'])
+      def build_error_message(xms_edov_result)
+        base_msg = "The email domain '#{email_domain}' does not match the principal domain '#{upn_domain}'."
+
+        case xms_edov_result
+        when :missing
+          <<~MSG
+            #{base_msg}
+
+            The xms_edov claim is missing from the token, which could verify your email domain.
+            Please ensure the xms_edov optional claim is configured in your Azure app registration.
+
+            You can either:
+              * Configure the xms_edov optional claim in Azure (recommended)
+              * Update the user's email to match the principal domain '#{upn_domain}'
+              * Skip verification on the '#{email_domain}' domain (not recommended)
+            Refer to the README for more details.
+          MSG
+        when :false
+          <<~MSG
+            #{base_msg}
+
+            The xms_edov claim indicates that '#{email_domain}' is NOT a verified domain in your Azure tenant.
+
+            You can either:
+              * Add '#{email_domain}' as a verified custom domain in Azure Entra ID
+              * Update the user's email to match the principal domain '#{upn_domain}'
+              * Skip verification on the '#{email_domain}' domain (not recommended)
+            Refer to the README for more details.
+          MSG
+        when :error
+          <<~MSG
+            #{base_msg}
+
+            Unable to verify the xms_edov claim (token validation error).
+
+            You can either:
+              * Update the user's email to match the principal domain '#{upn_domain}'
+              * Skip verification on the '#{email_domain}' domain (not recommended)
+            Refer to the README for more details.
+          MSG
+        else
+          verification_error_message
+        end
       end
 
       def get_jwks(oidc_config)

spec/omniauth/microsoft_graph/domain_verifier_spec.rb

@@ -105,8 +105,11 @@
     context 'when all verification strategies fail' do
       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
+      it 'raises a CallbackError with domain_verification_failed' do
+        expect { result }.to raise_error(OmniAuth::Strategies::OAuth2::CallbackError) do |error|
+          expect(error.error).to eq(:domain_verification_failed)
+          expect(error.error_reason).to include('not a verified domain')
+        end
       end
     end
   end

spec/omniauth/strategies/microsoft_graph_oauth2_spec.rb

@@ -282,7 +282,12 @@
 
     context 'when email verification fails' do
       let(:response_hash) { { mail: 'something@domain.invalid' } }
-      let(:error) { OmniAuth::MicrosoftGraph::DomainVerificationError.new }
+      let(:error) do
+        OmniAuth::Strategies::OAuth2::CallbackError.new(
+          :domain_verification_failed,
+          'Domain verification failed'
+        )
+      end
 
       before do
         allow(OmniAuth::MicrosoftGraph::DomainVerifier).to receive(:verify!).and_raise(error)