For a few sites where we're using a custom module that we've integrated into masquerade we've often ran into an issue with when you switch back from a user you end up on the page you clicked switch back on, which you may or may not have access to. For instance you switched to a user with permission for changing the theme from one without that permission, and on the theme settings page you click switch back and get greeted by a nice access denied message.

I'd propose we should be able to pass a 'return' url to the switch page, which stores the url and when you switch back it calls drupal_goto on that url instead of the referrer if its set.

CommentFileSizeAuthor
#6 1538428-masq-url-6.patch3.19 KBandypost
#1 masquerade-1538428-switch-back-return-url.patch1.21 KBjrreid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jrreid’s picture

jrreid CreditAttribution: jrreid commented
FileSize
masquerade-1538428-switch-back-return-url.patch1.21 KB

Here is my proposed patch, rolled against head

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs review » Needs work

Suppose this way wrong, simply you just need to hijack global referer in you custom module, I see no actual reason to store another variable in session

Also your code does not follow http://drupal.org/coding-standards

deekayen’s picture

deekayen CreditAttribution: deekayen commented

destination is the traditional drupal get variable for setting redirects. there's already some built-in core support for that var.

jrreid’s picture

jrreid CreditAttribution: jrreid commented

?destination=blah however influences where you go when you click on a masquerade link, in this case sending the masqueraded user to blah after the switch and already works as expected. This return url setting is to control where you go back to after you've clicked switch back.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

Suppose storing of return url in session makes no sense. But allow to handle common 'destination' a thing we should done.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
FileSize
1538428-masq-url-6.patch3.19 KB

Here's a patch:
- adds destination to all links
- form submission does not need got at all
- switch callbacks analyzes 'destination' parameter first

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs work » Needs review

@deekayen are you agree to commit this?

<code>
+++ b/masquerade.module
@@ -651,9 +657,6 @@ function masquerade_block_1_submit($form, &$form_state) {
-  else {
-    drupal_goto($_SERVER['HTTP_REFERER']);
-  }

@@ -736,6 +739,9 @@ function masquerade_autocomplete_multiple($string, $add_anonymous = TRUE) {
+    if (isset($_GET['destination'])) {
+      drupal_goto($_GET['destination']);
+    }

@@ -821,6 +827,9 @@ function masquerade_switch_back_page() {
+    if (isset($_GET['destination'])) {
+      drupal_goto($_GET['destination']);
+    }

This hunk are required to fix

obleser’s picture

obleser CreditAttribution: obleser commented

Thought it would be a better approach to create 2 "Triggers" which are triggered on switch and unswitch?
Then it would be possible to do a hole bunch if things, not only a redirect ;)

Just a thought...

Status: Needs review » Needs work

The last submitted patch, 1538428-masq-url-6.patch, failed testing.

jenlampton’s picture

jenlampton
she / her
CreditAttribution: jenlampton commented

I second the approach in #8. I just recommended the same thing over in https://drupal.org/node/763972#comment-8266267