Patch Proposal: Fix some Auth Bugs
Description
readTopicPermissionFailed is a string, set to "" initially, and growing to include a list of topics where read access failed. therefore, it must be compared against "" (since all strings compare as numbers in Perl). The methods used to read topics for checking auth. don't actually set said string; so I'm not sure why it is tested at all.
If a web is view-restricted, and a user types a new topic name in the 'Go' box, then redirection to viewauth is never attempted, and the "create new topic/search/etc" options are never presented. So, I add to the readTopicPermissionFailed condition a provision for a nonexistant file (and then reauthenticate through viewauth).
In general, it seems that if a whole web is blanket view-restricted, then it's an unnecessary (around 2x?) drag on performance to attempt an unauthed view in that web (I know that topic prefs can override web prefs, but let's say there is some way of indicating when the restriction is truly global).
addUserToTWikiUsersTopic needs to set the $internal flag in case
TWikiUsers is read-restricted, otherwise it clobbers the topic
Implementation
--- /home/graehl/isd/apache/orig_new_twiki/lib/TWiki/UI/RDiff.pm Sun Apr 25 18:26:22 2004
+++ lib/TWiki/UI/RDiff.pm Mon Jun 21 16:49:51 2004
@@ -427,7 +427,7 @@
# check access permission
my $wikiUserName = &TWiki::userToWikiName( $userName );
my $viewAccessOK = &TWiki::Access::checkAccessPermission( "view", $wikiUserName, "", $topic, $webName );
- if( $TWiki::readTopicPermissionFailed ) {
+ if( $TWiki::readTopicPermissionFailed ne "") {
# Can't read requested topic and/or included (or other accessed topics)
# user could not be authenticated, may be not logged in yet?
my $rdiffauthFile = $ENV{'SCRIPT_FILENAME'};
--- /home/graehl/isd/apache/orig_new_twiki/lib/TWiki/UI/View.pm Sun Apr 25 18:26:22 2004
+++ lib/TWiki/UI/View.pm Mon Jun 21 17:29:28 2004
@@ -251,7 +251,7 @@
# check access permission
my $viewAccessOK = &TWiki::Access::checkAccessPermission( "view", $wikiUserName, $text, $topic, $webName );
- if( $TWiki::readTopicPermissionFailed ) {
+ if( (!$topicExists) || $TWiki::readTopicPermissionFailed ne "") {
# Can't read requested topic and/or included (or other accessed topics
# user could not be authenticated, may be not logged in yet?
my $viewauthFile = $ENV{'SCRIPT_FILENAME'};
--- /home/graehl/isd/apache/orig_new_twiki/lib/TWiki/User.pm Thu Feb 26 17:52:37 2004
+++ lib/TWiki/User.pm Mon Jun 21 14:18:56 2004
@@ -233,7 +233,7 @@
my ( $wikiName, $remoteUser ) = @_;
my $today = &TWiki::formatTime(time(), "\$day \$mon \$year", "gmtime");
my $topicName = $TWiki::wikiUsersTopicname;
- my( $meta, $text ) = &TWiki::Store::readTopic( $TWiki::mainWebname, $topicName );
+ my( $meta, $text ) = &TWiki::Store::readTopic( $TWiki::mainWebname, $topicName, 1 );
my $result = "";
my $status = "0";
my $line = "";
Note: Patch is attached as
https://twiki.org/p/pub/Codev/VariousAuthBugsFixed/twiki_graehl_patches.diff . The patch is against the
TWikiBetaRelease of
07 May 2004.
-
JonathanGraehl - 22 Jun 2004
This is now commited into
SVN. thankyou for the fix!
--
SvenDowideit - 27 Jun 2004
Discussion:
$TWiki::readTopicPermissionFailed
get set in
twiki/lib/TWiki/Store.pm
Perl trivia:
- if( $TWiki::readTopicPermissionFailed ) {
+ if( $TWiki::readTopicPermissionFailed ne "") {
This fix was not necessary. A boolean evaluation of a variable returns 0 if the variable is undefined,
"0"
or empty; anything else returns 1.
$TWiki::readTopicPermissionFailed
is either empty or contains a space delimited list of topics. So, both lines do the same thing in this case.
There would be a warning if the variable would be initialized as undefined at a later point. Therefore it is safer to revert the boolean eval to the previous code. In
SVN.
--
PeterThoeny - 10 Jul 2004
That's true. I'd never really memorized the exact rules and had always been explicit about testing for empty-string because of the possible confusion of a nonnumeric string with "0". I later realized this but didn't think to correct myself here
(well, I guess I did in
DiffsFunctionDoesNotAuthenticateProperly)
--
JonathanGraehl - 08 Dec 2004
The current implementation (20041030beta) uses this
if( (!$topicExists) || $TWiki::readTopicPermissionFailed )
which causes a user-friendliness problem : when one asks for a non-existant topic, he/she is redirected to the viewauth script, and thus is asked for authentification ; on a world-readable web, the users are not used to be asked for this... and don't understand anything. We've reversed this to
if( $TWiki::readTopicPermissionFailed )
on
http://docs.indymedia.org.
--
BenVoui - 12 Dec 2004