Skip to content

Conversation

@philon-msft
Copy link
Contributor

@philon-msft philon-msft commented May 25, 2023

Fix #6, #15, #18, #34

@gladjohn
Copy link

changes looks good. please enable MSAL logging, this will help us with troubleshooting any issues you may see.

@philon-msft
Copy link
Contributor Author

changes looks good. please enable MSAL logging, this will help us with troubleshooting any issues you may see.

Thanks @gladjohn - we plan to enable MSAL logging in an upcoming release once StackExchange.Redis itself supports a logging story

/// <param name="configurationOptions">The configuration to update.</param>
/// <param name="clientId">Client ID of the user-assigned managed identity.</param>
/// <param name="clientId">Client ID or resource ID of the user-assigned managed identity.</param>
/// <param name="principalId">Principal (object) ID of the user-assigned managed identity.</param>

Choose a reason for hiding this comment

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

MSAL does not support principalId for Managed Identity. Is this something you want us to support?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@samsaha-ms can you comment on this? We pass the MI's object/principal ID as the username for the Redis connection, so I believe we use it in some capacity while validating tokens on the server side

Copy link

@samsaha-ms samsaha-ms May 26, 2023

Choose a reason for hiding this comment

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

Yes, this is something related to what our extension does to interact with Redis, not related to MSAL.
It does two things 1. Acquire token from MSAL 2. Send AUTH request to Redis with principal-id and token to comply with redis command format "auth username password" and we validate this pair in server side.
We could actually make this field optional here, we could parse the acquired token and get the oid field in token and use that value while sending to redis. We can choose it do later after giving some thought as it might add client side overhead of parsing token.

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.

Authentication fails for User Assigned Managed Identity

5 participants