BingAds / BingAds-Python-SDK

Other
118 stars 164 forks source link

Authentication for a single tenant app is not possible #147

Closed tector closed 4 years ago

tector commented 4 years ago

I use the BingAds Python SDK already for some time with a multi tenant app and it was working fine.

But then i had to switch to a single tenant app so I have a registered app that has 'supported account types' set to 'My organization only'.

So i have given:

I am using the OAuthWebAuthCodeGrant like this:

authentication = OAuthWebAuthCodeGrant(
    client_id=client_id,
    client_secret=client_secret,
    redirection_uri=[...]
    env=ENVIRONMENT
)

The (default) multi tenant environment with public client works without tenant id and even without using a client secret. But for a single tenant environment with non public client both is necessary.

Look at authorization.py Line 642, 646 and 650 https://github.com/BingAds/BingAds-Python-SDK/blob/8262f24f3ef3111ccf3b35b8fd0ef31030fd6621/bingads/authorization.py#L642

The redirection url is hardcoded(!) to https://login.microsoftonline.com/common/oauth2/nativeclient

But with a single tenant app this doesn't work. You have to put in your tenant id like this: https://login.microsoftonline.com//oauth2/nativeclient

Because i can not patch the library (because i don't know the possible side effects) i had to monkey patch the URIs in my own code. My code is based on the example auth_helper.py. I put my monkey patch after the imports, like this:

from bingads.authorization import _UriOAuthService

# monkey patch the bingads sdk to use tenant id:
tenant_id = '<MY_TENANT_ID>'
oauth2_uri = 'https://login.microsoftonline.com/' + tenant_id + '/oauth2'
_UriOAuthService.REDIRECTION_URI[('production', False)] = oauth2_uri + '/nativeclient'
_UriOAuthService.AUTH_TOKEN_URI[('production', False)] = oauth2_uri + '/v2.0/token'
_UriOAuthService.AUTHORIZE_URI[('production', False)] = oauth2_uri + '/v2.0/authorize?client_id={0}&scope=https%3A%2F%2Fads.microsoft.com%2Fads.manage%20offline_access&response_type={1}&redirect_uri={2}'

Then i can continue with the OAuthWebAuthCodeGrant:

def authenticate(auth_config, authorization_data):

    authentication = OAuthWebAuthCodeGrant(
        client_id='<MY_CLIENT_ID>',
        client_secret='<MY_CLIENT_SECRET>',
        redirection_uri=_UriOAuthService.REDIRECTION_URI[('production', False)],
        env=ENVIRONMENT
    )

    [...]

Conclusion:

I don't know why nobody else seems to have this problem with your library.. maybe it is not used much in an business environment where you have a single tenant and no public clients. Or i am just stupid and can't find an easier solution. But it would be really helpful if you offer the functionality and documentation on this usecase!

eric-urban commented 4 years ago

@tector is there a reason you would not want to use 'common'? You are correct, the SDK currently only supports 'common'.

tector commented 4 years ago

Yes. But it is a longer story: We worked with the 'common' endpoint for years now, and it worked fine. Over the last years Microsoft modified how their accounts work: For example, when we started using the BingAds API there was no OAuth at all and BingAds and Microsoft Accounts were completely separated - and all without requiring Azure at all (was it existing at all in 2012?)! We have gone through all these changes but that also created some technical debts. Then, in the last year or two, in our company we had multiple times issues with hacked BingAds Accounts (but not related to our API account). Microsoft was accommodating towards us because the reason for the hacks stayed unclear. But then Microsoft forced us to use 2FA with each account - otherwise they won't be accommodating in possible future issues. So we enabled 2FA everywhere. On our API account too. But this was not a personalized account. So we had to switch to a personalized account with API access (one of multiple). And also we decided to follow Best Practices, force personalized accounts with enabled 2FA everywhere and manage them in our tenant account (which is managed by superadmins).

So we are using BEST PRACTICES now, but the BingAds-Python-SDK does not support these best practices yet.

I hope this made clear why we NEED this feature now. Thank you for listening.

eric-urban commented 4 years ago

@tector the common tenant for the Microsoft identity endpoint will work with apps registered in the Azure portal as described here. 2FA should also work (I've confirmed at least on my own personal account). Please share any links to "best practices" documentation that suggests otherwise. If I'm still missing your point and/or you need to share more confidential information please feel to reach out via support.

tector commented 4 years ago

It is true that the 2FA works with the 'common' endpoint... But that is not the issue. The issue is that we have a tenant account and multiple personalized account with API access rights which are managed by the tenant account. It is NOT possible to use the 'common' endpoint in this scenario when using the SDK. And there is no option to use the tenant endpoint with the Python SDK (maybe with SDKs for other languages).

tector commented 4 years ago

I think the point is clear now and contacting the Advertising Support will delay things too much (because it is not technical support!) I even already give you the solution for this issue in my opening post. Please just validate it and test for conflicts with other parts of the code that i am not aware of! Thanks!

qitia commented 4 years ago

this has been supported from Version 13.0.4. Please try it out and reopen this if you still see anything wrong.