Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The cancel link on the approve/disapprove/cancel form does not use the destination query if it is set. Since the form is normally loaded via AJAX it's not a major defect but if the form is called from a different location and AJAX is not used the cancel link needs to redirect correctly.
See attached patch for proposed fix.
Comment | File | Size | Author |
---|---|---|---|
#18 | ur_respect_destination-489954-18.patch | 2.03 KB | mdupont |
#17 | ur_respect_destination-489954-17.patch | 1.05 KB | mdupont |
#11 | user_relationships_489954.patch | 790 bytes | Bilmar |
user_relationships_ui.forms_.patch | 1 KB | elliotttf |
Comments
Comment #1
alex.k CreditAttribution: alex.k commentedThanks but don't you think someone could craft a URL that requests to approve a relationship and insert a destination to a phishing site?
Comment #2
elliotttf CreditAttribution: elliotttf commentedIf that were true wouldn't the form redirect to the phishing site upon submission? I don't know if the Forms API sanitizes destinations in any way, but I thought it redirected to whatever the destination query is set to.
Comment #3
alex.k CreditAttribution: alex.k commentedYep it will redirect to whatever is given, hence my concern. So it seems risky to include it in UR due to the potential of misuse.
Comment #4
elliotttf CreditAttribution: elliotttf commentedRight, but the form API will redirect all forms to a given destination. This only becomes malicious if
In the first case you've got bigger problems to worry about than form redirection and in the second case site admins can remove the links/posts/users that introduce the vulnerability. I guess I'm just not clear why it's ok to redirect a form submission to destination but not the form cancel link.
Comment #5
alex.k CreditAttribution: alex.k commentedThe #2 is what I was thinking about... but you're right, submit and cancel should both respect destination if one does so. (for that matter, it would have been better to use a submit for cancel, not a link, perhaps I should modify it this way)
Comment #6
elliotttf CreditAttribution: elliotttf commentedPersonally I like the link because you don't have to write any submit handler code for it, but as long as there's an option to abort the submission that returns to the same place that submitting does I'm happy.
Comment #7
mErilainen CreditAttribution: mErilainen commentedsubscribing
I'm not sure what this "destination query" is, but currently users are always redirected to relationships/requests whether they approve, deny, cancel or say no in the AJAX form. Only when I add a relationship from user's profile, then it stays on the same page and prints a message that my request has been sent.
Comment #8
YK85 CreditAttribution: YK85 commentedHello,
Is there a chance of this patch being committed? or will there be further development?
I would like to set the URL to a specific page when users click cancel from the approve/remove page, but it currently leads to relationships/requests
Thank you!
Comment #9
robby.smith CreditAttribution: robby.smith commented+1 subscribing
Comment #10
Bilmar CreditAttribution: Bilmar commentedHello,
I am getting the below when trying to test the patch:
Comment #11
Bilmar CreditAttribution: Bilmar commentedI'm re-rolling the original patch so it applies smoothly.
Tested and it works great! Thanks for the work elliotttf
Comment #12
YK85 CreditAttribution: YK85 commentedtested this and it fixed the destination query issue
+1 for commit
Comment #13
chuckbar77 CreditAttribution: chuckbar77 commentedAlex - could you please check this out and commit? It helps greatly to fix the destination query issue for customized sites.
Comment #14
alex.k CreditAttribution: alex.k commentedCommitted in http://drupal.org/cvs?commit=420344, thanks all.
Comment #15
Bilmar CreditAttribution: Bilmar commentedthanks Alex the wave of commits!
Comment #17
mdupontSorry to reopen but the previous fix only covers Approve, Disapprove and Cancel actions. Destination query should also be respected in Remove action, and (but I'm less sure of that) Request action, thus all actions will act in a consistent way.
It proves useful when using UR actions from somewhere else than the standard pages (in views for example) and no ajax is used.
Attached a patch that make the Remove action respect destination query. Should it be the same for Request action? What do you think?
Comment #18
mdupontAfter looking more closely at the code I think Request action should respect destination query too. Patch attached.
Comment #19
mrf CreditAttribution: mrf commentedTagging to remind myself to take a look later.
Comment #20
mrf CreditAttribution: mrf commentedCommitted to 6.x-1.x. Looks like this portion is missing from 7.x as well.
Comment #21
BerdirYep. Commited.
Comment #23
srees CreditAttribution: srees commentedThe destination setting does not work for the cancel button with URL's that contain encodable characters. For example, if you set destination=user/somepage?var=1, the submit works correctly, but the cancel link takes you to user/somepage%3Fvar%3D1 which of course doesn't work.