Skip to content

Conversation

chlowell
Copy link
Member

ADFS identity tokens have different claims than do AAD identity tokens. This change ensures credentials can build a correct AuthenticationRecord from either authority's tokens.

@chlowell chlowell added Client This issue points to a problem in the data-plane of the library. Azure.Identity labels Aug 26, 2020
@chlowell chlowell requested review from mccoyp and xiangyan99 August 26, 2020 16:20
@chlowell chlowell requested a review from schaabs as a code owner August 26, 2020 16:20
xiangyan99
xiangyan99 previously approved these changes Aug 28, 2020
sub="subject",
aud="client-id",
username="username",
tenant_id="tenant-id",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason the tenant_id here is "tenant-id" but tenant_id is "tenant" in build_id_token()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a conscious one. I typically slug fake IDs because real ones are GUIDs and I can imagine something in the stack choking on a space. That's probably why past me dropped the " id" above. Can't say why he didn't slug it instead or do similar to "object id".

While thinking about this, I realized these fake tokens shouldn't contain a tenant ID (or an object ID) at all because real ones never do. So I made some changes here to improve consistency and accuracy. Thanks for the scrutiny!

@chlowell chlowell merged commit 8f8574e into Azure:master Sep 1, 2020
@chlowell chlowell deleted the adfs-auth-record branch September 1, 2020 19:25
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Sep 2, 2020
…into link_om_sample

* 'master' of https://github.com/Azure/azure-sdk-for-python: (23 commits)
  Int32 serialization (Azure#13452)
  add output to opinion mining sample (Azure#13494)
  Add Document w/ Eng Sys Checks (Azure#13492)
  update version (Azure#13495)
  Remove resources post test (Azure#13379)
  bing_id -> bing_entity_search_api_id (Azure#13491)
  [EventGrid] Read me + improve docstrings (Azure#13484)
  Build AuthenticationRecords from ADFS identity tokens (Azure#13341)
  Support Subject Name/Issuer authentication (Azure#13350)
  Add KeyVaultAccessControlClient for data plane RBAC (Azure#13372)
  [text analytics] Add redacted_text (Azure#13449)
  add python sdk sample (Azure#13338)
  [text analytics] add versionadded sphinx documentation (Azure#13450)
  [text analytics] add bing_id property to LinkedEntity class (Azure#13446)
  fix typing for paging methods (Azure#13410)
  [text analytics] add domain_filter param (Azure#13451)
  fix issue Azure#11658 for is_valid_resource_id (Azure#11709)
  added create_table_if_not_exists method to table service client (Azure#13385)
  [ServiceBus] Test and failure improvements (Azure#13345)
  Proper encoding and decoding of source URLs - Fixes special characters in source URL issue (Azure#13275)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Identity Client This issue points to a problem in the data-plane of the library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants