Azure / azure-storage-php

Microsoft Azure Storage Library for PHP
MIT License
217 stars 198 forks source link

Integrate SharedAccessSignatureHelper into Proxy #96

Closed dkarlovi closed 4 years ago

dkarlovi commented 7 years ago

Should be able to generate SAS URLs directly from Proxy. See https://github.com/Azure/azure-storage-php/issues/78#issuecomment-320588632

XiaoningLiu commented 7 years ago

@dkarlovi Currently a proxy can be created with storage account key or SAS.

If we integrate SharedAccessSignatureHelper into Proxy, however, we will find out that only proxies created with account key are able to generate SAS URLs. Proxies created with SAS are not able to generate a SAS URL. It's decided by the SAS token generation algorithm. https://docs.microsoft.com/en-us/rest/api/storageservices/constructing-an-account-sas

Integrating SharedAccessSignatureHelper into Proxy won't help solve the above problem.

dkarlovi commented 7 years ago

@XiaoningLiu thanks for your reply.

I feel like having the proxy method throw an exception if not able to generate the SAS URL would suffice here: you (the developer) get informed the operation failed and why, also how to fix it if desired.

On the other hand, if you have created a proxy with storage credentials, you get the benefit of using the SAS generator easily, without tracking down how to use it somewhere else, wrapping in helper methods, etc. It seems reasonable to me.

dkarlovi commented 7 years ago

As suggested in https://github.com/Azure/azure-storage-php/issues/97#issuecomment-321783198:

Might consider introducing a new class, SharedAccessSignatureRequest (or similar) which could simplify the code here. Functionally the same as a TLS CSR sent to a CA when requesting a new HTTPS key.

To expand on this: as it stands, creating a SAS is quite involved, as evidenced by the params required in the helper method.

My suggestion is: create a value object ("model") SharedAccessSignatureRequest which would encapsulate all the SAS params validation and normalization. It would mean your method would accept exactly one param and be much easier to proxy in the... well, Proxy. :) A reasonable thing would be to also introduce a SharedAccessSignature response which can provide either the token or the full URL, as mentioned before.

Rough idea outline:

class SharedAccessSignatureRequest
{
    public function__construct($resourceName, $permissions, \DateTime $expiry)
    public function setStart(\DateTime $start);
    // etc.
}

class SharedAccessSignature
{
    // full SAS URL, so there's no need to concatenate strings in user app
    public function getResourceUrl(): string

    // only the token, so there's no need to parse the URL for it
    public function getToken(): string

    // etc.
}

class BlobProxy
{
    public function generateSharedAccessSignature(SharedAccessSignatureRequest $request): SharedAccessSignature
}
XiaoningLiu commented 6 years ago

Sorry for late response. Actually I'm still concern about the "throw an exception if not able to generate the SAS URL". In the future, the client library will add support for OAuth, which could create a proxy using OAuth token. In this case, it still cannot generate a SAS token.

katmsft commented 4 years ago

Since there is no more actionable items on this issue, we will close this issue. Feel free to reopen or submit another issue.

dkarlovi commented 2 years ago

Just found this issue again since I'm looking to generate a SAS URL again from the blob proxy. It being unable to do it is quite a PITA.

Currently I'm using the SDK from within a Symfony app and need to generate the \MicrosoftAzure\Storage\Common\Internal\StorageServiceSettings as a service which I then inject into a factory for the shared signature helper.