Destination URL is lost when using Drupals private files

I was not able to replicate with the latest 8.x-1.11 release. May have been a duplicate of #2886119: Keep destination parameter when auth_path is overridden which has been fixed. Please try with the latest module and re-open if the issue still occurs. Thanks.

Steps to reproduce

1. Install a clean instance of Drupal (I used 8.5.3, minimal install profile)
2. Configure private files: add $settings['file_private_path'] = '../private'; to settings.php
3. Log in as uid 1
4. Add a content type
5. Add a File field to the content type: enter "File" for the required label and leave all settings as default, except choose "Private files" as the Upload destination
6. Create and save a node using this content type and upload a .txt file to the File field
7. View the node and ensure the File link shows - the URL will be something like http://d8.vm7/system/files/2018-05/test_0.txt
8. Click on the link and ensure the file is rendered or downloaded by the browser (i.e. returns a 200 response)
9. Enable Require Login 1.11
10. Visit the private file URL again (e.g. http://d8.vm7/sites/default/files/2018-05/test.txt)
11. Note user doesn't have to logout here, issue persists for both anon + authenticated users. If logged in: we are redirected to http://d8.vm7/system/files and if anon: we are redirected to http://d8.vm7/user/login?destination=/system/files

In both cases, the "true" destination is lost. So when the user does log in, they end up on http://d8.vm7/system/files (which is a 403).

We would expect to be redirected to either http://d8.vm7/system/files/2018-05/test.txt (logged in) or http://d8.vm7/user/login?destination=/system/files/2018-05/test.txt (anonymously).

Please re-try testing with the latest version (Require Login 8.x-1.11). There were some fixes around the redirect system. Re-open if the issue is still happening with the latest version. Thanks.

Apologies, a mistake in my notes. This was with 1.11.

Had a quick look at this today - just to note that the issue seems related to the call to getPath() in RequireLoginSubscriber:

In \Drupal\Core\Path\CurrentPathStack:

public function getPath(Request $request = NULL) {
    if (!isset($request)) {
      $request = $this->requestStack->getCurrentRequest();
    }
    if (!isset($this->paths[$request])) {
      $this->paths[$request] = $request->getPathInfo();
    }

    return $this->paths[$request];
  }

!isset($this->paths[$request] returns FALSE, and at this point $this->paths[$request] === '/system/files', and so that's what gets returned.

If !isset($this->paths[$request] was TRUE here, we'd get the correct path as $request->getPathInfo() returns the full string we want, i.e. /system/files/2018-05/test.txt

HTH.

And as a quick fix/kick-off for discussion, this patch fixes the issue for me.

It seems $current_route is actually the current request, so I think we can call $current_route->getPathInfo() instead of $this->service->get('path.current')->getPath().

(On a side-note, $current_route and $request are identical objects, as far as I can tell, with $current_route being a bit of a misnomer - it's actually a Request object.)

Issue has been fixed in latest code refactor and has been committed to the 8.x-1.x branch. New releases will be created soon.