Merge branch '16689-shell-sync-groups' refs #16689

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

diff --combined services/login-sync/bin/arvados-login-sync
index aee911cfc43fca5806e700a40437f2641d63abe8,9152b29336b7ed44225c0a2a43a56ae1ac93d705..8162e22a2ff6f815ac7904d28ad6ec0413faa7c3
--- 1/services/login-sync/bin/arvados-login-sync
--- 2/services/login-sync/bin/arvados-login-sync
+++ b/services/login-sync/bin/arvados-login-sync
@@@ -91,36 -91,63 +91,63 @@@ begi
    end
  
    seen = Hash.new()
-   devnull = open("/dev/null", "w")
+ 
+   current_user_groups = Hash.new
+   while (ent = Etc.getgrent()) do
+     ent.mem.each do |member|
+       current_user_groups[member] ||= Array.new
+       current_user_groups[member].push ent.name
+     end
+   end
+   Etc.endgrent()
  
    logins.each do |l|
      next if seen[l[:username]]
      seen[l[:username]] = true
  
+     username = l[:username]
+ 
      unless pwnam[l[:username]]
        STDERR.puts "Creating account #{l[:username]}"
-       groups = l[:groups] || []
-       # Adding users to the FUSE group has long been hardcoded behavior.
-       groups << "fuse"
-       groups.select! { |g| Etc.getgrnam(g) rescue false }
        # Create new user
        unless system("useradd", "-m",
-                 "-c", l[:username],
+                 "-c", username,
                  "-s", "/bin/bash",
-                 "-G", groups.join(","),
-                 l[:username],
-                 out: devnull)
+                 username)
          STDERR.puts "Account creation failed for #{l[:username]}: #{$?}"
          next
        end
        begin
-         pwnam[l[:username]] = Etc.getpwnam(l[:username])
+         pwnam[username] = Etc.getpwnam(username)
        rescue => e
          STDERR.puts "Created account but then getpwnam() failed for #{l[:username]}: #{e}"
          raise
        end
      end
  
+     existing_groups = current_user_groups[username] || []
+     groups = l[:groups] || []
+     # Adding users to the FUSE group has long been hardcoded behavior.
+     groups << "fuse"
+     groups << username
+     groups.select! { |g| Etc.getgrnam(g) rescue false }
+ 
+     groups.each do |addgroup|
+       if existing_groups.index(addgroup).nil?
+         # User should be in group, but isn't, so add them.
+         STDERR.puts "Add user #{username} to #{addgroup} group"
+         system("adduser", username, addgroup)
+       end
+     end
+ 
+     existing_groups.each do |removegroup|
+       if groups.index(removegroup).nil?
+         # User is in a group, but shouldn't be, so remove them.
+         STDERR.puts "Remove user #{username} from #{removegroup} group"
+         system("deluser", username, removegroup)
+       end
+     end
+ 
      homedir = pwnam[l[:username]].dir
      userdotssh = File.join(homedir, ".ssh")
      Dir.mkdir(userdotssh) if !File.exist?(userdotssh)
@@@ -154,6 -181,8 +181,6 @@@
      userdotconfig = File.join(homedir, ".config")
      if !File.exist?(userdotconfig)
        Dir.mkdir(userdotconfig)
 -      FileUtils.chown_R(l[:username], nil, userdotconfig)
 -      File.chmod(0700, userdotconfig)
      end
  
      configarvados = File.join(userdotconfig, "arvados")
@@@ -168,24 -197,20 +195,23 @@@
          f.write("ARVADOS_API_HOST=#{ENV['ARVADOS_API_HOST']}\n")
          f.write("ARVADOS_API_TOKEN=v2/#{user_token[:uuid]}/#{user_token[:api_token]}\n")
          f.close()
 -        File.chmod(0600, tokenfile)
        end
      rescue => e
        STDERR.puts "Error setting token for #{l[:username]}: #{e}"
      end
  
      FileUtils.chown_R(l[:username], nil, userdotssh)
 -    FileUtils.chown_R(l[:username], nil, configarvados)
 +    FileUtils.chown_R(l[:username], nil, userdotconfig)
      File.chmod(0700, userdotssh)
 +    File.chmod(0700, userdotconfig)
 +    File.chmod(0700, configarvados)
      File.chmod(0750, homedir)
      File.chmod(0600, keysfile)
 -    File.chmod(0700, configarvados)
 +    if File.exist?(tokenfile)
 +      File.chmod(0600, tokenfile)
 +    end
    end
  
-   devnull.close
  rescue Exception => bang
    puts "Error: " + bang.to_s
    puts bang.backtrace.join("\n")