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.

CommentFileSizeAuthor
#18 ur_respect_destination-489954-18.patch2.03 KBmdupont
#17 ur_respect_destination-489954-17.patch1.05 KBmdupont
#11 user_relationships_489954.patch790 bytesBilmar
user_relationships_ui.forms_.patch1 KBelliotttf

Comments

alex.k’s picture

alex.k commented

Thanks but don't you think someone could craft a URL that requests to approve a relationship and insert a destination to a phishing site?

elliotttf’s picture

elliotttf commented

If 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.

alex.k’s picture

alex.k commented

Yep 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.

elliotttf’s picture

elliotttf commented

Right, but the form API will redirect all forms to a given destination. This only becomes malicious if

  1. the site is compromised and destination queries are altered, or
  2. a malicious user creates a URL with an undesirable destination

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.

alex.k’s picture

alex.k commented
Category: bug » feature

The #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)

elliotttf’s picture

elliotttf commented

Personally 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.

merilainen’s picture

merilainen commented

subscribing

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.

YK85’s picture

YK85 commented
Priority: Minor » Normal

Hello,

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!

robby.smith’s picture

robby.smith commented

+1 subscribing

Bilmar’s picture

Bilmar commented

Hello,

I am getting the below when trying to test the patch:

can't find file to patch at input line 8
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|Index: user_relationships_ui.forms.inc
|===================================================================
|RCS file: /cvs/drupal-contrib/contributions/modules/user_relationships/user_relationships_ui/Attic/user_relationships_ui.forms.inc,v
|retrieving revision 1.1.2.12
|diff -u -p -r1.1.2.12 user_relationships_ui.forms.inc
|--- user_relationships_ui.forms.inc	14 May 2009 09:53:11 -0000	1.1.2.12
|+++ user_relationships_ui.forms.inc	12 Jun 2009 16:18:45 -0000
Bilmar’s picture

Bilmar commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new user_relationships_489954.patch790 bytes

I'm re-rolling the original patch so it applies smoothly.
Tested and it works great! Thanks for the work elliotttf

YK85’s picture

YK85 commented

tested this and it fixed the destination query issue
+1 for commit

chuckbar77’s picture

chuckbar77 commented

Alex - could you please check this out and commit? It helps greatly to fix the destination query issue for customized sites.

alex.k’s picture

alex.k commented
Status: Reviewed & tested by the community » Fixed

Committed in http://drupal.org/cvs?commit=420344, thanks all.

Bilmar’s picture

Bilmar commented

thanks Alex the wave of commits!

Status: Fixed » Closed (fixed)

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

mdupont’s picture

mdupont
Primary language French
Location Switzerland
commented
Title: Approve, Disapprove, Cancel Form Should Respect Destination Query » Relationship action forms should respect destination query
Category: feature » bug
Status: Closed (fixed) » Needs review
StatusFileSize
new ur_respect_destination-489954-17.patch1.05 KB

Sorry 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?

mdupont’s picture

mdupont
Primary language French
Location Switzerland
commented
StatusFileSize
new ur_respect_destination-489954-18.patch2.03 KB

After looking more closely at the code I think Request action should respect destination query too. Patch attached.

mrf’s picture

mrf commented
Issue tags: +6.x-1.1

Tagging to remind myself to take a look later.

mrf’s picture

mrf commented
Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Needs review » Patch (to be ported)

Committed to 6.x-1.x. Looks like this portion is missing from 7.x as well.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Patch (to be ported) » Fixed

Yep. Commited.

Status: Fixed » Closed (fixed)

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

srees’s picture

srees commented
Version: 7.x-1.x-dev » 6.x-1.3
Status: Closed (fixed) » Needs review

The 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.