[PATCH] Update OpenIdAuthentication to use Ruby OpenID 2.x.x

http://groups.google.com/group/rubyonrails-core/browse_thread/thread/177e3155432c5a56/

patch seems to work great, accept for one minor thing. The migration should either not accept a name at the command line, or it should use the provided name for the migration class. The generated migration would not run because there was a mismatch between the filename and the migration class name. Change the first line of the template to:

class <%= class_name %> < ActiveRecord::Migration

fixes the issue.

Found a small hiccup - in templates/migration.rb, line 11 reads:

      t.string :server_url, :salt, :null => false

I recommend changing that to:

      t.string :server_url, :null => true
      t.string :salt, :null => false

Reason being in ruby-openid (2.0.2) if authenticating with a OpenIDv1 server the 'server_url' is set to a blank string which Rails attempts to save as a NULL.

This is set in lib/openid/consumer/idres.rb of the library - line 277:

      def check_nonce
        case openid_namespace
        when OPENID1_NS
          nonce =
            @message.get_arg(BARE_NS, Consumer.openid1_return_to_nonce_name)

          # We generated the nonce, so it uses the empty string as the
          # server URL
          server_url = ''
        when OPENID2_NS
          nonce = @message.get_arg(OPENID2_NS, 'response_nonce')
          server_url = self.server_url
        else
          raise StandardError, 'Not reached'
        end

Here's the error thrown:

ActiveRecord::StatementInvalid (Mysql::Error: Column 'server_url' cannot be null: INSERT INTO `open_id_authentication_nonces` (`salt`, `timestamp`, `server_url`) VALUES('abcxyz', 1199901999, NULL)):
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/connection_adapters/abstract_adapter.rb:150:in `log'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/connection_adapters/mysql_adapter.rb:281:in `execute'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/connection_adapters/abstract/database_statements.rb:156:in `insert_sql'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/connection_adapters/mysql_adapter.rb:291:in `insert_sql'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/connection_adapters/abstract/database_statements.rb:44:in `insert_without_query_dirty'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/connection_adapters/abstract/query_cache.rb:19:in `insert'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/base.rb:2272:in `create_without_callbacks'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/callbacks.rb:226:in `create_without_timestamps'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/timestamp.rb:29:in `create'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/base.rb:2238:in `create_or_update_without_callbacks'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/callbacks.rb:213:in `create_or_update'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/base.rb:1972:in `save_without_validation'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/validations.rb:934:in `save_without_transactions'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/transactions.rb:108:in `save'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/connection_adapters/abstract/database_statements.rb:66:in `transaction'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/transactions.rb:80:in `transaction'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/transactions.rb:100:in `transaction'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/transactions.rb:108:in `save'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/transactions.rb:120:in `rollback_active_record_state!'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/transactions.rb:108:in `save'
    /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.0.2/lib/active_record/base.rb:572:in `create'
    /vendor/plugins/open_id_authentication/lib/open_id_authentication/db_store.rb:51:in `use_nonce'
    /usr/local/lib/ruby/gems/1.8/gems/ruby-openid-2.0.2/lib/openid/consumer/idres.rb:303:in `check_nonce'
    /usr/local/lib/ruby/gems/1.8/gems/ruby-openid-2.0.2/lib/openid/consumer/idres.rb:76:in `id_res'
    /usr/local/lib/ruby/gems/1.8/gems/ruby-openid-2.0.2/lib/openid/consumer/idres.rb:58:in `initialize'
    /usr/local/lib/ruby/gems/1.8/gems/ruby-openid-2.0.2/lib/openid/consumer.rb:346:in `handle_idres'
    /usr/local/lib/ruby/gems/1.8/gems/ruby-openid-2.0.2/lib/openid/consumer.rb:385:in `complete_id_res'
    /usr/local/lib/ruby/gems/1.8/gems/ruby-openid-2.0.2/lib/openid/consumer.rb:289:in `complete'
    /vendor/plugins/open_id_authentication/lib/open_id_authentication.rb:104:in `complete_open_id_authentication'
    /vendor/plugins/open_id_authentication/lib/open_id_authentication.rb:151:in `timeout_protection_from_identity_server'
    /vendor/plugins/open_id_authentication/lib/open_id_authentication.rb:104:in `complete_open_id_authentication'
    /vendor/plugins/open_id_authentication/lib/open_id_authentication.rb:89:in `authenticate_with_open_id'
    /app/controllers/sessions_controller.rb:31:in `open_id_authentication'
    /app/controllers/sessions_controller.rb:14:in `create'

Patch works for me. +1

I can confirm the patch works, and the code looks good. +1

Works for me. +1

Works for me too. +1

+1 old version was no longer usable

+1 works for me perfectly

+1 lets get this committed

It doesn't apply cleanly anymore. I also saw a few questionable lines of code:

new.new?

      def open_id_store
        case store
        when :db
          OpenIdAuthentication::DbStore.new
        when :file
+         OpenID::Store::Filesystem.new.new(OPEN_ID_AUTHENTICATION_DIR)
        else
          raise "Unknown store: #{store}"
        end
      end

Looks like the redirection is adding the protocol and port twice...

+     def open_id_redirect_url(open_id_request)
+       open_id_request.return_to_args['open_id_complete'] = '1'
+       open_id_request.redirect_url(request.protocol + request.host_with_port + "/", requested_url)
      end
+ 
+     def requested_url
+       "#{request.protocol + request.host_with_port + request.relative_url_root + request.path}"
      end

Somethings wrong w/ the diff you downloaded. I repatched mine and it was fine.

Updated the diff, lets just blame it on Trac ;)

I blame svn, it still doesn't apply cleanly. That's fine, but it doesn't look like this will work. Why is new.new being called? Why is the redirect_url adding the protocol and host to request_url when the request url already has it? Do you have this running in an app already?

Replying to technoweenie:

Why is new.new being called? Why is the redirect_url adding the protocol and host to request_url when the request url already has it? Do you have this running in an app already?

Fixed, Fixed. I've tested it in a sample app and production app myself. Not sure why that double request_url didn't cause any failures. I would of hoped all those people who +1 it actually tested it?

Give this pastie one a try. curl http://pastie.caboo.se/pastes/149074/download | patch -p0

Tested it? Not extensively, but I was at least able to login with the patch.

First of all: Thanks for providing this patch! It does however seem to be not quite perfect so far as it fails with

"return_to" is mandatory for OpenID 1 requests

in my setup.

The problem is that ruby-openid 2.0.3 expects a second (return_to) parameter passed to Consumer::CheckIDRequest.get_message() when interacting with OpenID 1 servers (e.g. if @message.is_openid1).

This is easily fixed by changing line 143 from lib/open_id_authentication.rb from

open_id_request.redirect_url(requested_url)

to

open_id_request.redirect_url(requested_url, requested_url)

It might be a better idea to "fix" this in ruby-openid by just using realm as return_to if it isn't specifically passed but i don't know enough about the ruby-openid internals to decide that..

Unverified, still needs more testing with edge cases.

I didn't realize there were so many differences, even between working with openid 1 and 2. Lets work out some more bugs before we continue.

Replying to technoweenie:

I blame svn, it still doesn't apply cleanly. That's fine, but it doesn't look like this will work.

svn the problem, lets try this over git ;)

http://github.com/josh/open_id_authentication/tree/ruby_openid_2

Incidentally, in order for the example in the readme to work for handling different status types (:missing, :canceled, :failed, :successful) in the open_id_authentication method, the OpenIdAuthentication#complete_open_id_authentication must be changed to return the appropriate symbols rather than their corresponding error messages stored in the Result hash.

diff -Naur a/lib/open_id_authentication.rb b/lib/open_id_authentication.rb
--- a/lib/open_id_authentication.rb	2008-02-13 12:29:02.000000000 -0800
+++ b/lib/open_id_authentication.rb	2008-02-13 12:20:22.000000000 -0800
@@ -106,13 +106,13 @@
 
       case open_id_response.status
       when OpenID::Consumer::SUCCESS
-        yield Result[:successful], identity_url, OpenID::SReg::Response.from_success_response(open_id_response)
+        yield :successful, identity_url, OpenID::SReg::Response.from_success_response(open_id_response)
       when OpenID::Consumer::CANCEL
-        yield Result[:canceled], identity_url, nil
+        yield :canceled, identity_url, nil
       when OpenID::Consumer::FAILURE
-        yield Result[:failed], identity_url, nil
+        yield :failed, identity_url, nil
       when OpenID::Consumer::SETUP_NEEDED
