Generate url #504

kyleknap · 2015-03-30T21:16:55Z

So this pull request is to expose an interface for supporting presigned url. Note that this PR does not include support for s3's presigned POST's. With this interface, in order to generate a presigned url. All you need is a RequestSigner. With the RequestSigner you can call generate_url() method that takes in a request dictionary (from the serializer and prepare_request_dict) and will return a url that is presigned. Note that I considered calling the method presign_url(), but I decided to name it how boto does. You also have the option to specify how many seconds till the url expires and if you want to sign with a different region name.

Also, this PR removes the last of the skips in the integration test for the clients-only branch.

cc @jamesls @danielgtaylor

kyleknap · 2015-03-30T21:21:16Z

botocore/auth.py

+            # We only want to include relevant headers in the query string.
+            # These can be anything that starts with x-amz, is Content-MD5,
+            # or is Content-Type.
+            elif lk.startswith('x-amz-') or lk in ['content-md5',


I considered abstracting a public method out in HmacV1Auth, but I was trying to avoid touching existing code. The bottom line is that the auth class picks what headers it will sign for (much like the sigv4 signer), but it just does not have a public headers_to_sign method. I can add one though.

coveralls · 2015-04-01T17:21:57Z

Coverage increased (+0.1%) to 95.73% when pulling 770f802 on kyleknap:generate-url into e318dd6 on boto:clients-only.

kyleknap · 2015-04-01T17:34:00Z

@jamesls @danielgtaylor

So the commits I recently pushed are for the ability to presign s3 post (both sigv2 and sigv4). The public method is build_post_form_args. To get a sense on how the CLI and boto3 will use it, check out the integration tests in test_s3.py. That has some examples of how you can leverage the presigner and requests library to make the post request.

One thing that would be great to get your opinions on is the interface in general. I feel that it is a little weird that the method is attached to the general signing class, but only applies to s3. Ideally for me, there would be some isolation between this method and the general signer class (because it only applies to s3 posts) but I have not been able to think of a better way. Maybe I can check that the service of the signer is s3, and if not throw an error? Or rename it to have s3 in the name of the method? Also let me know what you think of the name build_post_form_args. I picked it because that is the name boto uses, but the inputs and return values are a little different.

coveralls · 2015-04-02T20:53:56Z

Coverage increased (+0.49%) to 96.12% when pulling 77cc71d on kyleknap:generate-url into e318dd6 on boto:clients-only.

kyleknap · 2015-04-02T20:54:15Z

In the latest commit, I exposed some client level methods, generate_url and build_s3_post_form_args, in signers.py that will eventually be translated to actual methods of the client for the presigning methods. So I would pay more attention to the internals of the methods. When I do dangle them off the client, they will pretty much take the same arguments as they are currently minus the client argument.

kyleknap · 2015-04-02T20:56:10Z

botocore/signers.py

+    if conditions is None:
+        conditions = []
+
+    request_signer = client._request_signer


I realize that these are internal. However, we will get access to them when we add the appropriate event for adding methods to classes.

coveralls · 2015-04-02T20:57:12Z

Coverage increased (+0.49%) to 96.12% when pulling 77cc71d on kyleknap:generate-url into e318dd6 on boto:clients-only.

kyleknap · 2015-04-02T20:58:15Z

botocore/signers.py

+
+    # We choose the CreateBucket operation model because its url gets
+    # serialized to what a presign post requires.
+    operation_model = client.meta.service_model.operation_model('CreateBucket')


I considered creating my own model for this. However, I felt CreateBucket is suitable. All I really need is the url when I serialize the request and the CreateBucket serialization is exactly how I need the url serialized. Let me know if you want me to change this.

kyleknap · 2015-04-02T21:25:13Z

tests/unit/auth/test_signers.py

+            [{"acl": "public-read"},
+             {"bucket": "mybucket"},
+             ["starts-with", "$key", "mykey"]])
+        self.assertIn('signature', result_fields)


I did not assert do assert equal because we dump a dictionary to json and there is no guarantee that gets dumped the same each time unless you order the json elements, which is not needed functionality-wise.

coveralls · 2015-04-02T21:32:18Z

Coverage increased (+0.49%) to 96.12% when pulling f293551 on kyleknap:generate-url into e318dd6 on boto:clients-only.

jamesls · 2015-04-02T23:55:13Z

botocore/auth.py

@@ -23,6 +23,7 @@
 import functools
 import time
 import calendar
+import json


from botocore.compat import json

jamesls · 2015-04-03T18:59:26Z

Overall looks good, just some minor review feedback. As far as the interface goes, I'd prefer to just have a separate class that does the s3 post forms stuff. I agree that it doesn't feel right having it in the general signer class.

Allows users to presign a url. Converted existing customizations and tests to use the presigner as well.

The presigner only supports get requests.

Also updated code based on feedback.

coveralls · 2015-04-03T22:24:51Z

Coverage increased (+0.53%) to 96.17% when pulling 3bcfda5 on kyleknap:generate-url into edfc623 on boto:clients-only.

kyleknap · 2015-04-03T22:25:52Z

Alright in the very latest commit I did two things:

Update the code based on feedback
Dangle the presigning methods on clients as: client.generate_presigned_url, client.generate_presigned_post

With these additions presigning is officially available in boto3 as well.

Let me know what you think. @jamesls @danielgtaylor

danielgtaylor · 2015-04-03T22:48:42Z

botocore/model.py

