Use redirect url to edit from initializer as well

[MaicolBen]

Resolves #1219

[MaicolBen]

can you @dks17 review this one?

[dks17]

It looks not bad.
I think you may rewrite it something like this:

    before_action :validate_redirect_url_param, only: [:create, :edit]

    private

    def validate_redirect_url_param
      # give redirect value from params priority
      @redirect_url ||= params.fetch(
        :redirect_url,
        DeviseTokenAuth.default_password_reset_url
      )

      render_create_error_missing_redirect_url unless @redirect_url
      render_create_error_not_allowed_redirect_url if blacklisted_redirect_url?
    end

    def redirect_url
      @redirect_url
    end

By this approach you can cover two actions at the same code. I suppose edit action needs the same redirect_url validation.

[dks17]

Try to add some tests for edit action. To test redirect_url validation you may use tests for create action.

      describe 'not redirect_url should return 401' do
        before do
          @auth_headers = @resource.create_new_auth_token
          @new_password = Faker::Internet.password

          put :edit,
               params: { email: 'chester@cheet.ah' }
          @data = JSON.parse(response.body)
        end

        test 'response should fail' do
          assert_equal 401, response.status
        end

        test 'error message should be returned' do
          assert @data['errors']
          assert_equal(
            @data['errors'],
            [I18n.t('devise_token_auth.passwords.missing_redirect_url')]
          )
        end
      end

[MaicolBen]

@dks17 thank you for your review! I used your code

[dks17]

You should also check error rendering if a redirect_url is blacklisted.
return render_error_not_allowed_redirect_url if blacklisted_redirect_url?

Please, add test into group is below for edit action too. Like you did it good for missing redirect_url.

devise_token_auth/test/controllers/devise_token_auth/passwords_controller_test.rb

Lines 335 to 373 in fc7bd03

	describe 'Using redirect_whitelist' do
	before do
	@resource = create(:user, :confirmed)
	@good_redirect_url = Faker::Internet.url
	@bad_redirect_url = Faker::Internet.url
	DeviseTokenAuth.redirect_whitelist = [@good_redirect_url]
	end
	teardown do
	DeviseTokenAuth.redirect_whitelist = nil
	end
	test 'request to whitelisted redirect should be successful' do
	post :create,
	params: { email: @resource.email,
	redirect_url: @good_redirect_url }
	assert_equal 200, response.status
	end
	test 'request to non-whitelisted redirect should fail' do
	post :create,
	params: { email: @resource.email,
	redirect_url: @bad_redirect_url }
	assert_equal 422, response.status
	end
	test 'request to non-whitelisted redirect should return error message' do
	post :create,
	params: { email: @resource.email,
	redirect_url: @bad_redirect_url }
	@data = JSON.parse(response.body)
	assert @data['errors']
	assert_equal @data['errors'],
	[I18n.t('devise_token_auth.passwords.not_allowed_redirect_url',
	redirect_url: @bad_redirect_url)]
	end
	end

[MaicolBen]

@dks17 ready! sorry for the delay

[dks17]

@MaicolBen Please, read I have wrote in suggested changes message. I don't insist you have to add those tests. I would like you do it.

So you have fixed #1219 @MaicolBen. Good job. Approved.

[MaicolBen]

Please, add test into group is below for edit action too

This one? I added it 20 days ago, thanks

[dks17]

You added tests in this group

devise_token_auth/test/controllers/devise_token_auth/passwords_controller_test.rb

Line 40 in 2e9f001

describe 'not redirect_url should return 401' do

to cover that line

devise_token_auth/app/controllers/devise_token_auth/passwords_controller.rb

Line 184 in 2e9f001

return render_create_error_missing_redirect_url unless @redirect_url

You did it well.

I asked you add tests in this group for edit action too

devise_token_auth/test/controllers/devise_token_auth/passwords_controller_test.rb

Line 362 in 2e9f001

describe 'Using redirect_whitelist' do

to cover that line

devise_token_auth/app/controllers/devise_token_auth/passwords_controller.rb

Line 185 in 2e9f001

return render_error_not_allowed_redirect_url if blacklisted_redirect_url?

Maybe I expressed my thought unclearly before.

[MaicolBen]

@dks17 Sorry I misunderstood you, you wrote it right, the PR is #1247