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.

Sign up for GitHub

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

Jump to bottom

Add OnBehalfOf authentication to V2 for edgehub #3353

Merged
merged 8 commits into from
Aug 4, 2023
Merged

Add OnBehalfOf authentication to V2 for edgehub #3353

merged 8 commits into from
Aug 4, 2023

Conversation

patilsnr
Copy link
Contributor

Adds OnBehalfOf authentication to SDK for Edge devices

@patilsnr patilsnr changed the title Npatilsen/add e4 k sas utils Add OnBehalfOf authentication to V2 for edgehub Jul 24, 2023
rido-min
rido-min suggested changes Jul 26, 2023
View reviewed changes
Copy link
Member

@rido-min rido-min left a comment

Choose a reason for hiding this comment

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

  • Clarify some comments
  • Avoid bringing more classes from SdkLite
  • We need a test strategy to provision required parent/child devices

iothub/device/src/Authentication/ClientAuthenticationForEdgeHubOnBehalfOf.cs Outdated Show resolved Hide resolved
iothub/device/src/Authentication/ClientAuthenticationForEdgeHubOnBehalfOf.cs Outdated Show resolved Hide resolved
iothub/device/src/Authentication/Security/SharedAccessSignatureBuilder.cs Show resolved Hide resolved
iothub/device/tests/Authentication/EdgeDeviceOnBehalfOfTests.cs Outdated Show resolved Hide resolved
iothub/device/tests/Authentication/EdgeDeviceOnBehalfOfTests.cs Outdated Show resolved Hide resolved
iothub/device/tests/Authentication/EdgeDeviceOnBehalfOfTests.cs Outdated Show resolved Hide resolved
iothub/device/tests/Authentication/EdgeDeviceOnBehalfOfTests.cs Outdated Show resolved Hide resolved
andyk-ms
andyk-ms approved these changes Aug 3, 2023
View reviewed changes
Copy link
Contributor

@andyk-ms andyk-ms left a comment

Choose a reason for hiding this comment

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

Minor feedbacks that would improve supportability but should not stop moving forward.

@patilsnr
Copy link
Contributor Author

patilsnr commented Aug 4, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@patilsnr patilsnr merged commit 987e0a2 into previews/v2 Aug 4, 2023
@patilsnr patilsnr deleted the npatilsen/addE4KSasUtils branch August 4, 2023 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rido-min rido-min rido-min requested changes

@andyk-ms andyk-ms andyk-ms approved these changes

@timtay-microsoft timtay-microsoft Awaiting requested review from timtay-microsoft timtay-microsoft is a code owner

@abhipsaMisra abhipsaMisra Awaiting requested review from abhipsaMisra abhipsaMisra is a code owner

@brycewang-microsoft brycewang-microsoft Awaiting requested review from brycewang-microsoft brycewang-microsoft is a code owner

@tmahmood-microsoft tmahmood-microsoft Awaiting requested review from tmahmood-microsoft tmahmood-microsoft is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@patilsnr @rido-min @andyk-ms