Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for using any model field as identifier and not only :email. #119

Closed
wants to merge 6 commits into from

Conversation

wielinde
Copy link

Hi everybody,

Thank you for providing this Gem. It is a real pain killer. Unfortunately I ran into troubles when I did not have the email of a user. So I changed this gem for allowing other fields for identifying a model than only :email.

Please be so kind an consider this pull request.

Scenario

  • As a developer of mobile applications,
  • when I want to realize token based authentication after a Facebook login
  • and the user denied the provision of her email address
  • then I still want to be able to use token based authentication but using a model field that is different from "email".
  • Further, old configurations of simple_token_authentication should still work as before.

The solution in this pull request

I changed the format for header_names. It now provides the options identifier_field and identifier, where identifier holds the HTTP-Header-Name that shall be used for providing the identifier such as "X-User-Uuid". identifier_name holds a Symbol which tells which model field shall be used, such as ":email" or ":uuid".

Please feel free to contact me.

@gonzalo-bulnes
Copy link
Owner

Hello @wielinde,

I'm currently performing some refactoring to solve #104, and I'm applying some kind of feature freeze until that's solved. Be sure, however, I'll consider your PR then.

Meanwhile, since you didn't mention it, please do notice that any conversations about the implementation of this feature should take place in #68. It would be great if you could explain there how you implementation would be used (see also #90), so everyone involved can participate. I'm specially thinking about the name and structure of the new configuration options to provide, that's the most important thing to focus on. (The implementation itself won't be a problem.)

@nicolo
Copy link
Contributor

nicolo commented Jan 5, 2015

@gonzalo-bulnes What is left to do on this PR so it can be merged?

Sorry for being absent on this recently. I have some time now to pick things back up and would like to contribute to get this feature merged in.

Should we finish of this PR, go back to #90, or should I start a new one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants