Feature Proposal: Add User To Groups On Registration

Motivation

Add the a ability for Foswiki to be configured such that on User registration, the new user is added to a group, or set of groups.

Also, to extend the registrationHandler to pass the data that has been entered in the registration form - to allow for future expansion.

Similarly, this needs to be added to BulkRegistration.

At the same time, I'll look at refactoring registration, and moving it into TWikiUserMappingContrib.

Description and Documentation

Enable the registration script to add a new user to specified groups during the registration process.

2 basic scenarios:

1. an Admin User Registers User And Selects Groups
2. User Registers Self And Is Added To Group

To enable the greatest flexibility, I have added a new POST parmeter to TWikiRegistration Twk0AddToGroups, which will be a comma seperated list (or a HTML input that can return a list of values). The registration cgi script will then add the new user to the groups listed conditional on the TWikiTopicPermissions.

• Permissions for what group a user can be added to based on either:
  • Permissions of an authenticated user submitting the registration form (addresses first use case).
  • TWikiAdministrationAgent user permissions (in the case of a new user registering them selves.

If groups are requested for which the user does not have CHANGE permissions, they are silently ignored (a warning is added to the TWiki warning.txt file).

This means that to Always add a new user to a 'NewUserGoup' you would have to add CHANGE permission to that group's topic for TWikiRegistrationAgent, and add a hidden html input to the TWikiRegistration topic.

Thankyou to LynnwoodBrown for the original spec, and for encouraging me to look into it.

Examples

Impact

WhatDoesItAffect:

Auth, Documentation, Refactoring, Security, Usability

edit

Implementation

I have a patch to add groups for 4.1.2 -

ok, so we add

<tr>
  <td align="right"> %MAKETEXT{"TWiki Groups to join:"}% </td>
  <td>
    Main.EveryOneGroup: <input type="checkbox" name="Twk0AddToGroups" class="twikiInputField" value="EveryOneGroup" label="EveryOneGroup" />
    Main.TWikiAdminGroup: <input type="checkbox" name="Twk0AddToGroups" class="twikiInputField" value="TWikiAdminGroup" label="TWikiAdminGroup" />
    Main.TWikiRegistrationAgentGroup: <input type="checkbox" name="Twk0AddToGroups" class="twikiInputField" value="TWikiRegistrationAgentGroup" label="TWikiRegistrationAgentGroup" />
    Main.DoesNotExistGroup: <input type="checkbox" name="Twk0AddToGroups" class="twikiInputField" value="label="DoesNotExistGroup"" label="DoesNotExistGroup" />
  </td>
</tr>

to TWikiRegistration.

=== lib/TWiki/UI/Register.pm
==================================================================
--- lib/TWiki/UI/Register.pm (revision 31496)
+++ lib/TWiki/UI/Register.pm (local)
@@ -795,16 +795,22 @@
                                     params => [ $data->{WikiName} ] );
     }
 
-    # Plugin callback to set cookies.
-    $session->{plugins}->registrationHandler( $data->{WebName},
-                                              $data->{WikiName},
-                                              $data->{LoginName} );
+    #ported from 4.2.0
+    #only change the session's identity _if_ the registration was done by TWikiGuest
+    if ($session->{user}->login() eq $TWiki::cfg{DefaultUserLogin}) {
+        $session->writeDebug("changing user from ".$session->{user}->login().
+                               " to ".$data->{LoginName}) if DEBUG;
+        # Plugin callback to set cookies.
+        $session->{plugins}->registrationHandler( $data->{WebName},
+                                                  $data->{WikiName},
+                                                  $data->{LoginName} );
+    
+        # let the client session know that we're logged in. (This probably
+        # eliminates the need for the registrationHandler call above,
+        # but we'll leave them both in here for now.)
+        $session->{loginManager}->userLoggedIn( $data->{LoginName}, $data->{WikiName} );
+    }
 
-    # let the client session know that we're logged in. (This probably
-    # eliminates the need for the registrationHandler call above,
-    # but we'll leave them both in here for now.)
-    $session->{loginManager}->userLoggedIn( $data->{LoginName}, $data->{WikiName} );
-
     # add user to TWikiUsers topic
     my $user = $session->{users}->createUser( $data->{LoginName},
                                               $data->{WikiName} );