@@ -1,6 +1,7 @@
 """Abstractions to interact with service models."""
 from collections import defaultdict

+from botocore import xform_name


Is this used anywhere?

Is this still needed?

Nah. It did not get cleaned up after I removed the client_name_to_operation_name function I added to the model. Will remove

danielgtaylor · 2015-04-03T22:59:04Z

Just a few minor comments. LGTM 👍

jamesls · 2015-04-03T23:23:20Z

Just some small stuff, otherwise looks good.

coveralls · 2015-04-06T16:31:16Z

Coverage increased (+0.53%) to 96.17% when pulling 95c4913 on kyleknap:generate-url into edfc623 on boto:clients-only.

kyleknap · 2015-04-06T16:31:47Z

@danielgtaylor
I updated the code based on your feedback. The only thing that I did not change was the regular expression as I was hesitant to change that as it was just a copy and paste from a different file. Let me know what you think.

danielgtaylor · 2015-04-06T17:19:30Z

Thanks, LGTM 🚢-it!

Generate url

vlcinsky · 2015-05-07T19:09:38Z

It would be great, if the method would allow also absolute expiration time.

Adding additional parameter expires_in_absolute with True of False would work (copying the way, boto does that).

kyleknap reviewed Mar 30, 2015
View reviewed changes

kyleknap force-pushed the generate-url branch from d0ece1b to 77cc71d Compare April 2, 2015 20:49

kyleknap reviewed Apr 2, 2015
View reviewed changes

kyleknap force-pushed the generate-url branch from 5f284ba to f293551 Compare April 2, 2015 21:21

kyleknap reviewed Apr 2, 2015
View reviewed changes

jamesls reviewed Apr 2, 2015
View reviewed changes

botocore/auth.py

@@ -23,6 +23,7 @@

import functools

import time

import calendar

import json

Copy link

Member

jamesls Apr 2, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from botocore.compat import json

kyleknap force-pushed the clients-only branch from e318dd6 to edfc623 Compare April 3, 2015 22:09

kyleknap added 8 commits April 3, 2015 15:11

Create a generate_url method on the signer class

f500016

Allows users to presign a url. Converted existing customizations and tests to use the presigner as well.

Add v2 presigner class for s3

f2e8ad9

The presigner only supports get requests.

Enable more than gets for sigv4 presign

d07b42e

Touch up presigner code

982e476

Add s3 sigv4 presign post support

f604295

Implemented sigv2 presigning for s3 posts

e7356d6

Touch up presign post code

ebad81a

Support dns naming conventions for s3 presigning

80725be

kyleknap added 4 commits April 3, 2015 15:11

Fix py3 test failure

afdc404

Add doc strings to presigning methods

5baad52

Add client level wrapper for presign methods

5bc95d5

Fix an auth test failure

d2d439f

kyleknap force-pushed the generate-url branch from f293551 to d9ed75b Compare April 3, 2015 22:14

Dangle presign methods off of client

3bcfda5

Also updated code based on feedback.

kyleknap force-pushed the generate-url branch from d9ed75b to 3bcfda5 Compare April 3, 2015 22:19

danielgtaylor reviewed Apr 3, 2015
View reviewed changes

Update code based on feedback

95c4913

kyleknap added a commit that referenced this pull request Apr 6, 2015

Merge pull request #504 from kyleknap/generate-url

bc2c982

Generate url

kyleknap merged commit bc2c982 into boto:clients-only Apr 6, 2015

kyleknap deleted the generate-url branch April 6, 2015 17:42

vlcinsky mentioned this pull request May 9, 2015

Support generation of signed URL's for S3 access aws/aws-cli#462

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate url #504

Generate url #504

kyleknap commented Mar 30, 2015

kyleknap Mar 30, 2015

coveralls commented Apr 1, 2015

kyleknap commented Apr 1, 2015

coveralls commented Apr 2, 2015

kyleknap commented Apr 2, 2015

kyleknap Apr 2, 2015

coveralls commented Apr 2, 2015

kyleknap Apr 2, 2015

kyleknap Apr 2, 2015

coveralls commented Apr 2, 2015

jamesls Apr 2, 2015

jamesls commented Apr 3, 2015

coveralls commented Apr 3, 2015

kyleknap commented Apr 3, 2015

danielgtaylor Apr 3, 2015

jamesls Apr 3, 2015

kyleknap Apr 3, 2015

danielgtaylor commented Apr 3, 2015

jamesls commented Apr 3, 2015

coveralls commented Apr 6, 2015

kyleknap commented Apr 6, 2015

danielgtaylor commented Apr 6, 2015

vlcinsky commented May 7, 2015

Generate url #504

Generate url #504

Conversation

kyleknap commented Mar 30, 2015

Choose a reason for hiding this comment

coveralls commented Apr 1, 2015

kyleknap commented Apr 1, 2015

coveralls commented Apr 2, 2015

kyleknap commented Apr 2, 2015

Choose a reason for hiding this comment

coveralls commented Apr 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Apr 2, 2015

Choose a reason for hiding this comment

jamesls commented Apr 3, 2015

coveralls commented Apr 3, 2015

kyleknap commented Apr 3, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielgtaylor commented Apr 3, 2015

jamesls commented Apr 3, 2015

coveralls commented Apr 6, 2015

kyleknap commented Apr 6, 2015

danielgtaylor commented Apr 6, 2015

vlcinsky commented May 7, 2015