Skip to content

Sample with JWT#68

Open
patilsnr wants to merge 10 commits into
mainfrom
npatilsen/aadSample
Open

Sample with JWT#68
patilsnr wants to merge 10 commits into
mainfrom
npatilsen/aadSample

Conversation

@patilsnr

@patilsnr patilsnr commented Sep 21, 2023

Copy link
Copy Markdown
Contributor

Adds sample with JWT authentication tied to a managed identity

Comment thread scenarios/jwt_authentication/dotnet/jwt_authentication/Program.cs
@patilsnr patilsnr marked this pull request as ready for review September 25, 2023 19:37
@patilsnr

Copy link
Copy Markdown
Contributor Author

/azp run

@rido-min rido-min left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd like to include JWT support in the MqttConnectionSettings, at least we should use .env files to specify the hostname

My machine takes 5+ secs to acquire the token, there might be some optimizations we can make, and also test with a ManagedIdentity

Review how to refresh the token

Let's wait until we have the builtin roles

@andyk-ms

andyk-ms commented Oct 4, 2023

Copy link
Copy Markdown

Please note additional improvements in the PBI as TODO.

@rido-min

rido-min commented Oct 4, 2023

Copy link
Copy Markdown
Contributor

I'd like to do a quick test pass with the new builtin roles before merging.

@varunpuranik

Copy link
Copy Markdown

I'd like to include JWT support in the MqttConnectionSettings, at least we should use .env files to specify the hostname

My machine takes 5+ secs to acquire the token, there might be some optimizations we can make, and also test with a ManagedIdentity

Review how to refresh the token

Let's wait until we have the builtin roles

Do we need to block on this? In the spirit of iterative improvements on these samples, i would suggest we check it in, and put another item on our backlog to improve it based on the steps above.
Any concerns @rido-min ?
Thoughts @patilsnr ?

@patilsnr

patilsnr commented Nov 1, 2023

Copy link
Copy Markdown
Contributor Author

I'd like to include JWT support in the MqttConnectionSettings, at least we should use .env files to specify the hostname
My machine takes 5+ secs to acquire the token, there might be some optimizations we can make, and also test with a ManagedIdentity
Review how to refresh the token
Let's wait until we have the builtin roles

Do we need to block on this? In the spirit of iterative improvements on these samples, i would suggest we check it in, and put another item on our backlog to improve it based on the steps above. Any concerns @rido-min ? Thoughts @patilsnr ?

I'm pro merging it in, and have been for some time. There are readme instructions on how to use this -- the main improvement to be had is using the built-in roles that have been recently rolled out but don't currently work (as opposed to creating a custom role).

@rido-min

Copy link
Copy Markdown
Contributor

this PR is blocked by dotnet/MQTTnet#1881

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.

4 participants