Discuss: Why not add fields as param to build_from_identity_url

[joshco]

For discussion not approval/merge. Please share your thoughts

I've started using this openid module in conjunction with existing database_authenticatable users in my app. I looked at the wiki doc on strategy

https://github.com/nbudin/devise_openid_authenticatable/wiki/Using-database_authenticatable-and-openid_authenticatable-together

But this seems unnecessarily complex when dealing with the cases

• Matching an openid identity to an existing user (via email)
• creating a new user from an openid identity

Because the gem's workflow first creates a new user (when no identity url matches) but does not pass along the fields.

My patch includes the fields into the build class method on the resource (User). This allows one to handle these cases IMHO more simply with a method like:

def self.build_from_identity_url(identity_url,fields=nil)


    email = fields['http://openid.net/schema/contact/internet/email'].try(:[],0)
    name = fields['http://axschema.org/namePerson'].try(:[],0)
    first=fields['http://axschema.org/namePerson/first'].try(:[],0)
    last=fields['http://axschema.org/namePerson/last'].try(:[],0)

    mobile = fields['http://openid.net/schema/contact/phone/business'].try(:[],0)

    if user=User.find_by_email(email)
      user.identity_url=identity_url
    else
      user=User.new(:identity_url => identity_url)
      user.email=email
      user.name= name
      user.given_name=first
      user.family_name=last
      user.mobile=mobile
      user.password=SecureRandom.hex(10)
    end

    user

  end

[nbudin]

This seems totally reasonable to me. Would you mind writing a test or two to verify that this workflow works, and possibly updating the docs in your PR?

[joshco]

Great. I will do that. Right now I'm finishing up an update to my app including this feature. I'll circle back in about a month with this (and any other issues that arise). Feel free to nag me if needed :)

[joshco]

Having a little trouble. How do I make it so the login form can also work with database_authenticatable? I tried doing this by having two forms. One for openID, and another for user/pass. However, it always goes to openID auth, even if user/pass is present in the form. How do I selectively choose openID or user/pass?

My login form is here: http://joshco.marriagehero.org/users/sign_in

(ActionID is the openID provider)

[nbudin]

@joshco: the key is the valid? method of the strategy class. A Warden strategy will attempt to authenticate if the valid? method returns true.

You can see the code for ours here: https://github.com/nbudin/devise_openid_authenticatable/blob/master/lib/devise_openid_authenticatable/strategy.rb#L6

I'm not sure why it would be returning true in your case, but my guess is that it must be.