Bug: Incorrect redirection from view to viewauth

Redirection from view to viewauth doesn't terminate the script after printing the redirection.

Test case

Set ALLOWWEBVIEW for a web, e.g., to your user id.

Simulate CGI environment with environment variables and call view for that Web's WebHome by hand. Two redirections are output, first the one to viewauth, then the one to oops. It will depend on the Web server and/or browser which redirection is actually taken.

With Apache 2 and mod_perl 1.999, the second redirection (to oops :-() was taken.

I used a web named Admin as an example, and called view in the bin directory as

SCRIPT_FILENAME=~+/view REQUEST_URI=/twiki/bin/view/Admin SCRIPT_NAME=/twiki/bin/view PATH_INFO=/Admin ~+/view

• view_redirect.txt: Erroneous output of unpatched view script

Environment

-- JoachimSchrod - 25 May 2005

Impact and Available Solutions

Follow up

could you provide a cvs diff or diff -u patch please?

-- WillNorris - 25 May 2005

Fix record

• View.pm.patch: Just output redirection to viewauth or to oops, and terminate

The patch is against View.pm from the 02Sep2004 release. In TWiki::UI::View.pm, a return is inserted after the call to TWiki::UI::redirect(). Without that return, the code continues to the ! $viewAccessOK clause and runs into the TWiki::UI::oops(). Incidentially, it might be that a return after that oops() should be added as well.

Discussion

Patch added, as per request of Will.

I'm very sure that the first added return is necessary, as the redirect of TWiki::UI::oops() must not be printed after TWiki::UI::redirect() to viewauth. I think that view() should not continue after the oops() either-- that's the second inserted return. The rest of the output in that function does not look sensible, as the oops-Text must follow.

I also added a trace of the unpatched script's output, with the errors. It's linked in the test case.

-- JoachimSchrod - 25 May 2005

thanks, joachim. CrawfordCurrie has reviewed the code in DevelopBranch, and because access checking happens at a lower level, this is no longer an issue. however, this patch can be valuable to currently-deployed CairoRelease installations.

-- WillNorris - 26 May 2005

OK. Thanks for the info that DevelopBranch has a different authentication concept. I'll check it out and see if my other irritations with authentication have been gone there...

-- JoachimSchrod - 27 May 2005

Joachim, if you want to give me a summary of what your irritations are, I can probably give you a fast answer.

-- CrawfordCurrie - 27 May 2005

I investigated a bit and think it's a documentation issue: When one sets ALLOWWEBVIEW for a Web and has SITEMAPLIST = on, all topics that include TWiki.SiteMap suddenly need read authentication as well. It's because the search in SiteMap accesses WebPreferences of the respective web to get SITEMAPWHAT and SITEMAPUSETO, and this taints read topic permissions.

Since the site map is built with %SEARCH% and is not a build-in tag, I don't think one can alter that behaviour. But I (and my users) were quite astonished that my main pages needed suddenly read authentication, and a warning about that consequence might be nice to have both in WebPreferences comments, and in the authentication docs. I could prepare a patch proposal against the DEVELOP SVN branch if that would be of interest.

-- JoachimSchrod - 28 May 2005