Fix Azure Container Token Auth#5093
Conversation
|
I'm not 100% what user documentation needs to be added, if any. Then manual doesn't really currently talk about different types of SAS token for the Azure backend, and I'm unsure if that it's the place for it - as both token types just work as expected now, do we need to draw attention to the fact we handle them ever so slightly differently behind the scenes? |
MichaelEischer
left a comment
There was a problem hiding this comment.
LGTM. Thanks for fixing this issue!
Ignore AuthorizationFailure caused by using a container level SAS/SAT token when calling GetProperties during the Create() call. This is because the GetProperties call expects an Account Level token, and the container level token simply lacks the appropriate permissions. Supressing the Authorization Failure is OK, because if the token is actually invalid, this is caught elsewhere when we try to actually use the token to do work.
Add changelog entry in the 'unreleased' sub-folder for changes introduced when fixing issue restic#4004.
Add two new test cases, TestBackendAzureAccountToken and TestBackendAzureContainerToken, that ensure that the authorization using both types of token works. This introduces two new environment variables, RESTIC_TEST_AZURE_ACCOUNT_SAS and RESTIC_TEST_AZURE_CONTAINER_SAS, that contain the tokens to use when testing restic. If an environment variable is missing, the related test is skipped.
16d0ab6 to
b434f56
Compare
|
I've slightly squashed the commits, without changing anything: https://github.com/restic/restic/compare/16d0ab6f4e1ceaf35c2bcbf108b3e8f6e7a9ae34..b434f560cc53b074c23f4aad1619014f0f9be465 |
Fair enough! Happy to have been able to be some help. |
What does this PR change? What problem does it solve?
This change ignores the Authorization Failed error returned from the Azure Storage Account backend when attempting to use a Shared Access Signature token that was not created with the Storage Account key to initalise a new repository.
The issue is caused by the Azure library that we use in the backend. When enumerating the container properties, the library internally calls
container.CreateIfNotExistswhich is where things fail, because it tries to create a container, which is denied, as the token doesn'thave permission to create new containers, only poke around in the container it was made for.Was the change previously discussed in an issue or on the forum?
Closes #4004
Checklist
changelog/unreleased/that describes the changes for our users (see template).gofmton the code in all commits.