Custom access policy for BYO Container is not supported

adobe / aio-tvm

Token Vending Machine, integrate external cloud services into I/O Runtime

https://www.adobe.io

Apache License 2.0

11 stars 13 forks source link

Custom access policy for BYO Container is not supported #46

Closed moritzraho closed 3 years ago

moritzraho commented 3 years ago

Issue

generating a presign url will get or create a policy
running revokeAllPresignURLs() will delete existing policies As a result revokeAllPresignURLs() will remove any pre-existing policy set by the user in the blob storage container

The underlying issue is that files lib does not support byo containers with custom properties but it also does not explicitly throws an error if the users does so.

Possible solutions

Throw an error on init if the container has custom access policies defined - OR any custom setting really
Support custom containers

meryllblanchet commented 3 years ago

Hi @moritzraho , with regards to the latter Support custom containers solution, I am afraid that we might have to support too many corner cases and custom setups. Do we have a larger overview of what could be impacting custom containers?

IMO the first solution combined to an enhancement of the documentation could be satisfying.

moritzraho commented 3 years ago

I am afraid that we might have to support too many corner cases and custom setups.

Yes agree, + the support for custom containers would be a challenge to maintain, as new features in azure or files lib might break it.

Do we have a larger overview of what could be impacting custom containers?

Not really, for now I only see custom policies breaking due to the way presign urls are generated and revoked but other custom settings might have more subtle impacts

meryllblanchet commented 3 years ago

I'd then suggest

going for the first solution
documenting our requirements and expectations for a BYO scenario
doing the same for aio-lib-state if needed

@sarahxxu how do you feel with that approach?

sarahxxu commented 3 years ago

I concur

tmathern commented 3 years ago

I would love that we get (1) Throw an error on init if the container has custom access policies defined - OR any custom setting really. So it makes sure we don't unexpectedly break existing setup. (Another idea, which is even "harsher": for developers, when they use the lib, force them to explicitly switch a config flag to allow aio to overwrite/create permission policies... ? This would make it impossible to break this by chance, but is an API breaking change).

aiojbot commented 3 years ago

JIRA issue created: https://jira.corp.adobe.com/browse/ACNA-1230

sandeep-paliwal commented 3 years ago

Added PR to address above issue aio-lib-files will check for any custom policies at time of init and will throw error if custom policies are present

moritzraho commented 3 years ago

@tmathern do you want to take a quick look to the PR raised by Sandeep before we merge it ? https://github.com/adobe/aio-lib-files/pull/97

tmathern commented 3 years ago

@moritzraho Sure, I'll take a look.

One comment:

When we test for an existing policy, I don't think there is a a guarantee that a uuidv4-like identifier was created by aio... It could be someone also has code to create those automatically using uuidv4 (since it's a good way to create identifiers). I would suggest, when aio creates policies, to prefix them with aio or aio-files to avoid confusion. So the id creation and id check could look for a policy named ${aio-prefix}-${uuidv4-id}. Wdyt?

sandeep-paliwal commented 3 years ago

@tmathern we would only support a single policy for a given container, as per the new change if there are more than one policies, aio-lib-files init will anyway error out(more than one policy will mean there are custom policies). So we won't need any prefix and any custom policy with uuid4 format will anyway error out.