@@ -820,6 +826,10 @@
     my $log = _createUserTopic($session, 'NewUserTemplate', $data);
 
     $user->setEmails( $data->{Email} );
+    
+    foreach my $group (split(/,/, $data->{AddToGroups})) {
+        _addUserToGroup($session, $group, $data->{WikiName});
+    }
 
     # write log entry
     if ($TWiki::cfg{Log}{register}) {
@@ -845,6 +855,90 @@
                                 params => [ $status ] );
 }
 
+#TODO: this should be moved to be an add / remove user from group call
+sub _addUserToGroup {
+    my ($session, $groupName, $userName) = @_;
+    $groupName = TWiki::Sandbox::untaintUnchecked($groupName);
+    my ( $groupWeb, $groupTopic ) = $session->normalizeWebTopicName( $TWiki::cfg{UsersWebName}, $groupName );
+
+    
+    
+    #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 - should not happen, this is registration.
+    
+    #run this as calling user, if the registration is being run by an existing user
+    # (often done by admins)
+    my $user = $session->{user};
+    $session->writeDebug("TRYING to add $userName to $groupTopic, as ".$user->wikiName) if DEBUG;
+    if (($user->wikiName() eq $TWiki::cfg{DefaultUserWikiName}) ||
+        ($user->wikiName() eq $userName)) {
+        $user = $session->{users}->findUser( $twikiRegistrationAgent,
+                                             $twikiRegistrationAgent);
+        $session->writeDebug("using $twikiRegistrationAgent") if DEBUG;
+    }
+    
+    #code duplicated from 4.1.2 TWikiUserMapping::groupMembers
+    my $store = $session->{store};
+
+    if( $store->topicExists( $groupWeb, $groupTopic ) &&
+       $session->{security}->checkAccessPermission
+            ( 'change', $user, undef, undef, $groupWeb, $groupTopic )) {
+        my ($meta, $text) =
+          $store->readTopic($user, $groupWeb, $groupTopic);
+        my $before = '';
+        my $groupSetting = '';
+        my $after = '';
+        foreach my $line ( split( /\r?\n/, $text ) ) {
+            if( $line =~ /$TWiki::regex{setRegex}GROUP\s*=\s*(.+)$/ ) {
+                next unless( $1 eq 'Set' );
+                # Note: if there are multiple GROUP assignments in the
+                # topic, only the last will be taken.
+                if ($groupSetting eq '') {
+                    $groupSetting = $line;
+                } else {
+                    $before .= $line."\n".$after;
+                    $after = '';
+                    $groupSetting = $line;
+                }
+            } else {
+                if ($groupSetting eq '') {
+                    $before .= $line."\n";
+                } else {
+                    $after .= $line."\n";
+                }
+            }
+        }
+        if ($groupSetting ne '') {
+            #TODO: should probably normalise for saftey.
+            $groupSetting .= ', '.$userName."\n";
+            $text = $before.$groupSetting.$after;
+            # If there is an access control violation this will throw.
+            try {
+                $store->saveTopic( $user, $groupWeb, $groupTopic,
+                            $text, $meta, { minor => 1, comment => "adding $userName to $groupName" } );
+                $session->writeWarning("added $userName to $groupName");
+            } catch TWiki::AccessControlException with {
+                my $e = shift;
+                $session->writeWarning("ERROR1: cannot Save $groupName topic (".$e->stringify( $session ).")");
+            } catch TWiki::OopsException with {
+                my $e = shift;
+                $session->writeWarning("ERROR2: cannot Save $groupName topic (".$e->stringify( $session ).")");
+            } catch Error::Simple with {
+                my $e = shift;
+                $session->writeWarning("ERROR3: fallback cannot Save $groupName topic (".$e.")");
+            }
+        } else {
+                #error, not an ok GROUP topic
+            $session->writeWarning("ERROR4: cannot add $userName to $groupName - GROUP setting not valid");
+        }
+    } else {
+        #error, Group topic does not exist
+        $session->writeWarning("ERROR5: cannot add $userName to $groupName - Group topic does not exist / not changeable");
+    }
+}
+
+
 #Given a template and a hash, creates a new topic for a user
 #   1 reads the template topic
 #   2 calls RegistrationHandler::register with the row details, so that a plugin can augment/delete/change the entries
