Merge lp:~rvb/gwacl/signed-access into lp:gwacl

[Raphaël Badin]

Add support for the Windows Azure Shared Access Signature URL mechanism.

This branch adds support for the Windows Azure Shared Access Signature URL mechanism. More specifically, it adds a way to get an anonymously-accessible url for a blob (even if the blob is located in a private storage).

See http://msdn.microsoft.com/en-us/library/windowsazure/dn140255.aspx for details.

[Julian Edwards (julian-edwards)]

Nice branch, reasonably well tested, but I think we should improve some of that testing!

In particular, the new sign() method is not unit tested. I think you should add a test for it, and then we can use that function in other tests like TestComposeSharedSignature so that we don't have to hard-code the magic string "COJt8UagHJR2LBT1129bhDChtgfLGFqfZ0YQpBdF0=" which makes the test difficult to verify what it's actually testing.

Ok so on to the details:

16 +func composeSharedSignature(permission, signedExpiry, path, accountName, signedIdentifier, signedVersion, accountKey string) string {

Since you documented the other internal compose* funcs, can you do the same here please.

19 + stringToSign := fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s",
20 + permission, "", signedExpiry, canonicalizedResource, signedIdentifier, signedVersion)

Gah I *really* miss Python's format() here. The second arg of "" is a bit magic, can you initalise a variable called 'signedStart' to "" and then use it instead of the bare "", it will make that Sprintf more readable (the "" sticks out like a pimple on a supermodel).

