-
Notifications
You must be signed in to change notification settings - Fork 94
Enable dynamic whitelisting of dSTS endpoints to support new buildouts #215
Conversation
add whitelist function so dsts hosts pass validation.
Adding unit test to test static instance discovery of dsts enpoints and validating of a dsts endpoint in AuthenticationContext.
Remove unnecessary white space, add documentation to whitelist, use self.fail in unit test.
Change dSTS host whitelisting to dynamic discovery of endpoints containing dsts.
Fix whitespace
Fix dSTS test endpoint value to correct format.
Fix dsts test endpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick question. Does dSTS accept resource
parameter rather than scope
parameter in the wire protocol? Is that the only reason that dSTS would stick with ADAL library, rather than picking up MSAL?
Also a specific comment below.
Update whitelist funtion to only check for ".dsts." in url.
I am not sure about the details on this, we could have a conversation with Abhinav the dSTS lead to add dSTS support to MSAL. For now, this is a short term fix to support customers already using ADAL who need to deploy a new cloud environment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concluding PR review. 🚢
Enable dynamic whitelisting of dSTS endpoints to support new buildouts of national cloud environments.
UPDATED info quoted from PR conversation:
The new authority.py implementation for dSTS is to support (future) new cloud environments in which we will not be allowed to explicitly whitelist endpoints in public GitHub repos due to customer restrictions on releasing DNS names. Since I have removed dSTS whitelisting constants, these test cases should be sufficient to cover all current and future dSTS endpoints that fit the .dsts. DNS address pattern.