Item9851: Item9848: Item9810: Fixes for remaining known groups issues…

… as of this time

Fixed problem with not being able to remove users when they has Main suffix
Fixed problem where users were seen as different if prefixed by Main when added.
Correct reporting when duplicate user is attempted added
Removed code no longer used
The creator of new group is not added to group if the user is admin unless he specifies it
Fixed problem with removing duplicate users
Removed old print STDERR statements


git-svn-id: http://svn.foswiki.org/trunk@9655 0b4bb1d4-4e5a-0410-9cc4-b2b747904278

TopicUserMappingContrib/lib/Foswiki/Users/TopicUserMapping.pm

@@ -814,14 +814,9 @@ sub addUserToGroup {
       $this->{session}
       ->normalizeWebTopicName( $Foswiki::cfg{UsersWebName}, $Group );
 
-    #the registration code will call this function using the rego agent
+    # the registration code will call this function using the rego agent
     my $user = $this->{session}->{user};
 
-#open Group topic, parse for the GROUPs setting, append new user
-#find where GROUP is set, use that code if we can, so that when it goes multi-line it copes
-#TODO: LATER: check for duplicates
-#TODO: make sure the groupName ends in Group...
-
     my $usersObj = $this->{session}->{users};
 
     print STDERR "$user, aka("
@@ -837,25 +832,12 @@ sub addUserToGroup {
 
     if ( $usersObj->isGroup($groupName) ) {
 
-       #if you set create for a group that exists, use that to force an upgrade.
-        if ( ( not $create )
-            and $usersObj->isInGroup( $cuid, $groupName, '0' ) )
-        {
-
-            #TODO: not sure this is the right thing to do -
-            #it might make more sense to not expand the nested groups,
-            #and add a user if they're not listed here,
-            #that way we are able to not worry about subgroups changing.
-            #            print STDERR "$user is already in $groupName\n";
-            return 1;    #user already in group, nothing to do
-        }
         $groupTopicObject =
           Foswiki::Meta->load( $this->{session}, $groupWeb, $groupName );
 
         if ( !$groupTopicObject->haveAccess( 'CHANGE', $user ) ) {
 
-            #can't change topic.
-            #            print STDERR "$user cannot change $groupName\n";
+            # can't change topic.
             return 0;
         }
 
@@ -868,14 +850,13 @@ sub addUserToGroup {
                 $membersString,    $allowChangeString
             );
 
-            #            print STDERR "cUID is not defined \n";
             return 1;
         }
     }
     else {
 
- #see if we have permission to add a topic, or to edit the existing topic, etc..
- #        print STDERR "Checking permissions ... ";
+    # see if we have permission to add a topic, or to edit the existing topic, etc..
+
         return 0 unless ($create);
         return 0
           unless (
@@ -884,39 +865,32 @@ sub addUserToGroup {
             )
           );
 
-        #        print STDERR "PASSED \n ";
         $groupTopicObject =
           Foswiki::Meta->load( $this->{session}, $groupWeb, 'GroupTemplate' );
 
-        #expand the GroupTemplate as best we can.
+        # expand the GroupTemplate as best we can.
         $this->{session}->{request}
           ->param( -name => 'topic', -value => $groupName );
         $groupTopicObject->expandNewTopic();
 
         $allowChangeString = $groupName;
     }
 
-    #    print STDERR "Adding cUID $cuid ";
     my $wikiName = $usersObj->getWikiName($cuid);
 
-    #    print STDERR "wikiname $wikiName \n";
     if ( $membersString !~ m/$wikiName/ ) {
         $membersString .= ', ' if ( $membersString ne '' );
         $membersString .= $wikiName;
     }
 
-    #    print STDERR "membersString = $membersString \n";
-
     $this->_clearGroupCache($groupName);
 
     $this->_writeGroupTopic(
         $groupTopicObject, $groupWeb, $groupName,
         $membersString,    $allowChangeString
     );
 
-    #    print STDERR "Write is complete \n";
-
-    #reparse groups brute force :/
+    # reparse groups brute force :/
     _getListOfGroups( $this, 1 ) if ($create);
     return 1;
 }
