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

Would it be useful to integrate login with username ? #127

Closed
nicolas-besnard opened this issue Feb 1, 2015 · 12 comments
Closed

Would it be useful to integrate login with username ? #127

nicolas-besnard opened this issue Feb 1, 2015 · 12 comments

Comments

@nicolas-besnard
Copy link
Contributor

Has mentionned in #126

@lynndylanhurley
Copy link
Owner

I just scanned the devise documentation and found this:
https://github.com/plataformatec/devise/wiki/How-To:-Allow-users-to-sign-in-using-their-username-or-email-address

So it is possible with standard devise, but the process seems to be rather involved. Also, I'm not sure if this gem will require any modifications to accommodate for this kind of setup.

And in addition to following the steps from the wiki, a new config option will need to be added to ng-token-auth to specify which params are sent to the sessions controller (i.e. username instead of email).

I think the best course of action is to add a test suite that follows the example in the devise wiki. If it works, then that's great, and if not then we can modify this gem as is needed. I'm short on time right now, but I'll get around to it as soon as I can.

@nicolas-besnard
Copy link
Contributor Author

I'm available to do it, but I don't use to work with MiniTest :( I'll try tomorrow to work with MiniTest

@lynndylanhurley
Copy link
Owner

Thanks @nicolas-besnard - I appreciate any help I can get 😃

To get the dev environment set up, you can follow these steps.

I recommend the following approach to testing this feature:

  1. Create a new model and migration in the test app that follows the example in the devise wiki. The existing models are here - you can just follow those for examples.
  2. Create tests for each of the cases described in the wiki example. This includes:
  3. Once all of the tests pass, make sure they pass using the following:
    • ruby 1.9.3
    • ruby 2.1
    • sqlite
    • mysql
    • postgresql

Let me know if you run into any trouble!

Thanks in advance 👍

@nicolas-besnard
Copy link
Contributor Author

What happened if validation is required and you sign-in with username ? Email HAS to be present right ?

@lynndylanhurley
Copy link
Owner

Just to clarify, are you asking if an email address is required to use confirmable? In that case, yes - we would require an email address to create the account. But subsequent sign-ins should be possible using the the user's username (or whatever the authentication key is called).

@nicolas-besnard
Copy link
Contributor Author

Yes, but when a new user register, he should have specified an email, right ? With or without confirmable.

In this case, the unit tests for "confirmation" and "password" are the same, aren't they ?

@lynndylanhurley
Copy link
Owner

I'm not sure about this actually. I don't see any explicit references to an email param in the devise source for registrations or sessions.

I think devise may rely on the authentication_keys array, but I'm not sure how this works for modules like confirmable and recoverable that require emails to be sent.

You may need to check the devise source code on this.

@nicolas-besnard
Copy link
Contributor Author

I will dig into it. I usually use rspec for my test. Is there any way to run a single test ?

At first look, seems like you HAVE TO specify an email on register on Devise.

I think I'll create a new project to see that in action.

@lynndylanhurley
Copy link
Owner

To run a single test, just type the word focus right above the test. I'm using this gem I think.

@nicolas-besnard
Copy link
Contributor Author

I just made a PR. The login looks good to me, I need to dig into the registration now.

@lynndylanhurley
Copy link
Owner

@nicolas-besnard - I just merged your PR - this should be fixed as of v0.1.32.beta2.

I pushed everything up to the demo site and it seems to be working there. Let me know if you see any issues.

@lynndylanhurley
Copy link
Owner

Oh and congratulations, you're now an official contributor:
https://github.com/lynndylanhurley/devise_token_auth#callouts

💥 ✨ 💐

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

No branches or pull requests

2 participants