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

Can't unlock account through email link in lockable #927

Closed
dhurba87 opened this issue Jul 18, 2017 · 8 comments
Closed

Can't unlock account through email link in lockable #927

dhurba87 opened this issue Jul 18, 2017 · 8 comments

Comments

@dhurba87
Copy link

I was implementing lockable but it threw 500 error if I click on the email link.
I rake route and I found that it goes on Devise::UnlocksController and I checked for UnlocksController in devise_token_auth and I didn't find one.
Do I have to override unlocks controller or am I missing something else?

@nicholasshirley
Copy link

This is referencing the UnlocksController from Devise which devise_token_auth depends. Can you paste the error output?

@dhurba87
Copy link
Author

dhurba87 commented Aug 2, 2017

I don't remember exactly but the error was related to flash messages and i believe api-only application doesn't have session and I also think that the unlockable link should have redirect_url and should redirect to the client URL after successfully unlocked. I already fixed this in my application but it was not in the good design pattern so couldn't contribute.

@brycesenz
Copy link
Contributor

brycesenz commented Oct 1, 2017

I just ran into this issue myself. As near as I can tell, this is what's happening:

  1. The link that is generated in an unlock email posts to the show action of the unlock controller. There isn't explicitly a controller in this gem for unlocks, so it just uses the UnlocksController of the standard Devise gem.

  2. The default redirect URL for that controller is the new_session_path. Which, in standard Devise, makes a ton of sense, since GET /user/sign_in path makes sense - it renders the HTML sign in page.

  3. Unfortunately, in the SessionsController in this gem, that path doesn't exist. It actually gives an error (see code below)

  class SessionsController < DeviseTokenAuth::ApplicationController
    before_action :set_user_by_token, :only => [:destroy]
    after_action :reset_session, :only => [:destroy]

    def new
      render_new_error
    end

It's that render_new_error which is causing the issue.

MY SOLUTION:

I created my own Unlocks controller, which stricly inherits from the Devise controller. However, I specify a new redirect URL (for me, it was just the home path, but it could be whatever you prefer).

class Api::V1::Auth::UnlocksController < Devise::UnlocksController
  def new
    super
  end

  def create
    super
  end

  def show
    super
  end

  private
  def after_unlock_path_for(resource)
    # Commenting out the default behavior and specifying my own path
    # super(resource)
    home_path
  end
end

Ideally, I feel that this should be a controller in this project. You can build one like this (as an alternative to the above). The benefit is that, much like the redirect in the password controller, this one will encode the header info in the redirect URL.

class Api::V1::Auth::UnlocksController < ::DeviseTokenAuth::ApplicationController
  skip_after_action :update_auth_header, :only => [:show]

  # Not implemented
  # def new
  #   super
  # end

  # Not implemented
  # def create
  #   super
  # end

  def show
    @resource = resource_class.unlock_access_by_token(params[:unlock_token])

    if @resource && @resource.id
      client_id  = SecureRandom.urlsafe_base64(nil, false)
      token      = SecureRandom.urlsafe_base64(nil, false)
      token_hash = BCrypt::Password.create(token)
      expiry     = (Time.now + DeviseTokenAuth.token_lifespan).to_i

      @resource.tokens[client_id] = {
        token:  token_hash,
        expiry: expiry
      }

      @resource.save!
      yield @resource if block_given?

      redirect_to(@resource.build_auth_url(after_unlock_path_for(@resource), {
        token:          token,
        client_id:      client_id,
        unlock:         true,
        config:         params[:config]
      }))
    else
      render_show_error
    end
  end

  private
  def after_unlock_path_for(resource)
    #TODO: Whatever you want it to be
  end

  def render_show_error
    raise ActionController::RoutingError.new('Not Found')
  end
end

@zachfeldman
Copy link
Contributor

Solution posted! Will close this in 7 days if no activity.

@brycesenz
Copy link
Contributor

@zachfeldman - truthfully, I think that the right way to close this isn't just to have a hack in this thread, but to have an actual UnlocksController in this project. Just my two cents.

@zachfeldman
Copy link
Contributor

@brycesenz I definitely agree! It seems like you've done a lot of the base work above...how would you feel about taking it to the next level and opening a pull request ? :)

@brycesenz
Copy link
Contributor

@zachfeldman - sorry to leave you hanging. I'm working on a PR. The big delay is that I'm more of an RSpec than a Minitest person, so I have to get all of my tests over to that new syntax :/

@zachfeldman
Copy link
Contributor

That's awesome! Take your time @brycesenz . I will unmark this issue for closing and hope you get back sometime soon.

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