omniauth-azure-activedirectory-v2
omniauth-azure-activedirectory-v2 copied to clipboard
Shouldn't custom_policy be included in the main Azure OAuth URL - not just in the token URL?
Describe the bug
I have been testing the gem to login against an Azure AD B2C instance, with a custom policy. Even though I configure the custom policy in my initializer, the authorization URL does not include it. The generated authorization URL is "#{base_azure_url}/#{tenant_id}/oauth2/v2.0/authorize"
. But, according to Microsoft's docs, it seems that the URL should actually be "#{base_azure_url}/#{tenant_id}/#{custom_policy}/oauth2/v2.0/authorize"
.
To Reproduce
Just check the file lib/omniauth/strategies/azure_activedirectory_v2.rb
, where the method OmniAuth::Strategies::AzureActivedirectoryV2#client
is defined as:
def client
# ... (other not relevant code here) ...
options.custom_policy =
provider.respond_to?(:custom_policy) ? provider.custom_policy : nil
options.client_options.authorize_url = "#{options.base_azure_url}/#{options.tenant_id}/oauth2/v2.0/authorize"
options.client_options.token_url =
if options.custom_policy
"#{options.base_azure_url}/#{options.tenant_id}/#{options.custom_policy}/oauth2/v2.0/token"
else
"#{options.base_azure_url}/#{options.tenant_id}/oauth2/v2.0/token"
end
super
end
As you can see in the previous code snippet, the custom_policy is being included in the token URL, but not included in the authorize URL.
Expected behavior
If I did not miss anything in Microsoft's docs, I think the authorize URL should also include the custom policy: "#{base_azure_url}/#{tenant_id}/#{custom_policy}/oauth2/v2.0/authorize"
.
What do you think? Might I be right? If so, if you need me, I can create a PR. I haven't reviewed all the gem's code, but it looks like the patch would be easy:
def client
# ...
options.client_options.authorize_url =
if options.custom_policy
"#{options.base_azure_url}/#{options.tenant_id}/#{options.custom_policy}/oauth2/v2.0/authorize"
else
"#{options.base_azure_url}/#{options.tenant_id}/oauth2/v2.0/authorize"
end
# ...
end
@jrodrigosm That documentation is for the B2C Azure, and while I don't understand the three hundred super confusing similar versions of Azure AD (now Entra), I can at least see that we're definitely not doing this:
https://{tenant}.b2clogin.com/{tenant}.onmicrosoft.com/...
...as per your reference at https://learn.microsoft.com/en-us/azure/active-directory-b2c/authorization-code-flow - we are as you point out using options.base_azure_url
, which means by default we're doing this:
https://login.microsoftonline.com/...
...from what README.md
links as https://learn.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-auth-code-flow - a totally different doc, though it looks very similar! Here, you will see at https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-auth-code-flow#request-an-authorization-code that there is no requirement for a policy nor is such a property documented.
What's interesting, though, is that no policy parameter is documented at the above reference at all for any of the documented endpoints. So we have:
- One MS doc that has a totally different base URL with tenant in it twice, but which does have policy shown
- One MS doc that has a base URL matching this gem, but with no mention of policies
- An implementation which inserts a policy string in the path contrary to docs anyway, but only for one of the two endpoints
I'm really not sure what to make of that!
Are you able to confirm that if you make the suggested amendment, OAuth to Azure AD (Entra) does work as expected and does use the custom policy specified? Or is your suggestion entirely hypothetical at the moment?
@jrodrigosm Any thoughts about the above?
@pond I had the same experience as @jrodrigosm . When changing the base_url to B2C and specifying a custom_policy, it did not work as Jordan mentioned. If possible, could we adjust the authorize_url to accommodate the custom_policy when specifying a b2c custom policy and changing the base_url?
If needed, I can test this and submit a PR.
@Jureamer For sure, a PR with tests would be really great, thanks - but it would be good if we could also actually get a URL for a Microsoft document explaining all this, so that we understand the basis of what we're doing, and that README.md
can be updated with appropriately comprehensive instructions for users of the gem.