-        yield Result[:setup_needed], open_id_response.setup_url, nil
+        yield :setup_needed, open_id_response.setup_url, nil
       end
     end

Replying to watsonian:

Incidentally, in order for the example in the readme to work for handling different status types (:missing, :canceled, :failed, :successful) in the open_id_authentication method, the OpenIdAuthentication#complete_open_id_authentication must be changed to return the appropriate symbols rather than their corresponding error messages stored in the Result hash.

No, Result#=== matches symbols.

Replying to josh:

Give this pastie one a try. curl http://pastie.caboo.se/pastes/149074/download | patch -p0

I just tried this and it still does not apply cleanly.

Here is the hunk that fails on open_id_authentication.rb:

***************
*** 1,16 ****
  module OpenIdAuthentication
    OPEN_ID_AUTHENTICATION_DIR = RAILS_ROOT + "/tmp/openids"
-   
    def self.store
      @@store
    end
-   
    def self.store=(value)
      @@store = value
    end
-   
    self.store = :db
-   
    def store
      OpenIdAuthentication.store
    end
--- 1,19 ----
+ require 'uri'
+ require 'openid/extensions/sreg'
+ 
  module OpenIdAuthentication
    OPEN_ID_AUTHENTICATION_DIR = RAILS_ROOT + "/tmp/openids"
+ 
    def self.store
      @@store
    end
+ 
    def self.store=(value)
      @@store = value
    end
+ 
    self.store = :db
+ 
    def store
      OpenIdAuthentication.store
    end

It looks like the original file already has the "require 'uri'" line but it isn't in the context of the diff.

The patch also fails on init.rb:

***************
*** 1,5 ****
  begin
-   require 'openid'  
  rescue LoadError
    begin
      gem 'ruby-openid'
--- 1,5 ----
  begin
+   require 'openid'
  rescue LoadError
    begin
      gem 'ruby-openid'

The 2nd line in the original file is:

  gem 'ruby-openid', '=1.1.4'

A request to support sreg_request.policy_url and I offer help for those having trouble applying the patch.

1) Simple Registration Extension (sreg) supports a link to a privacy policy. If you don't provide the link, then myopenid.com will complain to the user the the site is requesting info, w/out providing a privacy policy. A one line patch to open_id_authentication.rb adds support for the privacy policy (this patch is for after the main patch above is applied).

--- open_id_authentication.rb.orig      2008-03-01 03:23:47.000000000 -0800
+++ open_id_authentication.rb   2008-03-01 03:25:36.000000000 -0800
@@ -135,6 +135,7 @@
       sreg_request = OpenID::SReg::Request.new
       sreg_request.request_fields(Array(fields[:required]).map(&:to_s), true) if fields[:required]
       sreg_request.request_fields(Array(fields[:optional]).map(&:to_s), false) if fields[:optional]
+      sreg_request.policy_url = fields[:policy_url] if fields[:policy_url]
       open_id_request.add_extension(sreg_request)
     end

Then you can send a privacy policy when calling authenticate_with_open_id. For example:

    authenticate_with_open_id(params[:openid_url],
        :required => [ :nickname ],
        :optional => [ :email, :fullname, :postcode, :country, :language],
        :policy_url => 'http://mykewlsite.com/myprivacy.html' ) do |result, identity_url, registration|
...

2) For those of you having trouble with the patch, the patch was diffed last on 02/09/08 while DHH checked in mods to SVN that broke it on 02/14/08 So, if you would like to use the current version of the patch, you need to get an earlier version of the plugin. For example, the following works:

  svn co -r '{2008-02-09}' http://svn.rubyonrails.org/rails/plugins/open_id_authentication/
  cd open_id_authentication
  wget 'http://dev.rubyonrails.org/attachment/ticket/10604/update_openid_plugin_to_ruby_openid_2.diff?format=raw'
  patch -p1 < 'update_openid_plugin_to_ruby_openid_2.diff?format=raw'

A patch for open_id_authentication.rb to fix bug when an open_id_response.endpoint is not returned.

I encountered this problem when verifying an OpenId against myopenid.com with an already used nonce:

