Item3580: redirectto implemented redundantly in several places

AC (sorry to mention you here, but you did it in effect .... us not checking back) copy-pasted the code from Save.pm to Upload.pm in Item3313. What makes the situation even worse is that checking a redirect url is done in TWiki.pm, but in its constructor, so not in a reusable way.

Next issue, when I want to redirect to an url pointing to the configured host itself, it does not allow to do so until I enable AllowUrlRedirect. But this opens up the site for phishing again. There exists an according patch in the debian package, done by SD, but not provided upstream.

Yea.

-- TWiki:Main/MichaelDaum - 08 Feb 2007

Ehm yes, could you summarize the steps to be taken?

-- TWiki:Main.ArthurClemens - 08 Feb 2007

1. move _getRedirectUrl() from Save.pm to TWiki.pm (rename it getRedirectUrl)
2. delete it from Save.pm and Upload.pm
3. fix the calls to getRedirectUrl()
4. fix getRedirectUrl() once and forever to compare the wanted url with the sites url and allow it to redirect to itself even if AllowRedirectUrl is turned off.
5. make tools/debian/patches/01...redirect...patch superfluous by fixing TWiki::new() to use the new getRedirectUrl()

-- TWiki:Main.MichaelDaum - 08 Feb 2007

OK, this is clear. I just wonder how the comparison between the url and the site url will look like (item 4). Everything up to bin?

-- TWiki:Main.ArthurClemens - 08 Feb 2007

You really need to use the URI library from CPAN, which allows you to compare canonical URLs. You will probably have to compare thus:

if( $targetUrl =~ m#^$TWiki::cfg{DefaultUrlHost}$TWiki::cfg{ScriptUrlPath}#o ) {

CC

There exists an according patch in the debian package, done by SD, but not provided upstream. ?

I thought http://svn.twiki.org/svn/twiki/branches/MAIN/tools/pkg/debian/patches/01_redirect_fix.dpatch was pretty much upstream.

-- TWiki:Main.SvenDowideit - 10 Feb 2007

What is the status of this patch?

-- TWiki:Main.ArthurClemens - 25 Feb 2007

OK, it seems ready for use, but some small modifications may be needed.

CPAN:URI is already prerequisite for configure (TWikiReleaseNotes04x00), so could be used.

-- TWiki:Main.ArthurClemens - 07 Mar 2007

its not using URI yet, but you can now call $twiki->redirect($url, undef, 1) , where the third param tells redirect to use the ?redirectto urlparam or POST param if its defined

-- SvenDowideit - 23 Mar 2007

sub isRedirectSafe {
    my $redirect = shift;
    
    #TODO: this should also grok aliases for the current host. (127.0.0.1, ip, multi-homed, localhost etc) though this raises the danger level somewhat.
   use URI;
   my $redirectURI = URI->new($redirect);
   if ((!$TWiki::cfg{AllowRedirectUrl}) && ( $redirectURI->scheme )) {
       my $defaultURI = URI->new($TWiki::cfg{DefaultUrlHost});
        return ($redirectURI->host eq $defaultURI->host);
    }
    return 1;
}

however, it looks to me like configure is no longer using URI? so I won't commit it atm (when I do I'll refactor alot more url parsing code..)

-- SvenDowideit - 23 Mar 2007

Can you confirm the status 'Waiting for Release' is correct?

-- TWiki:Main.ArthurClemens - 27 Mar 2007

The changes have been commited to MAIN. I have not heard from anyone that they cause issues.

-- TWiki:Main.SvenDowideit - 15 Apr 2007

Cleaned "WaitingFor" field.

-- TWiki:Main.GilmarSantosJr - 10 Aug 2008