Skip to content
This repository has been archived by the owner on Jul 19, 2024. It is now read-only.

Does not work with multi-tenant apps #25

Open
ckritzinger opened this issue Jul 7, 2016 · 3 comments
Open

Does not work with multi-tenant apps #25

ckritzinger opened this issue Jul 7, 2016 · 3 comments

Comments

@ckritzinger
Copy link

ckritzinger commented Jul 7, 2016

I am in the process of setting up a multi-tenant SSO (i.e. outside of my own Azure domains) and ran into the following:

  1. The Authorisation endpoint for a multi-tenant auth needs to be /common/ instead of /[tenant_id]/ as reported by the configuration from Azure
  2. When authenticating a multi-tenant app, we cannot verify the issuer, since it could be any Azure Tenant, not only our own. I'm not familiar enough with JWT to understand the full security impact of this change

Below is my monkey-patch (from config/initialisers/devise.rb) that seems to have solved the situation for me.

   # ==> OmniAuth
  # Add a new OmniAuth provider. Check the wiki for more information on setting
  # up on your models and hooks.
  # config.omniauth :github, 'APP_ID', 'APP_SECRET', scope: 'user,public_repo'
  config.omniauth :azure_activedirectory, Settings.azure.aad_client_id, Settings.azure.aad_tenant

  # monkey-patch
  module OmniAuth
    module Strategies
      class AzureActiveDirectory
        def authorize_endpoint_url
          wrong_uri = URI(openid_config['authorization_endpoint'])
          # as per http://stackoverflow.com/a/32529128
          uri = URI('https://login.microsoftonline.com/common/oauth2/authorize')
          uri.query = URI.encode_www_form(client_id: client_id,
                                          redirect_uri: callback_url,
                                          response_mode: response_mode,
                                          response_type: response_type,
                                          nonce: new_nonce)
          uri.to_s
        end
        def verify_options
          { verify_expiration: true,
            verify_not_before: true,
            verify_iat: true,
            # I am somewhat uneasy about this solution, since it allows any issuer to provide credential info.
            # However, since the issuer is ?guaranteed? to be an Azure Tenant, we're OK.  Or are we? (gulp)
            # TODO: Investigate
            # verify_iss: true,
            # 'iss' => issuer,
            verify_aud: true,
            'aud' => client_id }
        end
      end
    end
  end
@ricalo
Copy link

ricalo commented Jul 7, 2016

@ckritzinger, I think you should use the following authorize endpoint:
https://login.microsoftonline.com/common/oauth2/authorize instead of https://login.windows.net/common/oauth2/authorize per this MSDN article.

BTW, just want to reference this issue to #20, that doesn't have an answer yet.

@ckritzinger
Copy link
Author

@ricalo Thanks :) The URL I used seems to work, but this one makes more sense

@equivalent
Copy link

My version of monkey patch (basically stolen @ckritzinger monkey patch + endpoint fix mentioned by @ricalo

module OmniAuth
  module Strategies
    class AzureActiveDirectory
      def raw_authorize_endpoint_url
        'https://login.microsoftonline.com/common/oauth2/authorize'
      end

      def authorize_endpoint_url
        uri = URI(raw_authorize_endpoint_url)
        uri.query = URI.encode_www_form(client_id: client_id,
                                        redirect_uri: callback_url,
                                        response_mode: response_mode,
                                        response_type: response_type,
                                        nonce: new_nonce)
        uri.to_s
      end

      def verify_options
        { verify_expiration: true,
          verify_not_before: true,
          verify_iat: true,
          verify_aud: true,
          'aud' => client_id }
      end
    end
  end
end

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants