Item11458: TopicUserMapping blocks registration if passwords are not writable - FAIL!!

The TopicUserMapping switches off new user registration if the password manager reports that passwords are not writable. This is wrong - I have an install where passwords are not writable, but new user registration most certainly is possible. The test in question is:

# if password manager says sorry, we're read only today
# 'none' is a special case, as it means we're not actually using the password manager for
# registration.
    if (   $this->{passwords}->readOnly()
        && ( $Foswiki::cfg{PasswordManager} ne 'none' )
        && $Foswiki::cfg{Register}{EnableNewUserRegistration} )
    {
        $session->logger->log( 'warning',
'TopicUserMapping has TURNED OFF EnableNewUserRegistration, because the password file is read only.'
        );
        $Foswiki::cfg{Register}{EnableNewUserRegistration} = 0;
    }

In my case the password manager is Foswiki::Users::PasswdUser (i.e. it uses /etc/passwd) so that any Linus-registered user can register on the wiki using their linux login. The registration is used to create the list of users and maintain the meta-data associated with the user - such as mug shots, waist measurements and glasgow coma score.

-- CrawfordCurrie - 19 Jan 2012

Hm. We need another easy way to disable registration. I use that feature/technique all the time to briefly block registration on foswiki.org while I edit the password file to disable a spam registration. chmod .htpasswd -w / vi .htpasswd / chmod .htpasswd +w Otherwise editing the file without a lock can corrupt the file.

Maybe some sort of flag that says whether or not write is required? $this->{passwords}->readOnly() && $this->{passwords}->writeRequired() If the password manager requires write to be able to successfully register a user, then certainly registration should be blocked when it can't write, or we get in to the quagmire of partially-registered users.

-- GeorgeClark - 19 Jan 2012

There's a context identifier passwords_modifyable already used by the templates and the login managers. Looking into hos this can be used.

-- CrawfordCurrie - 20 Jan 2012

I decided the cleanest way to handle this is to push the responsibility for creating the pw file onto configure. That way the password managers can simply check for existence and writability, and not have to worry about creating it on the fly. It puts the filename more "in-your-face" for the admin.

-- CrawfordCurrie - 23 Jan 2012

Just out of curiostiy, I benchmarked the cascade of if statements, and the results were worse than I thought:

#!/usr/bin/perl

use strict;
use Time::HiRes;
use Benchmark qw(:all);

my $count       = 1_000_000;
my $nonexistent = "nonexistent_tmp.$$";
my $readf       = "read_tmp.$$";
my $writef      = "write_tmp.$$";
my $dir         = "dir_tmp.$$";
my @paths       = ( $nonexistent, $readf, $writef, $dir );

# Ensure it really does not exist
unlink $nonexistent;

# Create a new read-only file
open my $blah, '>', $readf or die "Can't create $readf: $!";
print $blah $$;
close $blah;
chmod 0444, $readf;

# Create a writable file
open my $blah, '>', $writef or die "Can't create $writef: $!";
print $blah $$;
close $blah;
chmod 0644, $writef;

# Create an empty directory
mkdir $dir or die "Cannot create new dir $dir: $!";

sub tear_down {
    unlink $readf, $writef;
    rmdir $dir;
}

$SIG{__DIE__} = \&tear_down;

sub cdot {
    my $path = shift;
    return 0 if -e $path && -f $path && -w $path;
}

sub babar {
    my $path = shift;
    return 0 if -e $path && -f _ && -w _;
}

cmpthese(
    $count,
    {
        babar => sub { babar($_) for @paths },
        cdot  => sub { cdot($_) for @paths },
    }
);

__DATA__
          Rate  cdot babar
cdot  178571/s    --  -33%
babar 266667/s   49%    --

So local optimisation seems to pay here. As I'm not sure CDot is done, I won't commit it. But it would be nice to review the code and use _ anywhere possible (beware of side-effects though).

-- OlivierRaginel - 23 Jan 2012

Is this intended to carry over to Release01x01?

• Assigned to TopicUserMappingContrib
• Set to target: patch. Please correct if wrong.

-- PaulHarvey - 27 Jan 2012

Marginal, as it is a significant change to the install process (however should not impact upgraders in any way). I was hoping for feedback on the implementation before committing it to the patch branch.

-- CrawfordCurrie - 27 Jan 2012

It doesn't set the passwords_modifyable context any more, and password change and reset are inhibited on. Failing on trunk.foswiki.org and on my local branch. Restoring the enterContext('passwords_modifiable') to HtPasswdUser.pm.

-- GeorgeClark - 02 Feb 2012

Thanks George; I merged all the latest stuff from trunk, so Release01x01 should be all up to date now.

-- CrawfordCurrie - 12 Mar 2012

Commenting is disabled while not logged in