Closed keystroke closed 2 years ago
@rayluo I can change the target branch of this PR but the target branch needs to already exist. Can you create a branch like "feature/adfs" (or whichever name you prefer) from main that I can send this PR to? That way customer can pip install from that "official" feature branch until larger change is available later this year with configuration-based feature controls.
@rayluo I can change the target branch of this PR but the target branch needs to already exist. Can you create a branch like "feature/adfs" (or whichever name you prefer) from main that I can send this PR to? That way customer can pip install from that "official" feature branch until larger change is available later this year with configuration-based feature controls.
Created an experimental
branch just now, and tentatively chose it as a base. It turns out lots of "noise commits" would show up in this PR. Perhaps you should consider rebase (or even redo) your 2-line changes to the current experimental
(which was based on dev
).
@rayluo why based on dev? If customer will be using this a drop-in replacement, shouldn't it based on main (assuming that's the branch you release from)?
using this a drop-in replacement
Branching off from dev
is still a drop-in replacement. Remember how I described MSAL Python 1.x has no breaking changes?
shouldn't it based on main (assuming that's the branch you release from)?
While our eventual stable release will come from the main
branch, that still does not mean our feature branches will branch off from main
. Quite the opposite, all our feature branches start from dev
. In this particular case, we will not actually cut a release from any branch, either. The customer can install directly from this feature branch, by pip install git+https://github.com/AzureAD/microsoft-authentication-library-for-python.git@experimental
. This was also briefly mentioned yesterday.
Sure, at the end of the day it's your team / product owning the solution for customer. I will redo PR against the experimental branch as requested and customer can consume from there. Thanks!
This is a scoped change to disable authority lookup when the authority is parsed as an "ADFS" environment. This check is already integrated into authority.py to disable instance discovery flows, but it was missed in this alias feature. This results in MSAL.py reaching out to internet in air-gapped clouds. A longer-term fix would be to standardize on the MSAL configuration patterns from .NET which allow user to provide an instance_metadata configuration object to bypass these checks with configured values.