You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@whimsical.apache.org by ru...@apache.org on 2018/02/24 00:10:15 UTC

[whimsy] branch master updated: rough in ASF::Person#rename support (untested)

This is an automated email from the ASF dual-hosted git repository.

rubys pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/whimsy.git


The following commit(s) were added to refs/heads/master by this push:
     new d71f1df  rough in ASF::Person#rename support (untested)
d71f1df is described below

commit d71f1dffb3131c94ce6b6d5a53a8ee3e97ccb24a
Author: Sam Ruby <ru...@intertwingly.net>
AuthorDate: Fri Feb 23 19:09:53 2018 -0500

    rough in ASF::Person#rename support (untested)
---
 lib/whimsy/asf/ldap.rb | 59 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/lib/whimsy/asf/ldap.rb b/lib/whimsy/asf/ldap.rb
index 709b42f..f3d1e40 100644
--- a/lib/whimsy/asf/ldap.rb
+++ b/lib/whimsy/asf/ldap.rb
@@ -554,6 +554,43 @@ module ASF
       person
     end
 
+    # rename a person
+    def rename(newid, attrs={})
+      # ensure person exists in LDAP
+      raise ArgumentError(self.id) unless self.dn
+
+      # create a new person
+      new_person = ASF::Person.create(self.attrs.merge(attrs).merge(uid: newid))
+
+      # determine what groups the individual is a member of
+      uid_groups = ASF.search_subtree('dc=apache,dc=org', 
+        'memberUid=#{self.id}', 'dn').flatten
+      dn_groups = ASF.search_subtree('dc=apache,dc=org', 
+        'member=#{self.dn}', 'dn').flatten
+
+      # add new user to all groups
+      uid_groups.each do |dn|
+        ASF::LDAP.modify(dn, [ASF::Base.mod_add('memberUid', new_person.id)])
+      end
+      dn_groups.each do |dn|
+        ASF::LDAP.modify(dn, [ASF::Base.mod_add('member', new_person.dn)])
+      end
+
+      # remove original user from all groups
+      uid_groups.each do |dn|
+        ASF::LDAP.modify(dn, [ASF::Base.mod_delete('memberUid', self.id)])
+      end
+      dn_groups.each do |dn|
+        ASF::LDAP.modify(dn, [ASF::Base.mod_delete('member', self.dn)])
+      end
+
+      # remove original user
+      ASF::Person.remove(person.id)
+
+      # return new user
+      new_person
+    end
+
     # completely remove a committer from LDAP
     # ** DO NOT USE **
     # In almost all cases, use deregister instead
@@ -813,9 +850,19 @@ module ASF
         ASF::search_one(ASF::Person.base, 'uid=*', 'uidNumber').
           flatten.map(&:to_i).max + 1
 
-      nextgid = attrs['gidNumber'] ||
-        ASF::search_one(group_base, 'cn=*', 'gidNumber').
+      nextgid = attrs['gidNumber']
+      unless nextgid
+        nextgid = ASF::search_one(group_base, 'cn=*', 'gidNumber').
           flatten.map(&:to_i).max + 1
