johvargas01 / oauth-signpost

Automatically exported from code.google.com/p/oauth-signpost
0 stars 0 forks source link

Using Signpost 1.2 with Digg API requires extensive changes to signpost behavior #31

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
First off, I'd like to say thank you to Mattias for a truly useful contribution 
to the java community.  I've used signpost 
(mostly 1.1) against three APIs (tripit, opentable, and now digg), and while 
it's often required some heavy modification 
and experimentation, I've generally been successful.  

But that being said, then along came Digg.  I spent two days trying to get 
signpost 1.2 working with the Digg API, and 
was about to throw in the towel before I finally found this thread from last 
month:

http://groups.google.com/group/diggapi/browse_thread/thread/51472d4c8ede6c8b/832
b74a731245824

If you follow the recommendations in all three pastie.org posts, then make a 
few tweaks to fix obvious errors, then set 
the Content-Type to OAuth.FORM_ENCODED, the proposed solution will work.

But it requires overloading FIVE signpost classes, and copying and pasting a 
majority of code from each one.

Surely there must be a way to access the Digg API from signpost that doesn't 
require overloading so many classes, let 
alone copying and pasting so much code.

As far as I can tell, Digg doesn't seem to be violating the oauth 1.0a 
specification, so it would be nice if signpost could 
handle this api gracefully.

Thanks again for the great work!

Original issue reported on code.google.com by mbur...@gmail.com on 10 Mar 2010 at 1:15

GoogleCodeExporter commented 8 years ago
that's a whole lot of stuff that was posted there. It seems that Digg requires 
a lot
of stuff that's beyond the standard. I noticed a couple things about your
modifications though, which are not (or won't be -- with the upcoming version) 
necessary:

1) As of 1.2.1-SNAPSHOT, POST is the default HTTP methods for getting tokens

2) even now, there is no hacking required to add a realm parameter.
With 1.2, just use OAuthProvider.setRequestHeader("Authorization", "OAuth
realm=www.digg.com") -- Signpost will honor any Auth header params set prior to
message signing
With 1.2.1-SNAPSHOT, that method is deprecated. You can use the new
OAuthProviderListener.prepareRequest(request) hook. It allows you to 
pre-configure
the requests that are sent by OAuthProvider.

3) You mentioned rewriting the sign() method to remove the oauth_token 
parameter.
That, too, should not be necessary. As of 1.2, if the token is blank, it's not 
added
to the signature anymore. You CAN force a consumer to do so, however, by calling
setSendEmptyTokens(true). I'm fairly sure that works as expected, so not sure 
why you
wanted to rewrite sign() here.

The remaining things are a bit difficult to handle. The thing about the nonces 
is
highly Digg specific; I won't add any provider specific code to Signpost, that 
only
encourages them to do weird things.

The other thing is the oauth_callback problem. For some reason, some providers 
don't
allow it in the Auth header (e.g. Twitter). That's wrong of course, it should be
allowed anywhere. Fixing this to work for either situation requires some 
hacking,
unfortunately. I'll see if I can come up with a solution that feels right and 
chose
sensible defaults (I tend to default to those cases which cover the more popular
providers, and Twitter is probably by far the most popular one...)

Anything else on your list?

Original comment by m.kaepp...@gmail.com on 12 Mar 2010 at 3:26

GoogleCodeExporter commented 8 years ago
btw, Digg is just as wrong as Twitter if they only look in the Auth header for a
callback. The standard nowhere states that it's required to have all OAuth 
parameters
in one place. They really should look in all 3 allowed places (query string, 
post
body and Auth header) and merge them.

Original comment by m.kaepp...@gmail.com on 12 Mar 2010 at 3:29

GoogleCodeExporter commented 8 years ago
I think I managed to fix this. I changed the provider code to be able to pass
additional parameters to OAuthConsumer manually (such as callbacks and 
verifiers),
which will go directly into the signer, and then let the consumer decide what 
to do
with them (i.e. where to write them), and apparently this also works with 
Twitter now.

Must have been an error on my side all the way along, but I can't really tell 
what
went wrong.

Original comment by m.kaepp...@gmail.com on 12 Mar 2010 at 4:38

GoogleCodeExporter commented 8 years ago
Nice!  I'm simplifying my hackjob based on your comments and 1.2 right now.  
When I 
have a sec I'll try switching over to the tip and see if that makes it even 
simpler

Original comment by mbur...@gmail.com on 12 Mar 2010 at 4:47

GoogleCodeExporter commented 8 years ago
cool, try with these updated JARs, I've fixed and flexibilitized (is that even a
word??) some major parts of the llibrary.

Original comment by m.kaepp...@gmail.com on 12 Mar 2010 at 4:50

