Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(service-client): Pass the correct amqp audience based on aut… #1835

Merged
merged 1 commit into from
Mar 18, 2021

Conversation

vinagesh
Copy link
Member

…h type

Checklist

  • I have read the contribution guidelines.
  • I added or modified the existing tests to cover the change (we do not allow our test coverage to go down).
  • This pull-request is submitted against the master branch.

Description of the changes

Reference/Link to the issue solved with this PR (if any)

@@ -553,7 +553,7 @@ private async Task SendCbsTokenAsync(AmqpCbsLink cbsLink, TimeSpan timeout)
Credential.AmqpEndpoint,
audience,
resource,
AccessRightsHelper.AccessRightsToStringArray(_accessRights),
Credential.AmqpAudience,
Copy link
Member Author

Choose a reason for hiding this comment

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

The IoT hub service ignores this value for AAD auth. But for the sake of correctness, we should not be passing audience related to SAS tokens during AAD auth.

@vinagesh
Copy link
Member Author

/azp run

@vinagesh vinagesh marked this pull request as ready for review March 18, 2021 01:03
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vinagesh
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@drwill-ms drwill-ms left a comment

Choose a reason for hiding this comment

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

Are you j

@vinagesh
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vinagesh vinagesh merged commit 2181425 into previews/RBAC Mar 18, 2021
@vinagesh vinagesh deleted the vinagesh/update branch March 18, 2021 17:34
vinagesh added a commit that referenced this pull request May 4, 2021
…#1922)

* (service-client): Design for IoT hub AAD authentication

* (service-client: Refactor and add implementation for token credential input) (#1781)

* (service-client): Refactor and add sas credential (#1786)

* (service-client): Add constructors in service client to accept aad and sas tokens. (#1787)

* (service-client): Add constructors in registry manager to accept aad and sas tokens. (#1788)

* (service-client): Add constructors in job client to accept aad and sas tokens. (#1789)

* (service-client): Add constructors to accept aad and sas tokens for digital twins client. (#1790)

* fix(service-client): Support for AzureSasCredential for a better user experience (#1797)

* doc(service-client): Update readme about the differnt client and operations (#1798)

* tests(service-client): E2E tests for aad auth on all our clients (#1800)

* test(service-client): Adding e2e tests for sas credential auth for IoT hub. (#1806)

* feature(service-client): Adding chaching for aad tokens. (#1807)

* fix(service-client)- Add IoT hub token scope. (#1812)

* feat(e2e) - Adding azure core and identity event sources. (#1826)

* fix(service-client): Fixing the required format for sending aad tokens to the service (#1834)

* refactor(service-client): Pass the correct amqp audience based on auth type (#1835)

* feat(e2e): Enable tests for IotHub AAD auth (#1861)

* feat(e2e): Enable jobs test on pipeline for aad auth (#1867)

* fix(service-client): Add default value for transpor type. (#1870)

* feat(e2e): Enable aad auth pnp tests now that the service supports it. (#1888)

* feat(e2e-setup): Update script to setup required assets for AAD auth tests (#1920)

* Fix merge error

* Made changes based on PR comments
timstewartm pushed a commit to timstewartm/azure-iot-sdk-csharp that referenced this pull request May 30, 2024
…Azure#1922)

* (service-client): Design for IoT hub AAD authentication

* (service-client: Refactor and add implementation for token credential input) (Azure#1781)

* (service-client): Refactor and add sas credential (Azure#1786)

* (service-client): Add constructors in service client to accept aad and sas tokens. (Azure#1787)

* (service-client): Add constructors in registry manager to accept aad and sas tokens. (Azure#1788)

* (service-client): Add constructors in job client to accept aad and sas tokens. (Azure#1789)

* (service-client): Add constructors to accept aad and sas tokens for digital twins client. (Azure#1790)

* fix(service-client): Support for AzureSasCredential for a better user experience (Azure#1797)

* doc(service-client): Update readme about the differnt client and operations (Azure#1798)

* tests(service-client): E2E tests for aad auth on all our clients (Azure#1800)

* test(service-client): Adding e2e tests for sas credential auth for IoT hub. (Azure#1806)

* feature(service-client): Adding chaching for aad tokens. (Azure#1807)

* fix(service-client)- Add IoT hub token scope. (Azure#1812)

* feat(e2e) - Adding azure core and identity event sources. (Azure#1826)

* fix(service-client): Fixing the required format for sending aad tokens to the service (Azure#1834)

* refactor(service-client): Pass the correct amqp audience based on auth type (Azure#1835)

* feat(e2e): Enable tests for IotHub AAD auth (Azure#1861)

* feat(e2e): Enable jobs test on pipeline for aad auth (Azure#1867)

* fix(service-client): Add default value for transpor type. (Azure#1870)

* feat(e2e): Enable aad auth pnp tests now that the service supports it. (Azure#1888)

* feat(e2e-setup): Update script to setup required assets for AAD auth tests (Azure#1920)

* Fix merge error

* Made changes based on PR comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants