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

Hardcoded resource.provider in RegistrationsController? #997

Closed
m4-miranda opened this issue Oct 27, 2017 · 10 comments
Closed

Hardcoded resource.provider in RegistrationsController? #997

m4-miranda opened this issue Oct 27, 2017 · 10 comments

Comments

@m4-miranda
Copy link
Contributor

m4-miranda commented Oct 27, 2017

The resource.provider field in Registrations#create is hardcoded to 'email'. Shouldn't this be obtained from Devise?

I'm trying to get rid of email dependency altogether

@zachfeldman
Copy link
Contributor

Hey @m4-miranda sure, I agree with your logic here. Mind opening a pull request to illustrate your exact changes in mind?

@MaicolBen
Copy link
Collaborator

If you make a PR, you can reuse the provider method defined in #975

@m4-miranda
Copy link
Contributor Author

@MaicolBen It isn't possible to override the concern, is it? I could modify the get_resource method to accept a provider parameter, but the method would have to be declared in each controller

@MaicolBen
Copy link
Collaborator

You can override it in your code, and yes, you have to do it in every controller (you can make a concern in your code to avoid repeating)

@MaicolBen
Copy link
Collaborator

Closed via #998

@KelseyDH
Copy link

KelseyDH commented Jan 16, 2018

This gotcha really screwed me up. It would be really nice for provider not to be hardcoded in these controllers as a default e.g. perhaps by checking first if its defined from params before hardcoding it to email?

This way I could add 'provider' as a permitted param to configure and use it as I wish with expected behaviour.

I lost many hours trying to figure out why my user model kept hardcoding provider to 'email'. Similar to email_changed? we should try to find ways of implementing devise without creating these crazy overrides, as they make for very hard to debug and confusing behaviour when custom behaviour to our User model is added.

@MaicolBen
Copy link
Collaborator

Sorry to hear that, as we don't have a lot of code documentation, you need to read the source code, in case of the provider, you can override with what was defined in #975

@KelseyDH
Copy link

KelseyDH commented Jan 16, 2018

@MaicolBen Thanks for getting back. My intention is to have provider switch between username and email based on the params received within the same controller. Any idea on how I could go about implementing that kind of dynamic override for the registrations controller -- and not have this hardcoded provider:

def provider
  'email'
end

unexpectedly modify and break my User model elsewhere on subsequent updates to a user (like password resets or logins)?

(Overall I'm skeptical why this code was moved into a concern at all, as it seems to make extensibility and customization of each controller more unclear and difficult.)

@KelseyDH
Copy link

KelseyDH commented Jan 16, 2018

I tried modifying the gem to override the registrations controller code from something like this:

module DeviseTokenAuth
  class RegistrationsController < DeviseTokenAuth::ApplicationController

    def create
      @resource            = resource_class.new(sign_up_params.except(:confirm_success_url))
      @resource.provider   = provider

over to this:

module DeviseTokenAuth
  class RegistrationsController < DeviseTokenAuth::ApplicationController
 
    def create
    @resource            = resource_class.new(sign_up_params.except(:confirm_success_url))
    if sign_up_params[:provider].present?
      @resource.provider = sign_up_params[:provider]
    else
      @resource.provider = provider
    end

but it didn't work and I still get back an email provider in my model despite explicitly sending in params for "provider" => "username" and explicitly whitelisting the param provider in my devise_sanitizer for sign_up. It is my hope that something like this could fix the gem to provide more expected behaviour when provider is customized.

@MaicolBen
Copy link
Collaborator

That's happening because you aren't saving the provider in the resource, which it isn't a good solution. I believe this has been worked on #990, but we need it finished in order to use it. In fact, we put a bounty on it in order to get it ASAP

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

4 participants