search

7digital / SevenDigital.Api.Wrapper

A fluent c# wrapper for the 7digital API
MIT License
16 stars 29 forks source link

Use https://github.com/danielcrenna/oauth to sign requests #116

Closed mattgray closed 10 years ago

mattgray commented 10 years ago

Pretty similar to https://github.com/7digital/SevenDigital.Api.Wrapper/pull/114, but broken down into smaller commits See per-commit comments :)

AnthonySteele commented 10 years ago

Looks good to me.

AnthonySteele commented 10 years ago

not directly related, is HttpClientWrapper still needed? GzipHttpClient seems to be a superset of it, i.e. handles responses that may or may not be gzipped

gregsochanik commented 10 years ago

Doesn't look like it is still needed. Might be worth looking at that as a separate issue / PR, where we can decide on a good strategy for cleaning this kind of thing up.

I'm just locally merging and testing this now...!

gregsochanik commented 10 years ago

All works fine, apart from a url encoding issue I'm having.

Certain characters are causing an Invalid signature to be returned from the api when using the ~/oauth/requestToken/authorise to sign in.

I had a test that's now failing:

Scenario: As a user with a really awful password that contains non standard chars (e.g. !n$%£()#@&�) I can sign up with 7digital So when I log in using Sonos I can access my account

This test works with the master Api Wrapper urlsigning code, but not with this branch. The chars I've highlighted as breaking it are ! and %, the others work fine.

Delving reveals that you need to replace OAuthTools.UrlEncodeRelaxed(collection[key]); with OAuthTools.UrlEncodeStrict(collection[key]); in DictionaryExtensions.ToQueryString

If you do that then everything works!

mattgray commented 10 years ago

Just pushed a commit which should fix your bug Greg

gregsochanik commented 10 years ago

Sorted - I'm ok to merge this too. @raoulmillais ?

raoulmillais commented 10 years ago

I haven't had a chance to look yet I'm afraid. We're going to peg our version after the merge so we don't have to integrate yet as we're on a deadline for other things at the moment. I'm happy if you and Anthony are happy :)