@@ -1283,9 +1377,11 @@
             my $form = {};
             $form->{required} = $2;
             my $name = $3;
-            my $value = $query->param($1.$2.$3);
+            my @values = $query->param($1.$2.$3);
+            my $value = join(',', @values);
             $form->{name} = $name;
             $form->{value} = $value;
+            $data->{$name} = $value;
             if ( $name eq 'Password' ) {
                 #TODO: get rid of this; move to removals and generalise.
                 $data->{passwordA} = $value;
@@ -1297,8 +1393,6 @@
             # change it, and 'Confirm' is a duplicate
             push( @{$data->{form}}, $form )
               unless ($name eq 'WikiName' || $name eq 'Confirm');
-
-            $data->{$name} = $value;
         }
     }
     $data->{WikiName} = TWiki::Sandbox::untaintUnchecked($data->{WikiName});

-- Contributors: SvenDowideit - 10 Dec 2007

Discussion

Nice proposal. However, other tools do better in terms of profile and user management. TWikiUsers should be managed with one central management tool.

-- MichaelDaum - 11 Dec 2007

Indeed. Got a propoal & some coding lined up? This proposal only adds the utility to add a user to Groups to the RegisterCgiScript. There is definatly a need for someone to design a full user management UI.

-- SvenDowideit - 11 Dec 2007

The groups that can be added to is limited to the set of groups that the TWikiRegistrationAgent has permission to edit. The KISS ness of this approach is that its up to the community of users to administer.

TWiki security is not violated.

re-wrote proposal, it wasn't clear at all.

-- SvenDowideit - 12 Dec 2007

Thanks a lot for that Sven. Much appreciated.

One question though: I have on my registration page a list of webs. In my register.pm, line 831, I added

my $webGroupToJoin = $group.'Group';

and sent that to your sub rather than $group, because I have a group associated with each web. Slight problem though. It doesn't work. It doesn't seem to append the "Group" on the end. Have I done something stupid?

Cheers

-- ChrisCauser - 17 Dec 2007

Sorry, can I change that: It is passing the variable along fine. It just isn't doing anything with it. Doesn't seem to be appending to the group file.

-- ChrisCauser - 17 Dec 2007

Nevermind. I can answer my own question: Yes, I did do something stupid.

When you try to save the group topic, the method first checks to see if you have permission. Since you are registering for the first time, and Group topic permissions are meant to be only editable by a member of that group. It's a bit of a catch 22. I'll work on a cure (although it won't be pretty since I'm no expert at perl)

-- ChrisCauser - 17 Dec 2007

I am sure the actual implementation will create some exchange of ideas but the basic spec is understood and I see no open objections so accepted by 14-day rule and ready to be implemented for Georgetown.

-- KennethLavrsen - 25 Dec 2007

Chris, the registration either uses the permisions of the current user OR the permission of the TWikiRegistrationAgent. so if you add TWikiRegistrationAgent to the list of allowed editors of the Group topic, all should be well.

-- SvenDowideit - 26 Dec 2007

Thanks a lot Sven.

I am currently away from the office, but I'll implement that as soon as possible.

Many thanks.

-- ChrisCauser - 02 Jan 2008

Yes, it does work

Cheers

-- ChrisCauser - 05 Jan 2008

Nice idea & much anticipated. It sounds like the spec does already include option for multiple groups & this would be good.

For example: new users are added to %!MAINWEB%Group and %HOMEWEB%Group (or whatever the case may be at the local installation)...

-- KeithHelfrich - 07 Jan 2008

Wouldn't it be easy for new users to be added into the admin group just like that? Or any group for that matter.

-- KwangErnLiew - 08 Jan 2008

I think it's OK because the page needs to be editable by TWikiRegistrationAgent, so the admin group is safe.

While I'm here, I want to upgrade to 4.2, but have noticed that this feature hasn't been implemented, and there isn't a patch for it as yet. Has anyone had any joy with getting this to work with 4.2?

