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

Fix issues in AzureFileShareService introduced by #779 #818

Closed
bpkroth opened this issue Jul 24, 2024 · 1 comment · May be fixed by #819
Closed

Fix issues in AzureFileShareService introduced by #779 #818

bpkroth opened this issue Jul 24, 2024 · 1 comment · May be fixed by #819
Assignees
@bpkroth

Comments

@bpkroth
Copy link
Contributor

bpkroth commented Jul 24, 2024

  1. _get_share_client is now only called once, but needs to be refreshed on demand using the AuthService

  2. This change caused tests to try and authenticate, but we need to mock that for tests:

           Proposed fix to that:

motus#14

Originally posted by @bpkroth in #779 (comment)
@bpkroth bpkroth mentioned this issue Jul 24, 2024
Open
@bpkroth bpkroth assigned bpkroth and unassigned motus Jul 24, 2024
@eujing eujing mentioned this issue Jul 25, 2024
Merged
motus added a commit that referenced this issue Aug 3, 2024
@eujing @motus
Proposed fixes for token-based auth in azure fileshare service (#820)
45528cf 
To use token-based authentication for `ShareClient`, I think we should
be passing in the credential object derived from `TokenCredential` (in
our case, some instance of `DefaultAzureCredential`.

Previously, we were passing specific string tokens to the `credential`
argument, 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](https://learn.microsoft.com/en-us/python/api/azure-storage-file-share/azure.storage.fileshare.shareclient?view=azure-python)
documentation on the `credential` argument.

By passing in the whole `TokenCredential` object, I believe
`ShareClient` will manage the token lifecycle and we won't need to do so
as mentioned in #818.

---------

Co-authored-by: Eu Jing Chua <eujingchua@microsoft.com>
Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
@motus
Copy link
Member

motus commented Aug 3, 2024

Closed by #820

@motus motus closed this as completed Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bpkroth bpkroth

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixups to AzureFileShareService auth token bpkroth/MLOS
2 participants
@motus @bpkroth