Problem/Motivation

When an unauthorized user (e.g. anonymous user) requests a page they don't have access to (e.g. /admin/content), Drupal redirects them to the login screen. When they successfully log in, the originally requested page is loaded.

Moderation Dashboard overrides this, and loads the dashboard on every login.

Proposed resolution

Only load the dashboard if the user logged in without requesting another page.

CommentFileSizeAuthor
#10 3005859-10.patch3.08 KBbendeguz.csirmaz
#8 interdiff-3005859-5-8.txt915 bytesbendeguz.csirmaz
#8 3005859-8.patch2.94 KBbendeguz.csirmaz
#5 interdiff-3005859-2-5.txt1.47 KBbendeguz.csirmaz
#5 3005859-5.patch2.55 KBbendeguz.csirmaz
#2 3005859-2.patch2.22 KBbendeguz.csirmaz

Comments

bendeguz.csirmaz created an issue. See original summary.

bendeguz.csirmaz’s picture

bendeguz.csirmaz commented
Assigned: bendeguz.csirmaz » Unassigned
Status: Active » Needs review
StatusFileSize
new 3005859-2.patch2.22 KB

Status: Needs review » Needs work

The last submitted patch, 2: 3005859-2.patch, failed testing. View results

bendeguz.csirmaz’s picture

bendeguz.csirmaz commented
Assigned: Unassigned » bendeguz.csirmaz

$redirect_url = $request->server->get('REDIRECT_URL');

It failed of course, the 'REDIRECT_URL' can't determine whether the user is currently logging in to the dashboard or to another page.
I'll try solving this by parsing the destination parameter from the response object.

bendeguz.csirmaz’s picture

bendeguz.csirmaz commented
Assigned: bendeguz.csirmaz » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3005859-5.patch2.55 KB
new interdiff-3005859-2-5.txt1.47 KB
samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented 
  1. +++ b/src/Routing/ResponseSubscriber.php
@@ -53,11 +54,14 @@ class ResponseSubscriber implements EventSubscriberInterface {
+      $response_url_components = UrlHelper::parse($response->getTargetUrl());
+      $has_destination = isset($response_url_components['query']['destination']);

    RedirectResponse extends Response so I think you should be able to get to the query with $response->query, mind looking to see if these lines can be simplified? 

  2. +++ b/tests/src/Functional/ModerationDashboardRedirectTest.php
@@ -0,0 +1,36 @@
+  /**
+   * Tests if login is not redirected when the user requested another page.
+   */
+  public function testNoRedirect() {

    I'd like a positive case test as well, if you don't mind.

samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented
Status: Needs review » Needs work
bendeguz.csirmaz’s picture

bendeguz.csirmaz commented
Status: Needs work » Needs review
StatusFileSize
new 3005859-8.patch2.94 KB
new interdiff-3005859-5-8.txt915 bytes

Response doesn't have a query property, only Request does, I think this is the simplest I can do.

Status: Needs review » Needs work

The last submitted patch, 8: 3005859-8.patch, failed testing. View results

bendeguz.csirmaz’s picture

bendeguz.csirmaz commented
Status: Needs work » Needs review
StatusFileSize
new 3005859-10.patch3.08 KB

Rerolled.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

This looks great.

  • bendeguz.csirmaz authored 7e69bf7 on 8.x-1.x 
    Issue #3005859 by bendeguz.csirmaz: Redirect to dashboard only if no...
samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Thanks all!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.