Attachments:

GoogleCodeExporter commented 8 years ago
Okay, I removed the code to handle realm specially and I simplified 
DiggSignatureBaseString, but 
unfortunately most of the other changes needed to stay in.  DiggOAuthProvider 
might be able to be 
simplified but not by much.

I've attached the source which authenticates with digg against signpost 1.2.

I tried switching over to the 1.2.1 snapshot, but enough of the interfaces were 
different that I ran 
into trouble so I abandoned it mostly for unrelated technical reasons (not 
familiar with git, couldn't 
figure out how to download and generate source jars, etc).

Original comment by mbur...@gmail.com on 12 Mar 2010 at 5:35

Attachments:

GoogleCodeExporter commented 8 years ago
If you had to hack the SBS implementation, then my guess is that Digg is way 
off the 
OAuth standard. The SBS specification does not leave any room for provider 
specific 
modifications, and Signpost's current SBS implementation is correct.

I must say that I don't feel very inclined to act on this. 1.2.1 already allows 
you 
to override basically any step in the token exchange. The signer and SBS 
implementations are conformous, and work with many popular OAuth providers out 
there.

Please try using 1.2.1 and see which issues remain. Then please compile a 
concise 
list of things that you think are broken in Signpost or not flexible enough to 
make 
it work with Digg.

Original comment by m.kaepp...@gmail.com on 14 Mar 2010 at 10:16

GoogleCodeExporter commented 8 years ago
Would be good to get a source code review on this by someone who is familiar 
with the
Digg API.

Original comment by m.kaepp...@gmail.com on 24 Mar 2010 at 4:19

GoogleCodeExporter commented 8 years ago
I have the Digg APIs working with Signpost 1.2.1.1 with just two minor tweaks.  
The first is straightforward:  There needs to be a null check in 
AbstractOAuthConsumer.collectBodyParameters on payload.  Because Digg requires 
the content type be set even if there is no content, this method will blow up.
The second issue is a bit trickier, and could just be my lack of understanding. 
 As mentioned above the realm must be specified in the Authorization header.  
In my listener prepareRequest, I have this line: 
request.setHeader(OAuth.HTTP_AUTHORIZATION_HEADER, "OAuth 
realm=http://services.digg.com/");
I see code has been added to AuthorizationHeaderSigningStrategy to handle this, 
but it never gets triggered.  The reason is that in the sign method this line:
collectHeaderParameters(request, requestParameters)
tries to take anything currently in the header and store it requestParameters, 
which are later written back into the header in the write signature method.  
But this collectHeaderParameters method doesn't actually work.  If you debug 
down to this call:
HttpURLConnectionRequestAdapter.getHeader(name) 
{connection.getRequestProperty(name)}
The value of name is "Authorization".  The implementation of connection is
sun.net.www.protocol.http.HttpURLConnection
If you look at the code behind this method it ignores values of:
"Proxy-Authorization" and "Authorization"
So this method always returns null, and anything set in Authorization header is 
not copied to requestParameters and eventually lost.  The same issue occurs in 
the sign method at this line:
OAuth.debugOut("Auth header", request.getHeader("Authorization"))
It always returns null.
This is only a problem in HttpURLConnectionRequestAdapter.  HttpRequestAdapter 
does not have this problem.

Thanks for the great package.  And thanks to the original poster of this issue. 
 I would have never got this working without this thread.

Original comment by dave.f.c...@gmail.com on 11 Jun 2010 at 1:19

GoogleCodeExporter commented 8 years ago
Any example code that you guys could share? Connecting to Digg with signpost 
1.2.1.1

Original comment by rga...@gmail.com on 12 Aug 2010 at 3:57

GoogleCodeExporter commented 8 years ago
dave could you attach the Digg API's that work with 1.2.1.1 with your tweaks?

Original comment by rga...@gmail.com on 13 Aug 2010 at 2:22

GoogleCodeExporter commented 8 years ago
I face the same realm problem from OAuth Provier (www.douban.com). 

After experiment, OAuthProvider.setRequestHeader("Authorization", "OAuth
realm=www.digg.com") will fail as request.getRequestProperty("Authorization") 
will always return null. Please refer to 
http://stackoverflow.com/questions/2864062/getrequestpropertyauthorization-alway
s-returns-null.

Anyone get it work with 1.2? Thanks.

Original comment by wangyizhuo on 7 Dec 2010 at 4:25

GoogleCodeExporter commented 8 years ago
I just saw Dave's post and it would save me a lot of time if I have seen it 
earlier. So is this issue fixed in 1.2.1.1 or any work around without modifying 
signpost's source code?

Thanks.

Original comment by wangyizhuo on 7 Dec 2010 at 4:28