@@ -1024,7 +998,7 @@ sub removeUserFromGroup {
             ->topicExists( $Foswiki::cfg{UsersWebName}, $groupName ) )
       )
     {
-        if (   !$usersObj->isInGroup( $cuid, $groupName )
+        if (   !$usersObj->isInGroup( $cuid, $groupName, 0 )
             && !$usersObj->isGroup($cuid) )
         {
             return 0;    # user not in group - report that it failed
@@ -1042,6 +1016,7 @@ sub removeUserFromGroup {
         my $membersString = $groupTopicObject->getPreference('GROUP');
         my @l;
         foreach my $ident ( split( /[\,\s]+/, $membersString ) ) {
+            $ident =~ s/^($Foswiki::cfg{UsersWebName}|%USERSWEB%|%MAINWEB%)\.//;
             next if ( $ident eq $WikiName );
             next if ( $ident eq $LoginName );
             next if ( $ident eq $cuid );

core/lib/Foswiki/Func.pm

@@ -1319,10 +1319,8 @@ sub addUserToGroup {
     return () unless ( $users->isGroup($group) || $create );
     if ( !$users->isGroup($user) )
     {    #requires isInGroup to also work on nested groupnames
-        $user = getCanonicalUserID($user) || $user;;
+        $user = getCanonicalUserID($user) || $user;
         return unless ( defined($user) );
-
-        #        print STDERR "Func::addUserToGroup - user passed test\n";
     }
     return $users->addUserToGroup( $user, $group, $create );
 }
@@ -1343,7 +1341,7 @@ sub removeUserFromGroup {
 
     if ( !$users->isGroup($user) )
     {    #requires isInGroup to also work on nested groupnames
-        $user = getCanonicalUserID($user) || $user;;
+        $user = getCanonicalUserID($user) || $user;
         return unless ( defined($user) );
     }
     return $users->removeUserFromGroup( $user, $group );

core/lib/Foswiki/UI/Register.pm

@@ -611,9 +611,9 @@ sub addUserToGroup {
         or ( $userNames[0] eq '' ) )
     {
 
-    #if $create is set, and there are no users in the list, and the group exists
-    #then we're trying to upgrade the user topic.
-    #I'm not sure what other mappers might make of this..
+    # if $create is set, and there are no users in the list, and the group exists
+    # then we're trying to upgrade the user topic.
+    # I'm not sure what other mappers might make of this..
         if ( $create and Foswiki::Func::isGroup($groupName) ) {
             try {
                 Foswiki::Func::addUserToGroup( undef, $groupName, $create );
@@ -643,13 +643,16 @@ sub addUserToGroup {
         @userNames = split( /,\s*/, $userNames[0] );
     }
 
-    # TODO: SMELL: if you create a new group, make sure __you__ are the
-    # first user in the list, otherwise you won't be able to add more
-    # than one user.
-    # because this code saves once per user - and the group will be
-    # restricted to that group.
-    # for now, I'll add the currently logged in user to the list..
-    if ( !Foswiki::Func::isGroup($groupName) and $create ) {
+    # If a users create a new group, make sure he is in the group
+    # Otherwise he will not be able to touch the group after the
+    # first user is added because this code saves once per user - and the
+    # group will be restricted to that group. It is also good to prevent the
+    # user from shooting himself in the foot.
+    # He can afterwards remove himself if needed
+    # We make an exception if you are an admin as they can always edit anything
+
+    if ( !Foswiki::Func::isGroup($groupName) and 
+         !$session->{users}->isAdmin($user) and $create ) {
         unshift( @userNames, $session->{users}->getLoginName($user) );
     }
 
@@ -658,17 +661,18 @@ sub addUserToGroup {
     foreach my $u (@userNames) {
         $u =~ s/^\s+//;
         $u =~ s/\s+$//;
+        # We strip off any usersweb prefix
+        $u =~ s/^($Foswiki::cfg{UsersWebName}|%USERSWEB%|%MAINWEB%)\.//;
+
         next if ( $u eq '' );
 
         next if ( Foswiki::Func::isGroup($groupName) && Foswiki::Func::isGroupMember($groupName, $u, 0) );
 
         try {
             if ( Foswiki::Func::addUserToGroup( $u, $groupName, $create ) ) {
-#                print STDERR "Func adding $u succeeded\n";
                 push( @succeeded, $u );
             }
             else {
-#                print STDERR "Func adding $u failed\n";
                 push( @failed, $u );
 
                 # Log the error
@@ -686,7 +690,7 @@ sub addUserToGroup {
                 "catch: Failed to add $u to $groupName " . $e->stringify() );
         };
     }
-    if (@failed) {
+    if (@failed || !@succeeded) {
         throw Foswiki::OopsException(
             'attention',
             web    => $web,

core/lib/Foswiki/Users.pm

@@ -845,8 +845,6 @@ sub isInGroup {
 
     $expand = 1 unless ( defined $expand );
 
-#    print STDERR "Users::isInGroup called for $cUID in $group - expand $expand \n";
-
     my $mapping = $this->_getMapping($cUID);
     my $otherMapping =
       ( $mapping eq $this->{basemapping} )