On 24 February 2018 at 12:49, Sam Ruby <ru...@intertwingly.net> wrote: > On Sat, Feb 24, 2018 at 7:11 AM, sebb <seb...@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 >>> ru...@apache.org.
