From: Tara L Andrews <tla@mit.edu>
Date: Sat, 14 Jul 2012 18:50:11 +0000 (+0200)
Subject: User store functionality actually in Directory; migrate changes there
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=b77f6c1bee7890af1189201ff40eb8005e3e671f;p=scpubgit%2Fstemmatology.git

User store functionality actually in Directory; migrate changes there
---

diff --git a/lib/Text/Tradition/Directory.pm b/lib/Text/Tradition/Directory.pm
index dfbbeee..0d7a2f9 100644
--- a/lib/Text/Tradition/Directory.pm
+++ b/lib/Text/Tradition/Directory.pm
@@ -376,8 +376,10 @@ sub add_user {
     my $password = $userinfo->{password};
     my $role = $userinfo->{role} || 'user';
 
-    return unless ($username =~ /^https?:/ 
-                   || ($username && $self->validate_password($password))) ;
+	throw( "No username given" ) unless $username;
+	throw( "Invalid password - must be at least " . $self->MIN_PASS_LEN 
+		. " characters long" )
+		unless ( $self->validate_password($password) || $username =~ /^https?:/ );
 
     my $user = Text::Tradition::User->new(
         id => $username,
@@ -468,11 +470,11 @@ sub modify_user {
     my $password = $userinfo->{password};
     my $role = $userinfo->{role};
 
-    return unless $username;
-    return if($password && !$self->validate_password($password));
+    throw( "Missing username or bad password" )
+    	unless $username && $self->validate_password($password);
 
     my $user = $self->find_user({ username => $username });
-    return unless $user;
+    throw( "Could not find user $username" ) unless $user;
 
     if($password) {
         $user->password(crypt_password($password));
@@ -502,10 +504,10 @@ sub deactivate_user {
     my ($self, $userinfo) = @_;
     my $username = $userinfo->{username};
 
-    return if !$username;
+    throw( "Need to specify a username for deactivation" ) unless $username;
 
     my $user = $self->find_user({ username => $username });
-    return if !$user;
+    throw( "User $username not found" ) unless $user;
 
     $user->active(0);
     foreach my $tradition (@{ $user->traditions }) {
@@ -536,10 +538,10 @@ sub reactivate_user {
     my ($self, $userinfo) = @_;
     my $username = $userinfo->{username};
 
-    return if !$username;
+    throw( "Need to specify a username for reactivation" ) unless $username;
 
     my $user = $self->lookup(Text::Tradition::User->id_for_user($username));
-    return if !$user;
+    throw( "User $username not found" ) unless $user;
 
     return $user if $user->active;
 
@@ -565,10 +567,10 @@ sub delete_user {
     my ($self, $userinfo) = @_;
     my $username = $userinfo->{username};
 
-    return if !$username;
+    throw( "Need to specify a username for deletion" ) unless $username;
 
     my $user = $self->find_user({ username => $username });
-    return if !$user;
+    throw( "User $username not found" ) unless $user;
 
     ## Should we be using Text::Tradition::Directory for this bit?
     $self->delete( @{ $user->traditions });
diff --git a/lib/Text/Tradition/UserStore.pm b/lib/Text/Tradition/UserStore.pm
index 03082f1..fdb6d47 100644
--- a/lib/Text/Tradition/UserStore.pm
+++ b/lib/Text/Tradition/UserStore.pm
@@ -82,7 +82,8 @@ sub add_user {
     my $password = $userinfo->{password};
 
 	throw( "No username given" ) unless $username;
-	throw( "Invalid password - too short?" )
+	throw( "Invalid password - must be at least " . $self->MIN_PASS_LEN 
+		. " characters long" )
 		unless ( $self->validate_password($password) || $username =~ /^https?:/ );
     
     my $user = Text::Tradition::User->new(
diff --git a/t/text_tradition_user.t b/t/text_tradition_user.t
index b9810b7..a401ebe 100644
--- a/t/text_tradition_user.t
+++ b/t/text_tradition_user.t
@@ -5,6 +5,7 @@ use warnings;
 
 use Test::More 'no_plan';
 use File::Temp;
+use TryCatch;
 
 use_ok('Text::Tradition::Directory');
 
@@ -21,6 +22,15 @@ my $scope = $user_store->new_scope;
 ## passwords
 my $shortpass = 'bloggs';
 ok(!$user_store->validate_password($shortpass), '"bloggs" is too short for a password');
+try {
+	my $dud_user = $user_store->add_user({ username => 'joe',
+										   password => $shortpass });
+	ok( 0, "User with short password should not have been created" );
+} catch ( Text::Tradition::Error $e ) {
+	is( $e->message, "Invalid password - must be at least " 
+		. $user_store->MIN_PASS_LEN . " characters long",
+		"Attempt to add user with too-short password threw correct error" );
+}
 
 ## create user
 my $new_user = $user_store->add_user({ username => 'fred',