social-core icon indicating copy to clipboard operation
social-core copied to clipboard

add possibility to configure tenant id

Open tiago-peres opened this issue 2 years ago • 8 comments

Bug: https://github.com/python-social-auth/social-core/issues/748

Tested locally and was able to change the Tenant ID from common to organizations with

SOCIAL_AUTH_AZUREAD_OAUTH2_TENANT_ID = "organizations"

tiago-peres avatar Feb 09 '23 12:02 tiago-peres

There already is dedicated backend for tenant authentication: https://github.com/python-social-auth/social-core/blob/master/social_core/backends/azuread_tenant.py

nijel avatar Feb 09 '23 13:02 nijel

@nijel read the bug report. What I'm changing is not the Tenant, but the assumption that we use common in the url all the time. That's not true and leads to errors doing that.

tiago-peres avatar Feb 09 '23 13:02 tiago-peres

The reason for using is that nomenclature is the way the BASE_URL is formed

BASE_URL = "https://{authority_host}/{tenant_id}"

The {tenant_id} is what I'd like have the possibility to change, between common or organizations, depending on the type of Azure application.

tiago-peres avatar Feb 09 '23 13:02 tiago-peres

can change it to signin_url and SOCIAL_AUTH_AZUREAD_OAUTH2_SIGNIN_URL if you prefer (based on Microsoft's documentation)

tiago-peres avatar Feb 09 '23 13:02 tiago-peres

Codecov Report

Merging #751 (6501fc0) into master (3f49832) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #751   +/-   ##
=======================================
  Coverage   77.27%   77.27%           
=======================================
  Files         325      325           
  Lines        9875     9875           
  Branches     1180     1180           
=======================================
  Hits         7631     7631           
  Misses       2088     2088           
  Partials      156      156           
Flag Coverage Δ
unittests 77.27% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
social_core/backends/azuread.py 79.68% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Feb 26 '23 12:02 codecov[bot]

I'd like to hear another opinion here as well. Using TENANT name for this seems confusing to me - it is not really tenant in this case, but rather a scope.

nijel avatar Mar 01 '23 15:03 nijel

Yea... the reason to use TENANT was to make as less changes as possible to what's already there... Look forward to see a resolution though.

tiago-peres avatar Mar 01 '23 18:03 tiago-peres

Instead of misusing TENANT I'd rather introduce configuration for sign-in URL, where whole URL would be specified. This way it would be consistent with Azure docs where they document which URL to use. If URL would not be configured, it would fall back to the current implementation.

nijel avatar Apr 25 '24 06:04 nijel