-- ChrisCauser - 04 Feb 2008

Keith, yep, the current implementation works for more than one group at a time - up to http post limitations.

I'll update this for 4.2 - and commit it to 5.0 - at this point those 2 should be the same thing

-- SvenDowideit - 05 Feb 2008

Thanks Sven. You're a real champ. I'm trying to patch it myself, but too much of the code has changed for me to do a good job.

-- ChrisCauser - 06 Feb 2008

Just in case anyone is interested, I have a patched version which works in 4.2. I'm not posting it up because I'm sure Sven will do a much nicer job than me and I don't want to pollute this page with bad code. However, if anyone is absolutely desperate for it, let me know and I'll try and get it to you.

-- ChrisCauser - 08 Feb 2008

How would this work for Groups that require an existing member of the group for ALLOWTOPICCHANGE ?

-- KeithHelfrich - 12 Mar 2008

to quote the from december

the registration either uses the permisions of the current user OR the permission of the TWikiRegistrationAgent? . so if you add TWikiRegistrationAgent? to the list of allowed editors of the Group topic, all should be well.

whatever happens, it does not break the security policy.

-- SvenDowideit - 12 Mar 2008

Hmmm. I was looking at adding a feature for auto-registration of users to groups based on regular expression pattern matching their (validated) email address. At first look this would seem to be incompatible with allowing users to choose their own groups (self registration). The process would use very similar code... auto-editing at the point of registration only those group membership files to which either TWikiRegistration or the current user has write access, to avoid breaking any security rules.

Thinking a bit harder, maybe adding a new user agent TWikiRegistrationAutoGroupAgent would solve any incompatability issues with the slef-registration code posted above. Only groups with change rights by TWikiRegistrationAutoGroupAgent would be edited by the auto-registration code. By default the TWikiRegistrationAutoGroupAgent user would have no change rights to anything and would have to be explicitly enabled per group by the group owner in the Group topic. So that means no change of behaviour by default, which is a good thing.

For self-registration the webadmin or anyone else would add tickboxes to the web application form, and pass the groups via the CGI as proposed above.

For auto-registration, the group owner or web admin would have to add directives somewhere via a list of regexp pattern matches and the group to be joined at the time of registration.

The registration script would then generate the list of groups to join based on the pattern matches with the email address, before adding users to groups using the above _addUserToGroup code, but with the method API extended to allow passing an agent of either TWikiRegistrationAutoGroupAgent or TWikiRegistrationAgent, and checking change rights relative to the appropriate registration user agent.

Is there then an issue here with storing the patterns in the TwikiRegister topic and users being able to edit the TwikiRegister page to change the pattern matches? I guess yes there is. So would it be better to store these pattern/group settings in an area that is only accessible by TWikiAdminGroup by default e.g. a separate TWikiAutoGroupRegistrationConfig topic? I think not. That just adds complication. Equally the LocalSite.cfg file does not seem appropriate.

An alternative is to scan all of the web group topics for auto group registration directives. This seems to fits with the TWiki way of doing things. My idea would therefore be the topic based approach of scanning for TWikiRegistrationAutoGroupPattern directives in the individual group topics to avoid over-complicating the interface. That way the owner of the group or people with change access to the group stay in control of the group membership, which seems the sensible thing to do.

The registration UI script would generate a hash of all possible groups and then the register.cgi should do a foreach to check if the validated email matched that pattern, and if so, add the user to the group but only if TWikiRegistrationAutoGroupAgent has change access. The validated email cannot be edited by the user, so that at least means we've checked that they have an email account that is registered to them.

# rough code sketch
my $patterns=();
my $agent = "TWikiRegistrationAutoGroupAgent";

$session->fetchTWikiRegistrationAutoGroupPatterns(\$patterns);

