Tags:
create new tag
, view all tags

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 http://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 smile (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

Edit | Attach | Watch | Print version | History: r7 < r6 < r5 < r4 < r3 | Backlinks | Raw View | Raw edit | More topic actions
Topic revision: r7 - 2004-12-12 - BenVoui
 
  • Learn about TWiki  
  • Download TWiki
This site is powered by the TWiki collaboration platform Powered by Perl Hosted by OICcam.com Ideas, requests, problems regarding TWiki? Send feedback. Ask community in the support forum.
Copyright © 1999-2015 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.