search

nbudin / devise_openid_authenticatable

OpenID authentication for Devise
MIT License
99 stars 32 forks source link

openid fields with Ruby 1.9.2 not working as expected #5

Closed mitnal closed 14 years ago

mitnal commented 14 years ago

Hi,

first of all thank you for your work.

I had a little problem with openid fields and ruby 1.9.2. Array.to_s returns now an escaped array: ["BLUB"].to_s => "BLUB" # 1.8.7

["BLUB"].to_s
=> "[\"BLUB\"]"  # 1.9.2-rc1

["BLUB"].join
=> "BLUB" # 1.8.7 and 1.9.2-rc1

So join works in ruby 1.8.7 and 1.9.2 as intended.

IMHO: I am not sure if it is a good idea to return a string instead of the array. What happens if a user has multiple email addresses? Would the returned value be something like: blub@example.comblim@example.com? Or is it not possible to get more then one email in return?

BTW a quickfix: when "http://axschema.org/contact/email" self.email = value[2..-3]

nbudin commented 14 years ago

Hmm. http://axschema.org/contact/email specifies that the value should be a string, not an array of strings. OTOH, I'm not sure whether it is possible to return more than one value for the same AX key and that may be why it's giving back an array.

I agree it's probably not a good idea to convert it to a string anyhow. The reason I had done it that way is because I was forcibly converting the keys to strings (sometimes they came back as YAML-library not-quite-strings) and so it seemed to make sense to convert the values as well. But on further reflection, the reason it was important that the keys be strings is because the user would end up having to do that in their openid_fields= function anyhow, when checking for the presence of certain keys. The same is probably not true for the values.

So my preferred fix would probably be to continue converting the keys to strings, but leave the values as they are. Would that make sense to you?

mitnal commented 14 years ago

Yes, that makes perfect sense to me.

I never had a problem with the key handling, just the values gave me some trouble because of ruby 1.9.

I have no idea if it is possible that http://axschema.org/contact/email returns more than one email, I am quite new to the whole OpenID business.

Thx.

nbudin commented 14 years ago

OK, I've corrected for this and published version 1.0.0.alpha5 on rubygems.org. Note that this change will require an additional check in your openid_fields= method; see the README for details.