408984 - Redirect not working if logged in to editors

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Admin/Editor Tools, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Log out of AMO and go to https://addons.mozilla.org/de/firefox/editors , the redirect doesn't take you to the editor panel (like it did before the last push).

Comment 1

[Fred Wenzel [:wenzel]]

Confirming; Wil, the login code does not seem to honor the ?to= parameter anymore. :(

Could be because you seem to have added locale and app to the ?to parameter, which was not part of it before.

Comment 2

[Wil Clouser [:clouserw]]

This is by design.  Any URL that matches:

/(:\/\/|developers|editors|localizers|admin|users|\r|\n)/ 

is ignored and the user is sent to the home page.  This is to prevent CSRF attacks (http://en.wikipedia.org/wiki/Csrf).

I'm WONTFIXing this, but I'm happy to talk about it if you'd like.

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Could we use a whitelist for allowed urls for these usergroups. At the moment, an editor has to log in, click 'editors', 'updates' and at least the link to the last page (or '50' if there are less than 51 updates pending review) if he wants to see new submissions. In the old system, I had a bookmark going directly to update queue. I can live without it, but it was a nice-to-have.

Comment 4

[Wil Clouser [:clouserw]]

Sure, I'll reopen it.  I'm going to assign it to nobody in the hopes a patch comes in though. :)

To anyone looking to write a patch, this happens on line 294 of users_controller.php.  The easiest way to do this would probably be just have an if (in_array($url,array('safeurl1','safeurl2'))) right before it gets reset to '/'.

Comment 5

[Ingo Müller]

Where can one get insight into the sources? Here? http://svn.mozilla.org/addons/trunk/site/app/controllers/users_controller.php

Comment 6

[Wil Clouser [:clouserw]]

(In reply to comment #5)
> Where can one get insight into the sources? Here?
> http://svn.mozilla.org/addons/trunk/site/app/controllers/users_controller.php
> 

Yep, that's the file.  Comment #4 narrows it down a bit.

Comment 7

[Wil Clouser [:clouserw]]

Attachment: Remove CSRF failsafe

The only other modify-data-without-POST is recalculating a file's hash.  Is that a blocker for this?  Does anyone else know of a place where we modify data with just GET?

Comment 8

[Fred Wenzel [:wenzel]]

Comment on attachment 322702 [details] [diff] [review]
Remove CSRF failsafe

works for me. Sad that our code is tomfoolery-free now, though.

Comment 9

[Wil Clouser [:clouserw]]

r14031

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using preview.* URLS with the paths mentioned in comment 2's regexp.