my $group; # group
my $pattern; # regex pattern
foreach $group in (keys{$patterns}) {
  foreach $pattern in (keys{$patterns{$g}} {
    if ($validatedEmail =~ $pattern) {
       _addUserToGroup($session,$group,$data->{WikiName},$agent);
    }
  }
}

sub fetchTWikiRegistrationAutoGroupPatterns() {
my $rPatterns = shift;
# fetch list of Groups in this web
# parse each group file for GROUP directive and TWIKIREGISTRATIONAUTOGROUPPATTERN 
# build a ref to hash of the results
$rPatterns ->{$group}{$pttern}=$pttern;
return $rPatterns ;
}

Each group in the Twiki simply needs to have an optional section added to allow the auto-registration:

TWikiRegistrationAutoGroup regular expression pattern used to automatically add group membership when a user registers, based on their authenticated email address (ensure Main.!TWikiRegistrationAutoGroupAgent has ALLOWTOPICCHANGE rights to this group topic)

• #Set TWIKIREGISTRATIONAUTOGROUPPATTERN = .*@somecompany.com^

This could of course be added to the standard group creation template.

I think this is a fairly neat solution that does not impact the UI too much, is backward compatible, does not break security, is not too difficult to implement, and still leaves auto generated group membership in the hands of the group owner.

I realise this may grate with a general death or freedom mentality, but my (long) experience of Internet based systems suggests there is sometimes a need to limit some actions on some pages, and the easier you make group membership to configure for admins the more likely that it will work.

Question: Should I start a new feature request for this or not, as it does overlap considerably with the _addUserToGroup code?

But the bottom line is I'd like to propose a change to the _addUserToGroup method listed above in any case.

A generic _addUserToGroup method should not attempt to second guess who the user is and effectively grant suid rights IMVHO: that should be kept in register.pm or the calling app IMHO. _addUserToGroup should simply carry out a task that it has been asked to do: edit a group file to add a user (on behalf of an agent). Especially if you want to create a group.pm library later. In other words, split the mechanics of text editing, from the granting of various user rights. That should also save copying code from elsewhere.

# potentially in group.pm
sub _addUserToGroup {
# add user userName to group groupName, optionally using the security settings from $agentName

#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 - should not happen, this is registration.

my $session =shift; # session object
my $groupName =shift; a name of an existing TWiki Group. Calling with a non existing groupName will throw an error.
my $userName= shift; a name of an existing TWiki user. Calling with a non existing userName will throw an error.
my $agentName= shift; # a name of an existing TWiki user.
                                  # Calling with undef attempts to register the userName to the group using the user's own rights
                                  # Calling with a non existing userName or without sufficient rights will throw an error.

$agentName= $userName unless defined ($agentName);

....


# potentially in register.pm

sub _registerUserToGroup
    my ($session, $groupName, $userName) = @_;
    $groupName = TWiki::Sandbox::untaintUnchecked($groupName);
    my ( $groupWeb, $groupTopic ) = $session->normalizeWebTopicName( $TWiki::cfg{UsersWebName}, $groupName );
  


    #  the part that handles any SUID elements of setting an agent to TWikiRegistration
    # but only if current user is the TWiki guest user, and we are currently running as the registration cgi,
    # which then calls a generic _addUserToGroup with the agent set.

    my $agentName;
    #run this by default as calling user, if the registration is being run by an existing user
    # (often done by admins)
    my $agentName= $session->{user};
    
    # suid portion for handling guest registration forms
    if ($session->{user}->login() eq $TWiki::cfg{DefaultUserLogin})  {
        $user = $session->{users}->findUser( $twikiRegistrationAgent,
                                             $twikiRegistrationAgent);
        $agentName= $twikiRegistrationAgent;
    }

    $session->writeDebug("TRYING to add $userName to $groupTopic, as ".$agentName) if DEBUG;
 
    _addUserToGroup($session, $groupName, $userName,$agentName);
}

.....

-- RayHunter - 15 Apr 2008

register with an emailed request to be added to a group where an email was sent to the first group member on the list of group members to consider the inclusion into the group, the first name could be the group leader or trustee who can decide appropriate action.

-- TravisBarker - 15 Apr 2008

@ Travis: I already have this functionality in place on my TWiki. In the WebPreferences, it has the name of the person to email for registration confirmation. I post the code somewhere if you are interested.

-- ChrisCauser - 16 Apr 2008

Ray, you asked how to get checkin access. Just in case you didn't discover it, see SoYouWantToBeAFosWikiDeveloper for guidance.

Chirs, it would be nice if you could contribute your email confirmation code back for the benefit of the community.

-- CrawfordCurrie - 19 Apr 2008

Crawford, I know this isn't the right place to ask, but how do I do that?

-- ChrisCauser - 20 Apr 2008

Sorry to join the party late Sven

I've included a patch for Registration.pm which works with the latest version (4.2.3.)

• Register.patch: Patch for adding user to groups. Works with Twiki 4.2.3

-- ChrisCauser - 07 Oct 2008

Chris - did you apply for SVN checkin for core development?

If you are capable of writing the patch - you could as well be the committed developer of the feature am sure he is happy to see someone else run with this one.

Remember that we also need a set of unit tests to follow the feat

-- KennethLavrsen - 08 Oct 2008

Hi Kenneth,

I'm touched that you want me to be a core developer. If Sven's happy with me butchering his beautiful code, I'm happy to run with the ball

Not the place I know, but my twiki has the functionality that each Web has assigned a %WEBMASTER% who is emailed instead of the registree for confirmation (the registree picks which Web to join.) How would I go about sounding that out as a useful feature?

This links with this page as when the person is registered, he's added to the Web's group which defines the permissions for the Web.

-- ChrisCauser - 09 Oct 2008

Chris - doit - I don't have time to do even half of what I'd like to. Just remember to commit it to trunk, not the 4.2 branch

-- SvenDowideit - 27 Oct 2008

Hum. It looks like I need this code in 1.0.x, so I may be porting it to 1.1, when we get there.

-- SvenDowideit - 14 Feb 2009

I've attached a (git) patch for foswiki 1.0.4 which seems to to the job. I've tried to make it as generic as possible. It's a little rough around the edges but I've put it up early so as people can advise on how to proceed.

What it does

• Need to set {SeparateWebs} in LocalSite.cfg
• WebPreferences defines a REGISTRABLE flag which when on, adds the Web to the registration page.
• When you register to the web and requires verification is set, it emails the webmaster of the web (defined in WebPreferences)
• When you are confirmed on the web, you get added to the group WebGroup which defines permissions for the web.
• When you add a Web, it asks for a webmaster, and this webmaster is checked to see if he/she is a valid user.

Any help appreciated. I haven't done any testing on it because I don't yet know how to do it.

• patch.diff: patch.diff

-- ChrisCauser - 24 Apr 2009

Adding one which works with 1.0.5

[Edit: Removed because it was buggy.]

-- ChrisCauser - 30 Apr 2009

mmm, Chris, reading the patch quickly while the kids get fed (3am), it looks to me like the code you've written isn't the same functionality as I did above - its specific to having a group per web, rather than generically adding a user to a group on registration.

which makes it a separate feature really - with a need for others to consider what it does and how it does it.

• worse, is that it needs to be developed against trunk, so that the feature can go into 1.1 -
• and that unit tests do need to be written (see foswiki/test/unit/RegisterTests.pm)
• oh, and docco - I'm not in a position to devote enough time to working out from the code what you intended to write, so i can't comment on if there are any issues with that implementation

mmm, time to go again - month old twins

-- SvenDowideit - 30 Apr 2009

Thanks for the input Sven. Maybe I should have said that this doesn't have the same functionality as yours. I was thinking of Separating the Webs to that each one had a different Wiki master. For future work, I was hoping to plumb up email notifications so that it sent stuff from the web master rather than WIKIWEBMASTER.

• The documentation for making commits seems to omit how to do it (as in what's the preferred style.) Shall I make small commits that don't break functionality? Obviously I'll write the tests first.
• Documentation: I thought this wasn't done as I found very little documentation in the code anyway. Obviously I'll fix this.

Thanks again, and congratulations on the twins. At least you'll be balanced when holding them one each arm

-- ChrisCauser - 01 May 2009

wow, long time no progress, and now I'm in need of this code again - I've started to add the bones to trunk

-- SvenDowideit - 28 Sep 2009

• code functions and is in trunk - needs completion for release:
  

-- SvenDowideit - 23 Nov 2009