GNOME Bugzilla – Bug 594046
OAuth support is broken
Last modified: 2009-10-28 21:56:02 UTC
Currently, tomboy sends oauth_callback=http://www.google.com/ during the token authorization request. If tomboy itself isn't spawning a temporary local web server to listen for callbacks on, it shouldn't be using oauth_callback. Also, oauth_callback is no longer sent during the authorization stage in 1.0a, which tomboy needs to be updated to support as well. Instead, it must send oauth_callback=oob (if it's not going to spawn a temporary local web server to listen for callbacks), during the request token request. Also, for 1.0a, there is now a verifier string (sent back via the callback, if callbacks are being used, or in body if not), and is required to be sent back to the server during the access token request. See http://oauth.net/core/1.0a for the full spec.
This is all true, thanks for the reminder. We'll at least fix the oauth_callback stuff, but I may not add support for 1.0a this cycle.
Ah, so I take it 1.0a is actually released now. Assuming it's supported properly in django-piston, I may try to tackle this for Monday's release. If you or anyone else would like to submit a patch today (preferably tested on Snowy), that would be great.
Oh, suck: "If the Consumer did not provide a callback URL, the Service Provider SHOULD display the value of the verification code, and instruct the User to manually inform the Consumer that authorization is completed. If the Service Provider knows a Consumer to be running on a mobile device or set-top box, the Service Provider SHOULD ensure that the verifier value is suitable for manual entry." So if we don't do the local server thing (a possible issue on non-Linux platforms), we have to do UI changes to let the user manually enter the verifier. :-/
Wouldn't it be possible to register a non-http url handler and let tomboy handle these? I'm doing this for Tomdroid by passing tomdroid://sync as a callback url, android makes it extremely easy to do this. This being said, I do not know if it is possible to register custom url handlers in windows or osx (or whatever platform I'm not thinking of).
(In reply to comment #4) > Wouldn't it be possible to register a non-http url handler and let tomboy > handle these? I'm doing this for Tomdroid by passing tomdroid://sync as a > callback url, android makes it extremely easy to do this. I thought about that, but... > This being said, I do not know if it is possible to register custom url > handlers in windows or osx (or whatever platform I'm not thinking of). On Windows and OS X there would be some issues getting the Tomboy UI to respond correctly to *any* out-of-process request. The safest thing to do is to show a page in the browser with instructions for continuing. However, doing this the way you suggest on Linux would probably be a lot nicer. Not enough time today to figure it out; maybe I'll change it before the stable release.
Fixed in 0.15.7. I haven't actually tested on any OAuth 1.0a servers. If you have any to test on, please do so ASAP so we can address bugs before the stable release.
(In reply to comment #5) > However, doing this the way you suggest on Linux would probably be a lot nicer. > Not enough time today to figure it out; maybe I'll change it before the stable > release. I would generally suggest avoiding custom URI handlers like this. The amount of work to get the infrastructure hooked up isn't any less than just having a simple web server start up, time out, and only accept connections from localhost. In fact, it's probably even more work. It also opens up another point of failure in the code. And an advantage to using HTTP, is that you can redirect the user back to a 'success' page on the site. You can't do that with a URL handler that isn't the browser, really. But also, OAuth wasn't designed to use it the way we are. It was designed to provide indirection for other web services to access your stuff on the web.
Good points Rodney. I think we will stick with the existing resolution for Tomboy (which is exactly what you suggested).
Yeah, I didn't realize that you were using a local temporary server now. Good choice. I'm REOPENing this though, as discussed yesterday in IRC, 1.0a support is still not correct.
Created attachment 144170 [details] [review] Patch for supporting OAuth 1.0a authentication This patch works with U1 server code now. I had to add oauth_callback parameter to the request token url. I guess snowy needs to support this, haven't tested it with snowy, but will later, when I get some free time
Created attachment 144406 [details] [review] Updated patch This patch includes a couple of fixes (getting the RootUri when pressing the connect button, not just when creating the OAuth object, which was giving 'URI is too short' exceptions when testing with ubuntu one server) and a missing (not sure why it got missed in the first patch) UrlEncode for the oauth_callback argument.
I don't understand why getting the Root object would ever result in an error; that seems odd to me. Will review patch this weekend.
If I've understood the code correctly, GetConfigSettings creates an empty OAuth object, so since the *Url properties were only been set in WebSyncPreferencesWidget.cs if the OAuth object was null, the *Url properties were empty. So moving the GetRoot code out of the "if (oauth == null)" block fixed it for me. Surprisingly, I only saw this when testing with the U1 server, not with a locally running instance. So let me know if it needs some other change. Also, will test snowy as soon as I get the U1 production server fully working
I just tested this patch with Snowy and it works. U1 seems to be having issues so I can't properly test it with that. I'll try to have it full reviewed by tomorrow to correspond with your Karmic deadlines.
Just to verify, is the patch from 9/30 the latest version of the patch?
Created attachment 145135 [details] [review] Patch from Karmic package I think the patch I attached is the latest, but just in case, re-attaching what we have in the karmic package.
The patches are identical. You said in IRC that you had trouble syncing to Snowy. Any chance you have some debug output from that?
Comment on attachment 145135 [details] [review] Patch from Karmic package Pushed to master and gnome-2-28 branch.
Patch applied, tested with U1 and Snowy, no problems. Did change whitspace a bit but otherwise identical to posted patch. Thanks everyone!