Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

401 Unauthorized: Temp URL invalid if path contains whitespaces #79

Closed
dperetti opened this issue Feb 3, 2017 · 23 comments
Closed

401 Unauthorized: Temp URL invalid if path contains whitespaces #79

dperetti opened this issue Feb 3, 2017 · 23 comments

Comments

@dperetti
Copy link

dperetti commented Feb 3, 2017

Obviously a quoting issue.
Suggestion : refactor url() using swiftclient's generate_temp_url().

FYI my quick and dirty workaround in my django app was as follow :

from swiftclient.utils import generate_temp_url
from six.moves.urllib import parse as urlparse

sw = SwiftStorage()
url = urlparse.urljoin(sw.base_url, urlparse.quote(sw.name_prefix + release_path))
path = urlparse.unquote(urlparse.urlsplit(url).path)
url = generate_temp_url(path=path, key=sw.temp_url_key, seconds=int(sw.temp_url_duration), method='GET')
return urlparse.urljoin(sw.base_url, url)
@einarf
Copy link
Contributor

einarf commented Feb 5, 2017

Got an example of an invalid url?

@einarf
Copy link
Contributor

einarf commented Feb 8, 2017

Please review the PR.

Also: Is this bug about file names containing spaces? It would be nice to get an example-url so we can write a test for this specific situation.

@dperetti
Copy link
Author

dperetti commented Feb 8, 2017

Will let you know shortly !

@einarf
Copy link
Contributor

einarf commented Feb 8, 2017

Great. Not sure if the quoting matters or not. That's the only thing missing from your example.

@dperetti
Copy link
Author

dperetti commented Feb 9, 2017

Trying to pip install your pull request like usual and this doesn't work:

pip install git+git://github.com/blacktorn/django-storage-swift.git@043fabe1af9532f9b5d649ef450abf0d75616802

Says it's not a tree. Any idea ?

@einarf
Copy link
Contributor

einarf commented Feb 9, 2017

Try from the repo/branch the pull requests came from:
https://github.com/ZettaIO/django-storage-swift/tree/temp-url

@einarf
Copy link
Contributor

einarf commented Feb 9, 2017

To be more specific, use github's archive url: (tar.gz of branch temp-url)
https://github.com/ZettaIO/django-storage-swift/archive/temp-url.tar.gz

pip install https://github.com/ZettaIO/django-storage-swift/archive/temp-url.tar.gz

.. or just clone the branch and run python setup.py install or python setup.py develop so you can easily modify the code while installed. The you don't have to mess around in site-packages etc.

@dperetti
Copy link
Author

dperetti commented Feb 9, 2017

Using your fork works ... but I'm back to 401 !
The url seems OK but the temp_url_sig is probably wrong.
It looks like you pass the sig as third argument of generate_temp_url, it should be an url_key as in my example above.

@einarf
Copy link
Contributor

einarf commented Feb 9, 2017

When do you get 401?

@dperetti
Copy link
Author

dperetti commented Feb 9, 2017

When I go to the generated url.

@einarf
Copy link
Contributor

einarf commented Feb 9, 2017

Are you sure that added the SWIFT_TEMP_URL_KEY in your swift account as well?

@dperetti
Copy link
Author

dperetti commented Feb 9, 2017

Of course, look at my example, I'm even using it and... it works.
Again, I think you don't pass the correct arguments to generate_temp_url(). Read this : the third argument should precisely be the temp url key, not the hash of it !

@einarf
Copy link
Contributor

einarf commented Feb 9, 2017

ahh yes you are right. generate_temp_url is also generating the hmac.

Fixing.

@dperetti
Copy link
Author

dperetti commented Feb 9, 2017

Side comment : the unit tests should have spotted that it didn't work ;-)

@einarf
Copy link
Contributor

einarf commented Feb 9, 2017

Yep they should ideally do that. Branch is updated 😄

Created issue #81

@dperetti
Copy link
Author

dperetti commented Feb 9, 2017

Doesn't work : the path must be unquoted here because it was quoted here !

@einarf
Copy link
Contributor

einarf commented Feb 9, 2017

fixed

@dperetti
Copy link
Author

dperetti commented Feb 9, 2017

Nope :

unquote() got an unexpected keyword argument 'encoding'

@einarf
Copy link
Contributor

einarf commented Feb 9, 2017

Must be a python 2 thing

@einarf
Copy link
Contributor

einarf commented Feb 9, 2017

Fixed. all good things are 5?

@dperetti
Copy link
Author

dperetti commented Feb 9, 2017

That was the one ! 👍

@einarf
Copy link
Contributor

einarf commented Feb 9, 2017

.. and I added unit test to verify the generated temp-url key.

Please close this issue if there is nothing more to add.

@einarf
Copy link
Contributor

einarf commented Feb 12, 2017

@Blacktorn @dperetti Can any one you close this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants