B2c implementation with policy parameter #110
Conversation
rayluo
left a comment
rayluo
left a comment
There was a problem hiding this comment.
Thanks for this effort! Made some suggestions below.
| token_cache=None, | ||
| verify=True, proxies=None, timeout=None, | ||
| client_claims=None): | ||
| client_claims=None, trust_framework_policy=None): |
There was a problem hiding this comment.
I believe the currently proposal is:
.WithAuthority(string authority, string trustFrameworkPolicy)not
.WithAuthority(string authority) .WithTrustFrameworkPolicy(string trustFrameworkPolicy)So the precise mapping would be just change the "shape" of the existing authority parameter, rather than introducing a new parameter for PCA.
Perhaps we want to have something like this (with the comparing to C# version, side-by-side)?
authority = {"authority": "https://domain/tenant", "policy": "B2C_1_SignIn"}
# .WithAuthority(string authority, string trustFrameworkPolicy)And then the pca can be created like this:
pca = PublicClientApplication(
...,
authority={"authority": "https://domain/tenant", "policy": "B2C_1_SignIn"},
...)
There was a problem hiding this comment.
note to use userFlow instead of policy or trustFrameworkPolicy. thanks.
There was a problem hiding this comment.
Then let's use snake_case user_flow.
|
|
||
| def instance_discovery(url, **kwargs): | ||
| return requests.get( # Note: This URL seemingly returns V1 endpoint only | ||
| def instance_discovery(url, response=None, **kwargs): |
There was a problem hiding this comment.
Since B2C does not do Instance Discovery at all, this PR should not need to change this helper.
Regardless of the B2C api interface, this helper was recently updated as part of an improvement for non-B2C scenarios. Now that change was lost, possibly caused by an improper merge between the dev branch and your local branch? We can fix that, by either cherry-pick those lost commits back, or it might actually be easier to branch off the latest dev, and then add the needed changes on top of it.
|
(Cleaning up osbolete PRs) This PR is no longer needed, as we've moved to a different direction of not introducing |
No description provided.