NoMethodError (You have a nil object when you didn't expect it! The error occurred while evaluating nil.claimed_id):

/vendor/plugins/open_id_authentication/lib/open_id_authentication.rb:103:in `complete_open_id_authentication'

When debugging I discovered:

#<OpenID::Consumer::FailureResponse:0x3130700 @reference=nil, @message="Nonce already used or out of range: \"2008-03-03T19:05:00ZahTJoH\"", @contact=nil, @endpoint=nil>

to fix I patched:

*** open_id_authentication.rb.orig      Mon Mar  3 11:40:21 2008
--- open_id_authentication.rb   Mon Mar  3 11:46:07 2008
***************
*** 100,106 ****
      def complete_open_id_authentication
        params_with_path = params.reject{ |key, value| request.path_parameters[key] }
        open_id_response = timeout_protection_from_identity_server { open_id_consumer.complete(params_with_path, requested_url) }
!       identity_url     = normalize_url(open_id_response.endpoint.claimed_id) if open_id_response.endpoint.claimed_id
  
        case open_id_response.status
        when OpenID::Consumer::SUCCESS
--- 100,108 ----
      def complete_open_id_authentication
        params_with_path = params.reject{ |key, value| request.path_parameters[key] }
        open_id_response = timeout_protection_from_identity_server { open_id_consumer.complete(params_with_path, requested_url) }
!       
!       # if this fails with invalid nonce, there will be no endpoint
!       identity_url = normalize_url(open_id_response.endpoint.claimed_id) if open_id_response.endpoint && open_id_response.endpoint.claimed_id
  
        case open_id_response.status
        when OpenID::Consumer::SUCCESS

Yahoo's OpenId provider requests the return_to url with no parameters before loading the authentication page on their end. This was causing an exception to be thrown form OpenIdAuthentication#normalize_url line 65.

To prevent this from happening, I have added a short circuit on blank identity_urls in authenticate_with_open_id.

Here is a patch:

*** open_id_authentication.rb   Thu Mar  6 15:27:53 2008
--- open_id_authentication.rb.orig      Mon Mar  3 11:40:21 2008
***************
*** 81,87 ****
      end
  
      def authenticate_with_open_id(identity_url = params[:openid_url], fields = {}, &block) #:doc:
-       return false if identity_url.blank?
        if params[:open_id_complete].nil?
          begin_open_id_authentication(normalize_url(identity_url), fields, &block)
        else
--- 81,86 ----
***************

I would have expected them to request the realm, not the return_to. But it's also possible they do both, I haven't checked.

I've been having trouble parsing OpenID from LiveJournal, using the OpenIDAuthentication plugin, patched w/ the diff attached on this ticket, ruby-openid 2.0.4, and rails 2.0.2.

I tracked it down in the Aptana debugger, and it seems I get a type_uri mismatch - "http://openid.net/signon/1.0" vs "http://openid.net/signon/1.1" - in the "verifying discovered services" stage. I slightly suspect that this is a difference between what's reported from e.g. http://saizai.livejournal.com vs http://www.livejournal.com/openid/server.bml; the former says it's 1.1, the latter 1.0.

http://openidenabled.com/ruby-openid/trunk/examples/consumer seems to work fine with my livejournal ID.

How can I fix this?

Hi all,

I'm getting the following error when I try to apply the new patch:

...d_authentication> patch -p0 < update_openid_plugin_to_ruby_openid_2.diff

patch: **** Only garbage was found in the patch input.

Could anyone check that the file didn't get corrupted when uplodaded to the server? If not, I don't have a clue what's wrong.

Without the patch I get the famous error: uninitialized constant OpenID::Store

My environment: rails: 2.0.2 ruby-openid : 2.0.4 mongrel:1.1.4

Any ideas?

The patch works fine.

I'm sorry I had a problem downloading it.

(In [9103]) Updated OpenIdAuthentication to use Ruby OpenID 2.x.x gem (closes #10604) [Josh Peek]

According to specs the form field's "name" attribute SHOULD have the value "openid_identifier".

May I make patch to fix it?

Replying to priit:

According to specs the form field's "name" attribute SHOULD have the value "openid_identifier". May I make patch to fix it?

The plugin doesn't offer any html helpers so that is up to you to add to your form.

i also had the exception and problems as described in http://dev.rubyonrails.org/ticket/10604#comment:32 and http://dev.rubyonrails.org/ticket/10604#comment:33

please commit the two helping patches from these links!