Repository navigation
Proposed fixes for token-based auth in azure fileshare service - #820
Conversation
|
Want to add that I haven't been able to test this fully on my side yet.
|
Sergiy Matusevych (motus)
left a comment
Sergiy Matusevych (motus)
left a comment
There was a problem hiding this comment.
Looks good, although I suspect that we need to make sure that the credential has been properly initialized in get_credential() implementation - just like we do in get_auth_token(). I've pushed an update along with a few cosmetic fixes to eujing#1 - Eu Jing Chua (@eujing) please take a look
make sure we initialize before invoking get_credential()
| credential = self._auth_service.get_credential() | ||
| assert isinstance( | ||
| credential, TokenCredential | ||
| ), f"Expected a TokenCredential, but got {type(credential)} instead." |
There was a problem hiding this comment.
This is still a late runtime error that I was trying to turn into a config load error in #819
There was a problem hiding this comment.
Alright, I made a new change to #819 to handle that now.
From what I can tell the rest of this one is good, but it'd be good to get confirmation that DefaultCredential doesn't need refreshed, especially with the SP part.
3 participants
To use token-based authentication for
ShareClient, I think we should be passing in the credential object derived fromTokenCredential(in our case, some instance ofDefaultAzureCredential.Previously, we were passing specific string tokens to the
credentialargument, which is being intepreted as a SAS token.This leads to errors like:
azure.core.exceptions.ClientAuthenticationError: Server failed to authenticate the request. Make sure the value of Authorization header is formed correctly including the signature.ShareClient documentation on the
credentialargument.By passing in the whole
TokenCredentialobject, I believeShareClientwill manage the token lifecycle and we won't need to do so as mentioned in #818.