Using email as login is broken
jip149 opened this issue · 13 comments
When trying to log in from masq interface with email_as_login: true in config, the user is logged in but https://www.le-pec.org/openid/giga@le-pec.org results in a 404.
What does the login field for this account in the database contain?
I've looked into it a bit more. Problem is, the url is localhost:3000/giga@le-pec.org
Rails routing thinks the account parameters is giga@le-pec and the format is org
The URL localhost:3000/giga@le-pec.org.hml works ok.
I see no easy way to have the url localhost:3000/giga@le-pec.org work.
I think we might be able to make this work by adding a routing constraint. Nevertheless I think we should not do this. Just because one is using his email to login does not mean, that the email should be the identifier. I think we should generate a proper url slug based on the email address or something like that, but having the email address in the openid seems weird.
Using constraint would work, but we would have to build a constraint that knows about the supported format of the controller. Not very DRY...
What identifier do you think could be used?
Using constraint would work, but we would have to build a constraint that knows about the supported format of the controller. Not very DRY...
Only if your client is using the URL + extension method for denoting format - you can always pass format explicitly as a parameter when making requests.
My thoughts:
- For standalone, I can't see a reason not to use route constraints (in fact, I'd like to know under what circumstances this would cause people problems?)
- Devise uses email as username by default
- If users have to login with their email address (eg, for LDAP or other multi-domain providers), but use a different identifier for their OpenID URL, they'll get confused
- I can't immediately think of a way to generate useful slugs for emails across multiple domains
Any progress on this?
Providing a patch to use a route constraint would be a trivial effort, but the maints need to make an executive decision about including it. :tumbleweed: ;-)
My vote would still be to just do that, since sane clients can/should to be sending format parameters or accept headers for format selection anyway - seems like an easy win.
I'm not sure of the constraint itself, if you know it, perhaps submitting a pull request would expedite it?
@dennisreimann said he didn't want to do that, and since I've heard nothing back since my last response, I haven't bothered with a PR.
If users have to login with their email address (eg, for LDAP or other multi-domain providers), but use a different identifier for their OpenID URL, they'll get confused
I can see your point, but I think that having an OpenID like https://www.le-pec.org/openid/giga@le-pec.org would also confuse those users. Like I said I don't think that's a good idea, but we also have to take care of cases that OpenIDs like that are already out there - I don't know whether or not someone already used an OpenID containing the email address, which would not work anymore after a potential change.
This leaves two possibilities:
- Quick fix: Use a routing constraint - everything is fine except for weird looking OpenID identifiers
- IMHO better: Migrate existing accounts with email as login to have a proper login, which can be used to generate the OpenID URL for that account. Users could still log in using their email address, but would have to explicitely specify a login, which would be used as the identifier.
Sorry for all of that. I messed it up when merging the pull request that allowed users to use their email address as login, because it did not take into account, that the login attribute should not be the email address in that case.
I can see your point, but I think that having an OpenID like https://www.le-pec.org/openid/giga@le-pec.org would also confuse those users. Like I said I don't think that's a good idea, but we also have to take care of cases that OpenIDs like that are already out there - I don't know whether or not someone already used an OpenID containing the email address, which would not work anymore after a potential change.
We already have an OpenID provider that uses this format. It's backed by LDAP and supports users on multiple domains, so the only canonical identifier that makes sense is email address. So, when we add an LDAP user, they already have an OpenID identity that authenticates with the same email/password combination as all the other services. The only other option I could see would be maybe being able to infer the domain part from the HTTP domain like some webmail applications do, but that's a whole other kettle of fish.
- Quick fix: Use a routing constraint - everything is fine except for weird looking OpenID identifiers
I'd suggest doing this for a start, at least it makes things work for people who are fine with that.
- IMHO better: Migrate existing accounts with email as login to have a proper login, which can be used to generate the OpenID URL for that account. Users could still log in using their email address, but would have to explicitely specify a login, which would be used as the identifier.
For my use-case, it's a lot easier to tell someone to enter https://id.domain.tld/<your-email-address> than, "Go to https://id.domain.tld, login using your email and password, enter some random thing that you will use as your identity, then use https://id.domain.tld/<random-thing-you-entered> to login to this other service." Of course, as users do, they'll forget the random thing they entered, or they'll enter something someone else has used and get an error, or screw it up some other way. It also means there's interaction required from the user to start using it, rather than simply being able to pre-populate from the backend and have them login using OpenID immediately.
I grant that my use-case is possibly atypical though.
I'll also say that users in my case don't know or care what 'OpenID' is, they just put whatever details they're told to into whatever boxes they're told to.
If this second option was chosen, it'd have to be optional for me, though I can see why it would be useful.
Okay, go ahead and implement the routing constraint. :)