29 +func composeAccessQueryString(permission, signedExpiry, path, accountName, signedIdentifier, signedVersion, signedRessource, accountKey string) url.Values {

Given that this shares almost all of its params with composeSharedSignature it might be worth declaring a struct type which encapsulates the data in a shared access signature. I think once functions get beyond more than three params they get very unwieldy very quickly. The new type could well be useful in the future if we do more things with SASes.

Also this function is mis-named, it is not composing a string. May I suggest composeAcessQueryValues instead.

+func composeReadBlobValues(container, filename, accountName, accountKey string, expires time.Time) url.Values {
45 + expiryDateString := expires.UTC().Format(time.RFC3339)

Christ - why did they not make the date header format and this expiry stamp consistent? :/ (The other is RFC1123)

71 +func (*sharedSignatureSuite) TestComposeSharedSignature(c *C) {
72 + permission := "r"
73 + signedExpiry := "2015-02-12"
74 + path := "/path/to/file"
75 + accountName := "name"
76 + signedIdentifier := "identifier"
77 + signedVersion := "2012-02-12"
78 + accountKey := base64.StdEncoding.EncodeToString([]byte("key"))
79 +
80 + signature := composeSharedSignature(permission, signedExpiry, path, accountName, signedIdentifier, signedVersion, accountKey)
81 + c.Check(signature, Equals, "C/COJt8UagHJR2LBT1129bhDChtgfLGFqfZ0YQpBdF0=")

Here, once sign() is independently tested, you can use its return value instead of hard-coding this string here. The test will improve in readability for that.

103 +func (*sharedSignatureSuite) TestComposeReadBlobValues(c *C) {
104 + container := "container"
105 + filename := "/path/to/file"
106 + accountName := "name"
107 + accountKey := base64.StdEncoding.EncodeToString([]byte("key"))
108 + expires := time.Now()
109 + values := composeReadBlobValues(container, filename, accountName, accountKey, expires)
110 + c.Check(values.Get("sv"), Equals, "2012-02-12")
111 + ...

review: Needs Fixing

[Julian Edwards (julian-edwards)]

Early approval, I am sure you'll fix things :)

review: Approve

[lp:~rvb/gwacl/signed-access]

206. By Raphaël Badin on 2013-07-26

Review fixes.

[Raphaël Badin (rvb)]

> Nice branch, reasonably well tested, but I think we should improve some of
> that testing!

Thanks for the review!

> In particular, the new sign() method is not unit tested. I think you should
> add a test for it, and then we can use that function in other tests like
> TestComposeSharedSignature so that we don't have to hard-code the magic string
> "COJt8UagHJR2LBT1129bhDChtgfLGFqfZ0YQpBdF0=" which makes the test difficult to
> verify what it's actually testing.

Like I said on IRC, this is not a new method, it's just extracted from somewhere but it was already there (and tested by a method which calls it)… but you're right, having a unit-test of the method itself is much better; done.

> 16 +func composeSharedSignature(permission, signedExpiry, path,
> accountName, signedIdentifier, signedVersion, accountKey string) string {
>
> Since you documented the other internal compose* funcs, can you do the same
> here please.

In this case, I felt the name of the method would be as good as any comment but okay, done.

> 19 + stringToSign := fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s",
> 20 + permission, "", signedExpiry, canonicalizedResource,
> signedIdentifier, signedVersion)
>
> Gah I *really* miss Python's format() here. The second arg of "" is a bit
> magic, can you initalise a variable called 'signedStart' to "" and then use it
> instead of the bare "", it will make that Sprintf more readable (the "" sticks
> out like a pimple on a supermodel).

Very good point, done (I've added a 'signedStart' field to the structure).

> 29 +func composeAccessQueryString(permission, signedExpiry, path,
> accountName, signedIdentifier, signedVersion, signedRessource, accountKey
> string) url.Values {
>
> Given that this shares almost all of its params with composeSharedSignature it
> might be worth declaring a struct type which encapsulates the data in a shared
> access signature. I think once functions get beyond more than three params
> they get very unwieldy very quickly. The new type could well be useful in the
> future if we do more things with SASes.

Excellent point, done.

> Also this function is mis-named, it is not composing a string. May I suggest
> composeAcessQueryValues instead.

True, I started with a method which returned a string then changed the signature but not the name, thanks for spotting that!

> 71 +func (*sharedSignatureSuite) TestComposeSharedSignature(c *C) {
> 72 + permission := "r"
> 73 + signedExpiry := "2015-02-12"
> 74 + path := "/path/to/file"
> 75 + accountName := "name"
> 76 + signedIdentifier := "identifier"
> 77 + signedVersion := "2012-02-12"
> 78 + accountKey := base64.StdEncoding.EncodeToString([]byte("key"))
> 79 +
> 80 + signature := composeSharedSignature(permission, signedExpiry,
> path, accountName, signedIdentifier, signedVersion, accountKey)
> 81 + c.Check(signature, Equals,
> "C/COJt8UagHJR2LBT1129bhDChtgfLGFqfZ0YQpBdF0=")
>
> Here, once sign() is independently tested, you can use its return value
> instead of hard-coding this string here. The test will improve in readability
> for that.

That wou...

[Raphaël Badin (rvb)]

> Early approval, I am sure you'll fix things :)

I did, thanks for the review :)

=== added file 'shared_signature.go'
--- shared_signature.go	1970-01-01 00:00:00 +0000
+++ shared_signature.go	2013-07-26 08:51:26 +0000
@@ -0,0 +1,74 @@
+// Copyright 2013 Canonical Ltd.  This software is licensed under the
+// GNU Lesser General Public License version 3 (see the file COPYING).
+
+package gwacl
+
+import (
+    "fmt"
+    "net/url"
+    "time"
+)
+
+// sharedSignatureParams is a object which encapsulate all the parameters
+// required to delegate access to a Windows Azure object using the
+// "Shared Access Signature" mechanism.
+type sharedSignatureParams struct {
+    permission       string
+    signedStart      string
+    signedExpiry     string
+    path             string
+    accountName      string
+    signedIdentifier string
+    signedVersion    string
+    signedRessource  string
+    accountKey       string
+}
+
+// composeSharedSignature composes the "Shared Access Signature" as described
+// in the paragraph "Constructing the Signature String" in
+// http://msdn.microsoft.com/en-us/library/windowsazure/dn140255.aspx
+func (params *sharedSignatureParams) composeSharedSignature() string {
+    // Compose the string to sign.
+    canonicalizedResource := fmt.Sprintf("/%s%s", params.accountName, params.path)
+    stringToSign := fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s",
+        params.permission, params.signedStart, params.signedExpiry, canonicalizedResource, params.signedIdentifier, params.signedVersion)
+    // Create the signature.
+    signature := sign(params.accountKey, stringToSign)
+    return signature
+}
+
+// composeAccessQueryValues returns the values that correspond to the query
+// string used to build a Shared Access Signature URI as described in
+// http://msdn.microsoft.com/en-us/library/windowsazure/dn140255.aspx
+func (params *sharedSignatureParams) composeAccessQueryValues() url.Values {
+    signature := params.composeSharedSignature()
+    // Compose the "Shared Access Signature" query string.
+    values := url.Values{}
+    values.Set("sv", params.signedVersion)
+    values.Set("se", params.signedExpiry)
+    values.Set("sr", params.signedRessource)
+    values.Set("sp", params.permission)
+    values.Set("sig", signature)
+    return values
+}
+
+// getReadBlobAccessValues returns the values that correspond to the query
+// string used to build a Shared Access Signature URI.  The signature grants
+// read access to the given blob.
+func getReadBlobAccessValues(container, filename, accountName, accountKey string, expires time.Time) url.Values {
+    expiryDateString := expires.UTC().Format(time.RFC3339)
+
+    path := fmt.Sprintf("/%s/%s", container, filename)
+    signatureParams := &sharedSignatureParams{
+        permission:       "r",
+        signedExpiry:     expiryDateString,
+        signedStart:      "",
+        path:             path,
+        accountName:      accountName,
+        signedIdentifier: "",
+        signedVersion:    "2012-02-12",
+        signedRessource:  "b",
+        accountKey:       accountKey,
+    }
+    return signatureParams.composeAccessQueryValues()
+}

=== added file 'shared_signature_test.go'
--- shared_signature_test.go	1970-01-01 00:00:00 +0000
+++ shared_signature_test.go	2013-07-26 08:51:26 +0000
@@ -0,0 +1,70 @@
+// Copyright 2013 Canonical Ltd.  This software is licensed under the
+// GNU Lesser General Public License version 3 (see the file COPYING).
+
+package gwacl
+
+import (
+    "encoding/base64"
+    . "launchpad.net/gocheck"
+    "time"
+)
+
+type sharedSignatureSuite struct{}
+
+var _ = Suite(&sharedSignatureSuite{})
+
+func (*sharedSignatureSuite) TestComposeSharedSignature(c *C) {
+    params := &sharedSignatureParams{
+        permission:       "r",
+        signedExpiry:     "2015-02-12",
+        path:             "/path/to/file",
+        accountName:      "name",
+        signedIdentifier: "identifier",
+        signedVersion:    "2012-02-12",
+        signedRessource:  "/the/ressource",
+        accountKey:       base64.StdEncoding.EncodeToString([]byte("key")),
+    }
+
+    signature := params.composeSharedSignature()
+    c.Check(signature, Equals, "C/COJt8UagHJR2LBT1129bhDChtgfLGFqfZ0YQpBdF0=")
+}
+
+func (*sharedSignatureSuite) TestComposeAccessQueryValues(c *C) {
+    params := &sharedSignatureParams{
+        permission:       "r",
+        signedExpiry:     "2015-02-12",
+        path:             "/path/to/file",
+        accountName:      "name",
+        signedIdentifier: "identifier",
+        signedVersion:    "2012-02-12",
+        signedRessource:  "/the/ressource",
+        accountKey:       base64.StdEncoding.EncodeToString([]byte("key")),
+    }
+
+    values := params.composeAccessQueryValues()
+
+    c.Check(values.Get("sv"), Equals, params.signedVersion)
+    c.Check(values.Get("se"), Equals, params.signedExpiry)
+    c.Check(values.Get("sr"), Equals, params.signedRessource)
+    c.Check(values.Get("sp"), Equals, params.permission)
+    c.Check(values.Get("sig"), Not(HasLen), 0)
+}
+
+func (*sharedSignatureSuite) TestGetReadBlobAccessValues(c *C) {
+    container := "container"
+    filename := "/path/to/file"
+    accountName := "name"
+    accountKey := base64.StdEncoding.EncodeToString([]byte("key"))
+    expires, err := time.Parse("Monday, 02-Jan-06 15:04:05 MST", time.RFC850)
+    c.Assert(err, IsNil)
+
+    values := getReadBlobAccessValues(container, filename, accountName, accountKey, expires)
+
+    c.Check(values.Get("sv"), Equals, "2012-02-12")
+    expiryDateString := values.Get("se")
+    expectedExpiryDateString := expires.UTC().Format(time.RFC3339)
+    c.Check(expiryDateString, Equals, expectedExpiryDateString)
+    c.Check(values.Get("sr"), Equals, "b")
+    c.Check(values.Get("sp"), Equals, "r")
+    c.Check(values.Get("sig"), Equals, "HK7xmxiUY/vBNkaIWoJkIcv27g/+QFjwKVgO/I3yWmI=")
+}

=== modified file 'storage_base.go'
--- storage_base.go	2013-06-27 11:53:50 +0000
+++ storage_base.go	2013-07-26 08:51:26 +0000
@@ -41,9 +41,9 @@
     "Range",
 }
 
-// Calculate the value required for an Authorization header.
-func composeAuthHeader(req *http.Request, accountName, accountKey string) string {
-    signable := composeStringToSign(req, accountName)
+// sign returns the base64-encoded HMAC-SHA256 signature of the given string
+// using the given base64-encoded key.
+func sign(accountKey, signable string) string {
     // Allegedly, this is already UTF8 encoded.
     decodedKey, err := base64.StdEncoding.DecodeString(accountKey)
     if err != nil {
@@ -57,6 +57,14 @@
     var hashed []byte
     hashed = hash.Sum(hashed)
     b64Hashed := base64.StdEncoding.EncodeToString(hashed)
+    return b64Hashed
+}
+
+// Calculate the value required for an Authorization header.
+func composeAuthHeader(req *http.Request, accountName, accountKey string) string {
+    signable := composeStringToSign(req, accountName)
+
+    b64Hashed := sign(accountKey, signable)
     return fmt.Sprintf("SharedKey %s:%s", accountName, b64Hashed)
 }
 
@@ -338,6 +346,14 @@
     return context.getContainerURL(container) + "/" + url.QueryEscape(filename)
 }
 
+// GetAnonymousFileURL returns an anonymously-accessible URL for a given file
+// in the given container.
+func (context *StorageContext) GetAnonymousFileURL(container, filename string, expires time.Time) string {
+    url := context.GetFileURL(container, filename)
+    values := getReadBlobAccessValues(container, filename, context.Account, context.Key, expires)
+    return fmt.Sprintf("%s?%s", url, values.Encode())
+}
+
 type ListContainersRequest struct {
     Marker string
 }

=== modified file 'storage_base_test.go'
--- storage_base_test.go	2013-07-17 09:11:43 +0000
+++ storage_base_test.go	2013-07-26 08:51:26 +0000
@@ -161,6 +161,19 @@
     c.Assert(observed, Equals, expected)
 }
 
+type TestSign struct{}
+
+var _ = Suite(&TestSign{})
+
+func (suite *TestSign) TestSign(c *C) {
+    key := base64.StdEncoding.EncodeToString([]byte("dummykey"))
+    signable := "a-string-to-sign"
+
+    observed := sign(key, signable)
+    expected := "5j1DSsm07IEh3u9JQQd0KPwtM6pEGChzrAF7Zf/LxLc="
+    c.Assert(observed, Equals, expected)
+}
+
 type TestComposeAuthHeader struct{}
 
 var _ = Suite(&TestComposeAuthHeader{})
@@ -303,6 +316,31 @@
         "http://"+url.QueryEscape(account)+".blob.core.windows.net/"+url.QueryEscape(container)+"/"+url.QueryEscape(file))
 }
 
+func (suite *TestStorageContext) TestGetSignedFileURL(c *C) {
+    account := "account"
+    container := "container"
+    file := "/a/file"
+    key := base64.StdEncoding.EncodeToString([]byte("dummykey"))
+    context := StorageContext{Account: account, Key: key}
+    expires := time.Now()
+
+    signedURL := context.GetAnonymousFileURL(container, file, expires)
+    // The only difference with the non-anon URL is the query string.
+    parsed, err := url.Parse(signedURL)
+    c.Assert(err, IsNil)
+    fileURL, err := url.Parse(context.GetFileURL(container, file))
+    c.Assert(err, IsNil)
+    c.Check(parsed.Scheme, Equals, fileURL.Scheme)
+    c.Check(parsed.Host, Equals, fileURL.Host)
+    c.Check(parsed.Path, Equals, fileURL.Path)
+
+    values, err := url.ParseQuery(parsed.RawQuery)
+    c.Assert(err, IsNil)
+    signature := values.Get("sig")
+    expectedSignature := getReadBlobAccessValues(container, file, account, key, expires).Get("sig")
+    c.Check(signature, Equals, expectedSignature)
+}
+
 func (suite *TestStorageContext) TestGetClientReturnsDefaultClient(c *C) {
     context := &StorageContext{client: nil}
     c.Assert(context.getClient(), Equals, http.DefaultClient)