diff --git a/README.md b/README.md index db6a238..3705e21 100644 --- a/README.md +++ b/README.md @@ -32,10 +32,43 @@ end ``` #### Login Hint -Just add {login_hint: "email@example.com"} to your url generation to form: +Just add `{login_hint: "email@example.com"}` to your url generation to form: ```ruby /auth/microsoft_graph?login_hint=email@example.com ``` + +#### Domain Verification +Because Microsoft allows users to set vanity emails on their accounts, the value of the user's "email" doesn't establish membership in that domain. Put another way, user malicious@hacker.biz can edit their email in Active Directory to ceo@yourcompany.com, and (depending on your auth implementation) may be able to log in automatically as that user. + +To establish membership in the claimed email domain, we use two strategies: + +* `email` domain matches `userPrincipalName` domain (which by definition is a verified domain) +* The user's `id_token` includes the `xms_edov` ("Email Domain Ownership Verified") claim, with a truthy value + +The `xms_edov` claim is [optional](https://github.com/MicrosoftDocs/azure-docs/issues/111425), and must be configured in the Azure console before it's available in the token. Refer to [Clerk's guide](https://clerk.com/docs/authentication/social-connections/microsoft#stay-secure-against-the-n-o-auth-vulnerability) for instructions on configuring the claim. + +If you're not able or don't need to support domain verification, you can bypass for an individual domain: +```ruby +Rails.application.config.middleware.use OmniAuth::Builder do + provider :microsoft_graph, + ENV['AZURE_APPLICATION_CLIENT_ID'], + ENV['AZURE_APPLICATION_CLIENT_SECRET'], + skip_domain_verification: %w[contoso.com] +end +``` + +Or, you can disable domain verification entirely. We *strongly recommend* that you do *not* disable domain verification if at all possible. +```ruby +Rails.application.config.middleware.use OmniAuth::Builder do + provider :microsoft_graph, + ENV['AZURE_APPLICATION_CLIENT_ID'], + ENV['AZURE_APPLICATION_CLIENT_SECRET'], + skip_domain_verification: true +end +``` + +[nOAuth: How Microsoft OAuth Misconfiguration Can Lead to Full Account Takeover](https://www.descope.com/blog/post/noauth) from [Descope](https://www.descope.com/) + ### Upgrading to 1.0.0 This version requires OmniAuth v2. If you are using Rails, you will need to include or upgrade `omniauth-rails_csrf_protection`. If you upgrade and get an error in your logs complaining about "authenticity error" or similiar, make sure to do `bundle update omniauth-rails_csrf_protection` diff --git a/lib/omniauth/microsoft_graph.rb b/lib/omniauth/microsoft_graph.rb index db2c1be..2d14f3e 100644 --- a/lib/omniauth/microsoft_graph.rb +++ b/lib/omniauth/microsoft_graph.rb @@ -1,2 +1,3 @@ +require "omniauth/microsoft_graph/domain_verifier" require "omniauth/microsoft_graph/version" require "omniauth/strategies/microsoft_graph" diff --git a/lib/omniauth/microsoft_graph/domain_verifier.rb b/lib/omniauth/microsoft_graph/domain_verifier.rb new file mode 100644 index 0000000..6cbdf03 --- /dev/null +++ b/lib/omniauth/microsoft_graph/domain_verifier.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true +require 'jwt' # for token signature validation +require 'omniauth' # to inherit from OmniAuth::Error +require 'oauth2' # to rescue OAuth2::Error + +module OmniAuth + module MicrosoftGraph + # Verify user email domains to mitigate the nOAuth vulnerability + # https://www.descope.com/blog/post/noauth + # https://clerk.com/docs/authentication/social-connections/microsoft#stay-secure-against-the-n-o-auth-vulnerability + OIDC_CONFIG_URL = 'https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-configuration' + + class DomainVerificationError < OmniAuth::Error; end + + class DomainVerifier + def self.verify!(auth_hash, access_token, options) + new(auth_hash, access_token, options).verify! + 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 + @access_token = access_token + @id_token = access_token.params['id_token'] + @skip_verification = options[:skip_domain_verification] + end + + def verify! + # The userPrincipalName property is mutable, but must always contain a + # verified domain: + # + # "The general format is alias@domain, where domain must be present in + # the tenant's collection of verified domains." + # https://learn.microsoft.com/en-us/graph/api/resources/user?view=graph-rest-1.0 + # + # 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 + end + + private + + attr_reader :access_token, + :email_domain, + :id_token, + :permitted_domains, + :skip_verification, + :upn_domain + + # https://learn.microsoft.com/en-us/entra/identity-platform/optional-claims-reference + # Microsoft offers an optional claim `xms_edov` that will indicate whether the + # user's email domain is part of the organization's verified domains. This has to be + # explicitly configured in the app registration. + # + # 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 + oidc_config = access_token.get(OIDC_CONFIG_URL).parsed + algorithms = oidc_config['id_token_signing_alg_values_supported'] + keys = JWT::JWK::Set.new(access_token.get(oidc_config['jwks_uri']).parsed) + decoded_token = JWT.decode(id_token, nil, true, algorithms: algorithms, jwks: keys) + # 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']) + rescue JWT::VerificationError, ::OAuth2::Error + false + end + + def verification_error_message + <<~MSG + The email domain '#{email_domain}' is not a verified domain for this Azure AD account. + You can either: + * Update the user's email to match the principal domain '#{upn_domain}' + * Skip verification on the '#{email_domain}' domain (not recommended) + * Disable verification with `skip_domain_verification: true` (NOT RECOMMENDED!) + Refer to the README for more details. + MSG + end + end + end +end diff --git a/lib/omniauth/strategies/microsoft_graph.rb b/lib/omniauth/strategies/microsoft_graph.rb index b5b87f8..3cef1c4 100644 --- a/lib/omniauth/strategies/microsoft_graph.rb +++ b/lib/omniauth/strategies/microsoft_graph.rb @@ -22,6 +22,7 @@ class MicrosoftGraph < OmniAuth::Strategies::OAuth2 option :scope, DEFAULT_SCOPE option :authorized_client_ids, [] + option :skip_domain_verification, false uid { raw_info["id"] } @@ -43,6 +44,12 @@ class MicrosoftGraph < OmniAuth::Strategies::OAuth2 } end + def auth_hash + super.tap do |ah| + verify_email(ah, access_token) + end + end + def authorize_params super.tap do |params| options[:authorize_options].each do |k| @@ -54,7 +61,7 @@ def authorize_params session['omniauth.state'] = params[:state] if params[:state] end - end + end def raw_info @raw_info ||= access_token.get('https://graph.microsoft.com/v1.0/me').parsed @@ -62,7 +69,7 @@ def raw_info def callback_url options[:callback_url] || full_host + script_name + callback_path - end + end def custom_build_access_token access_token = get_access_token(request) @@ -119,7 +126,11 @@ def verify_token(access_token) raw_response = client.request(:get, 'https://graph.microsoft.com/v1.0/me', params: { access_token: access_token }).parsed (raw_response['aud'] == options.client_id) || options.authorized_client_ids.include?(raw_response['aud']) - end + end + + def verify_email(auth_hash, access_token) + OmniAuth::MicrosoftGraph::DomainVerifier.verify!(auth_hash, access_token, options) + end end end end diff --git a/omniauth-microsoft_graph.gemspec b/omniauth-microsoft_graph.gemspec index ab2ca39..b5991fe 100644 --- a/omniauth-microsoft_graph.gemspec +++ b/omniauth-microsoft_graph.gemspec @@ -18,6 +18,7 @@ Gem::Specification.new do |spec| spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) spec.require_paths = ["lib"] + spec.add_runtime_dependency 'jwt', '>= 2.0' spec.add_runtime_dependency 'omniauth', '~> 2.0' spec.add_runtime_dependency 'omniauth-oauth2', '~> 1.8.0' spec.add_development_dependency "sinatra", '~> 0' diff --git a/spec/omniauth/microsoft_graph/domain_verifier_spec.rb b/spec/omniauth/microsoft_graph/domain_verifier_spec.rb new file mode 100644 index 0000000..0c5c94b --- /dev/null +++ b/spec/omniauth/microsoft_graph/domain_verifier_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'omniauth/microsoft_graph/domain_verifier' + +RSpec.describe OmniAuth::MicrosoftGraph::DomainVerifier do + subject(:verifier) { described_class.new(auth_hash, access_token, options) } + + let(:auth_hash) do + { + 'info' => { 'email' => email }, + 'extra' => { 'raw_info' => { 'userPrincipalName' => upn } } + } + end + let(:email) { 'foo@example.com' } + let(:upn) { 'bar@hackerman.biz' } + let(:options) { { skip_domain_verification: false } } + let(:access_token) { double('OAuth2::AccessToken', params: { 'id_token' => id_token }) } + let(:id_token) { nil } + + describe '#verify!' do + subject(:result) { verifier.verify! } + + context 'when email domain and userPrincipalName domain match' do + let(:email) { 'foo@example.com' } + 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 the email domain is explicitly permitted' do + let(:options) { super().merge(skip_domain_verification: ['example.com']) } + + it { is_expected.to be_truthy } + end + + context 'when the ID token indicates domain verification' do + # Sign a fake ID token with our own local key + let(:mock_key) do + optional_parameters = { kid: 'mock-kid', use: 'sig', alg: 'RS256' } + JWT::JWK.new(OpenSSL::PKey::RSA.new(2048), optional_parameters) + end + let(:id_token) do + payload = { email: email, xms_edov: true } + JWT.encode(payload, mock_key.signing_key, mock_key[:alg], kid: mock_key[:kid]) + end + + # Mock the API responses to return the local key + before do + allow(access_token).to receive(:get) + .with(OmniAuth::MicrosoftGraph::OIDC_CONFIG_URL) + .and_return( + double('OAuth2::Response', parsed: { + 'id_token_signing_alg_values_supported' => ['RS256'], + 'jwks_uri' => 'https://example.com/jwks-keys' + }) + ) + allow(access_token).to receive(:get) + .with('https://example.com/jwks-keys') + .and_return( + double('OAuth2::Response', parsed: JWT::JWK::Set.new(mock_key).export) + ) + end + + it { is_expected.to be_truthy } + end + + 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 + end + end + end +end diff --git a/spec/omniauth/strategies/microsoft_graph_oauth2_spec.rb b/spec/omniauth/strategies/microsoft_graph_oauth2_spec.rb index 6df68ea..b87a84c 100644 --- a/spec/omniauth/strategies/microsoft_graph_oauth2_spec.rb +++ b/spec/omniauth/strategies/microsoft_graph_oauth2_spec.rb @@ -280,6 +280,18 @@ end end + context 'when email verification fails' do + let(:response_hash) { { mail: 'something@domain.invalid' } } + let(:error) { OmniAuth::MicrosoftGraph::DomainVerificationError.new } + + before do + allow(OmniAuth::MicrosoftGraph::DomainVerifier).to receive(:verify!).and_raise(error) + end + + it 'raises an error' do + expect { subject.auth_hash }.to raise_error error + end + end end describe '#extra' do @@ -445,5 +457,4 @@ end.to raise_error(OAuth2::Error) end end - end