Openstreetmap as a relyingparty for OpenID #2500
Openstreetmap as a relyingparty for OpenID #2500
It would probably have been better to talk to us first before you spent a lot of time on this, but thanks for your work and I will review it fully as soon as possible.
One obvious question is what other openid plugins did you consider, if any (there are a number I believe) and what made you pick that one over any others?
As far as the code goes, from a quick scan of the patch I'm guessing that you should probably be adding an index on the new column that you've added to users? You should also consider what indexes and foreign key constraints are appropriate on the two new tables that you've added.
I think I would also prefer openid_url to identity_url for the new field on the user table.
Replying to [comment:1 tom[at]compton.nu]:
Thanks for reviewing it. I haven't spent too much time on it yet and it was really just a proof of concept to see how much work would be involved to implement it and it turned out it was less than I had thought. So treat this as an initial question if OpenID is something that is potentially acceptable for the main site together with a patch to indicate that I would be willing to help implement it.
Yes, I think I came accross three different ones, although to be honest I can't remember which ones I looked at. I think I chose this one, as it was the simplest one to work with and seemed to do all that is required, i.e. one can give it an OpenID url and it tells you if the user can authenticate the url. The other ones tied in deeper into the account system and given we don't want to replace the existing accounts, they were more problematic. But I'll need to check again to make sure I remember it correctly.
Good point, yes the user column should have an index. The other tables were created by the plugin it self. So I assumed it was in the way it needed it, but I'll have a close look to see what those actually are needed for.
That at least is an easy change.
I have finally got around to incorporating some of the feedback received here in the ticket and via IRC. It is still the same flavor as before, so it doesn't integrate any tighter into the rest of the user authentication code as suggested by zerebubuth yet, but at least it should fix some of the bugs and I think it would be an acceptable interims solution, but that is obviously up to you to decide.
Changes from previouse version:
I have attached two versions of the patch. The first patch is the actual code written and is smaller, whereas the second one includes the entire open_id_authentication plugin in addition and should be ready to apply and try out.
Further comments are welcome
As a status update:
There is now a branch in svn for the OpenID work at http://trac.openstreetmap.org/browser/sites/rails_port_branches/openID
And also a test instance running on the dev server if anyone wants to try it out: http://openid.dev.openstreetmap.org/
I guess any comments and improvement suggestions would still best be added to this ticket.
Tested with [http://siege.org/projects/phpMyID/ phpMyID 0.9] and works fine.
The only issue is that it's not clear that you don't need a password, it would probably be worth adding some text explaining this:
"You can leave the password field blank when logging in with OpenID, however if you want to access other site in the Openstreetmap project that use openstreetmap.org account info you will need to set a password."
[Submitted to the original trac issue database at 1.33pm, Tuesday, 24th November 2009]
Having to remember a new username and password for each new website is annoying and error prone and so being able to have a single sign on solution such as OpenID for OpenStreetMap would be nice and potentially even reduce the barrier of entry.
Attached is an initial version for implementing OpenID (as a relying party) on the main site of OpenStreetMap.
In this implementation user accounts continue to exist exactly as before including the need for email verification, however in addition to the possibility to login via username and password, with this patch it is now also possible to associate an OpenID to an existing account and login via your OpenID.
There are a couple of caveats with this though.
For one, this patch only implements OpenID login for the main website, however the OSM account credentials are needed at several other places too. E.g. talking to the API, logging in to the forum or trac, which means it is still necessary to have a password if one wants to use these services. For the API, OAuth provides an alternative and at least if you only use OAuth enabled clients, it is not necessary to have a password anymore. The wiki uses a separate user account anyway and with the mediawiki OpenID plugin this could benefit too. So I think this is a useful first step with other services to hopefully follow to make the experience even smoother.
The second caveat is that this is my first patch to the OpenStreetMap code base and indeed by first encounter of ruby or rails. So I suspect there are some problems with coding style and potentially bugs. It is a fairly short patch though so I should be able to fix up any comments fairly easily.
This patch contains one migration and in addition needs the open_id_authentication plugin ( http://github.com/rails/open_id_authentication )
The text was updated successfully, but these errors were encountered: