TfaLoginBlock not redirecting to the 2FA page

Thanks for the work! I needed to make a minor adjustment. The destination needed to be passed into the url parameters rather than the url "options".

Note: as-written, this will also add a destination when using the normal login form. The destination will be /user/login, and this doesn't cause any problems with logging, but I thought it was worth mentioning.

Tested against the latest branch.
This would be a nice change to have in the next release. Lets get this in!

After some discussion in slack, @therealssj and I thought it better to have this patch not change the outcome of where the user ends up after logging in using the normal login form at /user/login.

This patch ignores the destination if it is set to /user/login. Feedback welcome.

@manuel-garcia you're right, thanks! I just tested a vanilla Drupal login and landed on the user/1 page. Here is the patch re-rolled against 8.x-1.x (some line numbers changed) without the special case for /user/login.

Updated the patch to use dependency injection for the redirect.destination and request services.

@Manuel Garcia:

I requested the DI on Slack. I think that will make this login block easier to test, which is more important than how many times the objects are used.

Testing

Before applying the patch, I enabled the TFA Login block and tested with two users:

1. A user with TFA set up does not get logged in, nor redirected to the TFA form.
2. A user without TFA set up does get logged in.

In both cases, the current page reloads after submitting the form.

After applying the patch,

1. A user with TFA set up is redirected to the form for entering an application verification code. After entering the code, the user is logged in.
2. A user without TFA set up is logged in directly.

In both cases, the current page reloads after submitting the form.

Code review

1. There is a mismatch. We want a RequestStack object, not a Request:

  /**
   * Current Request object.
   *
   * @var \Symfony\Component\HttpFoundation\Request
   */
  protected $request;

Please update the description as well as the type.

2. Just a suggestion, you can leave this as is if you want.

        $form_state->setRedirect(
          'tfa.entry',
          [
            'user' => $account->id(),
            'hash' => $login_hash,
          ] + $destination
        );

I think this version will work exactly the same, and is a little easier to read:

	$destination['user'] = $account->id();
	$destination['hash'] = $login_hash;
        $form_state->setRedirect('tfa.entry', $destination);

As we discussed on Slack, my first requested change was off the mark, but it would be more consistent to call getCurrentRequest() in the create() method.

Thanks @benjifisher, I really appreciate the specific code suggestions.

1. As discussed on slack, I've modified the create() method to return the request from the request_stack.

2. I agree this is more clean. The new patch does what you suggest.

I re-tested and reviewed the changes. Looks good!

I am attaching an interdiff comparing the two patches that I reviewed.

The patch in 15 looks great. Merging!

Is this fixed currently?
I set up a GA Login plugin and still not getting redirected to the entry form, but instead to the homepage and not logged in.
Maybe I am missing some step in the setup, any hints?