+
+        # create new LDAP group
+        entry = [
+          mod_add('objectClass', ['posixGroup', 'top']),
+          mod_add('cn', availid),
+          mod_add('userPassword', '{crypt}*'),
+          mod_add('gidNumber', nextgid.to_s),
+        ]
+      end
  
       # fixed attributes
       attrs.merge!({
@@ -842,14 +889,6 @@ module ASF
         end
       end
 
-      # create new LDAP group
-      entry = [
-        mod_add('objectClass', ['posixGroup', 'top']),
-        mod_add('cn', availid),
-        mod_add('userPassword', '{crypt}*'),
-        mod_add('gidNumber', nextgid.to_s),
-      ]
-
       ASF::LDAP.add("cn=#{availid},#{group_base}", entry)
 
       # create new LDAP person

-- 
To stop receiving notification emails like this one, please contact
rubys@apache.org.

Re: [whimsy] branch master updated: rough in ASF::Person#rename support (untested)

Posted by sebb <se...@gmail.com>.
On 24 February 2018 at 12:49, Sam Ruby <ru...@intertwingly.net> wrote:
> On Sat, Feb 24, 2018 at 7:11 AM, sebb <se...@gmail.com> wrote:
>> Some initial comments:
>>
>> If an id is re-used for the new account, what happens if someone uses
>> their id whilst the changes are being made?
>
> I don't understand the question.
>
> What do you mean by "re-used"? If x is renamed y, and later x is created?

I meant the *numeric* id is reused.

e.g. if x uses 123 and x is renamed to y using 123 as well.

> As to using the id whilst the changes are being made, my guess is that
> being "logged on" is an illusion; the next time LDAP is checked for a
> give id, that check would fail if that id has been renamed or deleted.
>
>> In general it's not a good idea to delete the original LDAP entry.
>> I think it's risky.
>
> The use case here is that the secretary makes a typo and that id was never used.

But how do you know that for sure?

>> Does it matter if there are gaps in the uid/gid?
>
> I doubt it.  IIRC, this code was copied from the current logic to
> create an account, probably in Perl.

>> AFAICT the code does not adjust the committers groups.
>
> Try the following commands to get an idea of what this code is
> attempting to handle in the "# add new user to all groups" and "#
> remove original user from all groups" code:
>
> ldapsearch -x memberUid=uid=sebb dn
> ldapsearch -x member=uid=sebb,ou=people,dc=apache,dc=org dn

Doh.

>> I'm not sure that the calculation of nextuid and nextgid are safe in a
>> multi-processing environment.
>
> Probably not.

>> Also it looks like the uid and gid can be different - is that allowed?
>>
>> ==
>>
>> There are other non-LDAP changes that need to be made, for example
>> updating the qmail files on hermes
>
> The cron job that creates qmail files (generate-dotqmail-availid.py)
> will create new ones.  The old files will be orphaned, and presumably
> will be overwritten if the id were to be reused.

OK

>> And home directory on home.a.o?
>
> I believe that that is created on first login, so again there would be
> an orphaned directory.  As Chris Lambertus pointed out on anther
> thread, if this function were to be used on an id that had been
> actively used, there might be other things like host entries (which,
> for example, allow you to ssh into whimsy-vm4) that would need to be
> changed; but the intent is that this function only be used for the use
> case of renaming an id that was incorrect in the first place.

It's less fragile to assume that the user id may have been used.

So I think a safer approach is:
- disable the old user id
- add the new id to the same groups
- tidy up the old id groups.

If the user id can be shown to have not been used, it could then be
deleted as a separate step.

> Once the underlying script is thought to be correct, we could create a
> small CGI script that the secretary could use to rename an id.  Such a
> CGI could have a dropdown for the id to be renamed, which could be
> populated with the last 'n' ids which were recently created (i.e., the
> ones with the highest uid).

> - Sam Ruby
>
>> On 24 February 2018 at 00:10,  <ru...@apache.org> wrote:
>>> This is an automated email from the ASF dual-hosted git repository.
>>>
>>> rubys pushed a commit to branch master
>>> in repository https://gitbox.apache.org/repos/asf/whimsy.git
>>>
>>>
>>> The following commit(s) were added to refs/heads/master by this push:
>>>      new d71f1df  rough in ASF::Person#rename support (untested)
>>> d71f1df is described below
>>>
>>> commit d71f1dffb3131c94ce6b6d5a53a8ee3e97ccb24a
>>> Author: Sam Ruby <ru...@intertwingly.net>
>>> AuthorDate: Fri Feb 23 19:09:53 2018 -0500
>>>
>>>     rough in ASF::Person#rename support (untested)
>>> ---
>>>  lib/whimsy/asf/ldap.rb | 59 +++++++++++++++++++++++++++++++++++++++++---------
>>>  1 file changed, 49 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/lib/whimsy/asf/ldap.rb b/lib/whimsy/asf/ldap.rb
>>> index 709b42f..f3d1e40 100644
>>> --- a/lib/whimsy/asf/ldap.rb
>>> +++ b/lib/whimsy/asf/ldap.rb
>>> @@ -554,6 +554,43 @@ module ASF
>>>        person
>>>      end
>>>
>>> +    # rename a person
>>> +    def rename(newid, attrs={})
>>> +      # ensure person exists in LDAP
>>> +      raise ArgumentError(self.id) unless self.dn
>>> +
>>> +      # create a new person
>>> +      new_person = ASF::Person.create(self.attrs.merge(attrs).merge(uid: newid))
>>> +
>>> +      # determine what groups the individual is a member of
>>> +      uid_groups = ASF.search_subtree('dc=apache,dc=org',
>>> +        'memberUid=#{self.id}', 'dn').flatten
>>> +      dn_groups = ASF.search_subtree('dc=apache,dc=org',
>>> +        'member=#{self.dn}', 'dn').flatten
>>> +
>>> +      # add new user to all groups
>>> +      uid_groups.each do |dn|
>>> +        ASF::LDAP.modify(dn, [ASF::Base.mod_add('memberUid', new_person.id)])
>>> +      end
>>> +      dn_groups.each do |dn|
>>> +        ASF::LDAP.modify(dn, [ASF::Base.mod_add('member', new_person.dn)])
>>> +      end
>>> +
>>> +      # remove original user from all groups
>>> +      uid_groups.each do |dn|
>>> +        ASF::LDAP.modify(dn, [ASF::Base.mod_delete('memberUid', self.id)])
>>> +      end
>>> +      dn_groups.each do |dn|
>>> +        ASF::LDAP.modify(dn, [ASF::Base.mod_delete('member', self.dn)])
>>> +      end
>>> +
>>> +      # remove original user
>>> +      ASF::Person.remove(person.id)
>>> +
>>> +      # return new user
>>> +      new_person
>>> +    end
>>> +
>>>      # completely remove a committer from LDAP
>>>      # ** DO NOT USE **
>>>      # In almost all cases, use deregister instead
>>> @@ -813,9 +850,19 @@ module ASF
>>>          ASF::search_one(ASF::Person.base, 'uid=*', 'uidNumber').
>>>            flatten.map(&:to_i).max + 1
>>>
>>> -      nextgid = attrs['gidNumber'] ||
>>> -        ASF::search_one(group_base, 'cn=*', 'gidNumber').
>>> +      nextgid = attrs['gidNumber']
>>> +      unless nextgid
>>> +        nextgid = ASF::search_one(group_base, 'cn=*', 'gidNumber').
>>>            flatten.map(&:to_i).max + 1
>>> +
>>> +        # create new LDAP group
>>> +        entry = [
>>> +          mod_add('objectClass', ['posixGroup', 'top']),
>>> +          mod_add('cn', availid),
>>> +          mod_add('userPassword', '{crypt}*'),
>>> +          mod_add('gidNumber', nextgid.to_s),
>>> +        ]
>>> +      end
>>>
>>>        # fixed attributes
>>>        attrs.merge!({
>>> @@ -842,14 +889,6 @@ module ASF
>>>          end
>>>        end
>>>
>>> -      # create new LDAP group
>>> -      entry = [
>>> -        mod_add('objectClass', ['posixGroup', 'top']),
>>> -        mod_add('cn', availid),
>>> -        mod_add('userPassword', '{crypt}*'),
>>> -        mod_add('gidNumber', nextgid.to_s),
>>> -      ]
>>> -
>>>        ASF::LDAP.add("cn=#{availid},#{group_base}", entry)
>>>
>>>        # create new LDAP person
>>>
>>> --
>>> To stop receiving notification emails like this one, please contact
>>> rubys@apache.org.

Re: [whimsy] branch master updated: rough in ASF::Person#rename support (untested)

Posted by Sam Ruby <ru...@intertwingly.net>.
On Sat, Feb 24, 2018 at 7:11 AM, sebb <se...@gmail.com> wrote:
> Some initial comments:
>
> If an id is re-used for the new account, what happens if someone uses
> their id whilst the changes are being made?

I don't understand the question.

What do you mean by "re-used"? If x is renamed y, and later x is created?

As to using the id whilst the changes are being made, my guess is that
being "logged on" is an illusion; the next time LDAP is checked for a
give id, that check would fail if that id has been renamed or deleted.

> In general it's not a good idea to delete the original LDAP entry.
> I think it's risky.

The use case here is that the secretary makes a typo and that id was never used.

> Does it matter if there are gaps in the uid/gid?

I doubt it.  IIRC, this code was copied from the current logic to
create an account, probably in Perl.

> AFAICT the code does not adjust the committers groups.

Try the following commands to get an idea of what this code is
attempting to handle in the "# add new user to all groups" and "#
remove original user from all groups" code:

ldapsearch -x memberUid=uid=sebb dn
ldapsearch -x member=uid=sebb,ou=people,dc=apache,dc=org dn

> I'm not sure that the calculation of nextuid and nextgid are safe in a
> multi-processing environment.

Probably not.

> Also it looks like the uid and gid can be different - is that allowed?
>
> ==
>
> There are other non-LDAP changes that need to be made, for example
> updating the qmail files on hermes

The cron job that creates qmail files (generate-dotqmail-availid.py)
will create new ones.  The old files will be orphaned, and presumably
will be overwritten if the id were to be reused.

> And home directory on home.a.o?

I believe that that is created on first login, so again there would be
an orphaned directory.  As Chris Lambertus pointed out on anther
thread, if this function were to be used on an id that had been
actively used, there might be other things like host entries (which,
for example, allow you to ssh into whimsy-vm4) that would need to be
changed; but the intent is that this function only be used for the use
case of renaming an id that was incorrect in the first place.

Once the underlying script is thought to be correct, we could create a
small CGI script that the secretary could use to rename an id.  Such a
CGI could have a dropdown for the id to be renamed, which could be
populated with the last 'n' ids which were recently created (i.e., the
ones with the highest uid).

- Sam Ruby

> On 24 February 2018 at 00:10,  <ru...@apache.org> wrote:
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> rubys pushed a commit to branch master
>> in repository https://gitbox.apache.org/repos/asf/whimsy.git
>>
>>
>> The following commit(s) were added to refs/heads/master by this push:
>>      new d71f1df  rough in ASF::Person#rename support (untested)
>> d71f1df is described below
>>
>> commit d71f1dffb3131c94ce6b6d5a53a8ee3e97ccb24a
>> Author: Sam Ruby <ru...@intertwingly.net>
>> AuthorDate: Fri Feb 23 19:09:53 2018 -0500
>>
>>     rough in ASF::Person#rename support (untested)
>> ---
>>  lib/whimsy/asf/ldap.rb | 59 +++++++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 49 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/whimsy/asf/ldap.rb b/lib/whimsy/asf/ldap.rb
>> index 709b42f..f3d1e40 100644
>> --- a/lib/whimsy/asf/ldap.rb
>> +++ b/lib/whimsy/asf/ldap.rb
>> @@ -554,6 +554,43 @@ module ASF
>>        person
>>      end
>>
>> +    # rename a person
>> +    def rename(newid, attrs={})
>> +      # ensure person exists in LDAP
>> +      raise ArgumentError(self.id) unless self.dn
>> +
>> +      # create a new person
>> +      new_person = ASF::Person.create(self.attrs.merge(attrs).merge(uid: newid))
>> +
>> +      # determine what groups the individual is a member of
>> +      uid_groups = ASF.search_subtree('dc=apache,dc=org',
>> +        'memberUid=#{self.id}', 'dn').flatten
>> +      dn_groups = ASF.search_subtree('dc=apache,dc=org',
>> +        'member=#{self.dn}', 'dn').flatten
>> +
>> +      # add new user to all groups
>> +      uid_groups.each do |dn|
>> +        ASF::LDAP.modify(dn, [ASF::Base.mod_add('memberUid', new_person.id)])
>> +      end
>> +      dn_groups.each do |dn|
>> +        ASF::LDAP.modify(dn, [ASF::Base.mod_add('member', new_person.dn)])
>> +      end
>> +
>> +      # remove original user from all groups
>> +      uid_groups.each do |dn|
>> +        ASF::LDAP.modify(dn, [ASF::Base.mod_delete('memberUid', self.id)])
>> +      end
>> +      dn_groups.each do |dn|
>> +        ASF::LDAP.modify(dn, [ASF::Base.mod_delete('member', self.dn)])
>> +      end
>> +
>> +      # remove original user
>> +      ASF::Person.remove(person.id)
>> +
>> +      # return new user
>> +      new_person
>> +    end
>> +
>>      # completely remove a committer from LDAP
>>      # ** DO NOT USE **
>>      # In almost all cases, use deregister instead
>> @@ -813,9 +850,19 @@ module ASF
>>          ASF::search_one(ASF::Person.base, 'uid=*', 'uidNumber').
>>            flatten.map(&:to_i).max + 1
>>
>> -      nextgid = attrs['gidNumber'] ||
>> -        ASF::search_one(group_base, 'cn=*', 'gidNumber').
>> +      nextgid = attrs['gidNumber']
>> +      unless nextgid
>> +        nextgid = ASF::search_one(group_base, 'cn=*', 'gidNumber').
>>            flatten.map(&:to_i).max + 1
>> +
>> +        # create new LDAP group
>> +        entry = [
>> +          mod_add('objectClass', ['posixGroup', 'top']),
>> +          mod_add('cn', availid),
>> +          mod_add('userPassword', '{crypt}*'),
>> +          mod_add('gidNumber', nextgid.to_s),
>> +        ]
>> +      end
>>
>>        # fixed attributes
>>        attrs.merge!({
>> @@ -842,14 +889,6 @@ module ASF
>>          end
>>        end
>>
>> -      # create new LDAP group
>> -      entry = [
>> -        mod_add('objectClass', ['posixGroup', 'top']),
>> -        mod_add('cn', availid),
>> -        mod_add('userPassword', '{crypt}*'),
>> -        mod_add('gidNumber', nextgid.to_s),
>> -      ]
>> -
>>        ASF::LDAP.add("cn=#{availid},#{group_base}", entry)
>>
>>        # create new LDAP person
>>
>> --
>> To stop receiving notification emails like this one, please contact
>> rubys@apache.org.

Re: [whimsy] branch master updated: rough in ASF::Person#rename support (untested)

Posted by sebb <se...@gmail.com>.
Some initial comments:

If an id is re-used for the new account, what happens if someone uses
their id whilst the changes are being made?

In general it's not a good idea to delete the original LDAP entry.
I think it's risky.
Does it matter if there are gaps in the uid/gid?

AFAICT the code does not adjust the committers groups.

I'm not sure that the calculation of nextuid and nextgid are safe in a
multi-processing environment.

Also it looks like the uid and gid can be different - is that allowed?

==

There are other non-LDAP changes that need to be made, for example
updating the qmail files on hermes
And home directory on home.a.o?

On 24 February 2018 at 00:10,  <ru...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> rubys pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/whimsy.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
>      new d71f1df  rough in ASF::Person#rename support (untested)
> d71f1df is described below
>
> commit d71f1dffb3131c94ce6b6d5a53a8ee3e97ccb24a
> Author: Sam Ruby <ru...@intertwingly.net>
> AuthorDate: Fri Feb 23 19:09:53 2018 -0500
>
>     rough in ASF::Person#rename support (untested)
> ---
>  lib/whimsy/asf/ldap.rb | 59 +++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/lib/whimsy/asf/ldap.rb b/lib/whimsy/asf/ldap.rb
> index 709b42f..f3d1e40 100644
> --- a/lib/whimsy/asf/ldap.rb
> +++ b/lib/whimsy/asf/ldap.rb
> @@ -554,6 +554,43 @@ module ASF
>        person
>      end
>
> +    # rename a person
> +    def rename(newid, attrs={})
> +      # ensure person exists in LDAP
> +      raise ArgumentError(self.id) unless self.dn
> +
> +      # create a new person
> +      new_person = ASF::Person.create(self.attrs.merge(attrs).merge(uid: newid))
> +
> +      # determine what groups the individual is a member of
> +      uid_groups = ASF.search_subtree('dc=apache,dc=org',
> +        'memberUid=#{self.id}', 'dn').flatten
> +      dn_groups = ASF.search_subtree('dc=apache,dc=org',
> +        'member=#{self.dn}', 'dn').flatten
> +
> +      # add new user to all groups
> +      uid_groups.each do |dn|
> +        ASF::LDAP.modify(dn, [ASF::Base.mod_add('memberUid', new_person.id)])
> +      end
> +      dn_groups.each do |dn|
> +        ASF::LDAP.modify(dn, [ASF::Base.mod_add('member', new_person.dn)])
> +      end
> +
> +      # remove original user from all groups
> +      uid_groups.each do |dn|
> +        ASF::LDAP.modify(dn, [ASF::Base.mod_delete('memberUid', self.id)])
> +      end
> +      dn_groups.each do |dn|
> +        ASF::LDAP.modify(dn, [ASF::Base.mod_delete('member', self.dn)])
> +      end
> +
> +      # remove original user
> +      ASF::Person.remove(person.id)
> +
> +      # return new user
> +      new_person
> +    end
> +
>      # completely remove a committer from LDAP
>      # ** DO NOT USE **
>      # In almost all cases, use deregister instead
> @@ -813,9 +850,19 @@ module ASF
>          ASF::search_one(ASF::Person.base, 'uid=*', 'uidNumber').
>            flatten.map(&:to_i).max + 1
>
> -      nextgid = attrs['gidNumber'] ||
> -        ASF::search_one(group_base, 'cn=*', 'gidNumber').
> +      nextgid = attrs['gidNumber']
> +      unless nextgid
> +        nextgid = ASF::search_one(group_base, 'cn=*', 'gidNumber').
>            flatten.map(&:to_i).max + 1
> +
> +        # create new LDAP group
> +        entry = [
> +          mod_add('objectClass', ['posixGroup', 'top']),
> +          mod_add('cn', availid),
> +          mod_add('userPassword', '{crypt}*'),
> +          mod_add('gidNumber', nextgid.to_s),
> +        ]
> +      end
>
>        # fixed attributes
>        attrs.merge!({
> @@ -842,14 +889,6 @@ module ASF
>          end
>        end
>
> -      # create new LDAP group
> -      entry = [
> -        mod_add('objectClass', ['posixGroup', 'top']),
> -        mod_add('cn', availid),
> -        mod_add('userPassword', '{crypt}*'),
> -        mod_add('gidNumber', nextgid.to_s),
> -      ]
> -
>        ASF::LDAP.add("cn=#{availid},#{group_base}", entry)
>
>        # create new LDAP person
>
> --
> To stop receiving notification emails like this one, please contact
> rubys@apache.org.