From 330ab4494bf84ac9c9eb91eddf37b01addc4f345 Mon Sep 17 00:00:00 2001
From: William Brown <william@blackhats.net.au>
Date: May 22 2019 02:29:24 +0000
Subject: Ticket 50399 - replication manager create incorrectly changes agreements


Bug Description: During dsconf instance replication create-manager the command incorrectly modifies
all existing outbound agreements to use the new dn.

-- It is a surprise behaviour - the command only says it is creating a manager, not changing
  configuration. It should only do that one thing.

-- The manager we are creating is for replicas to authenticate to this server, not for this server
  to provide outbound replications. Changing the outbound agreements to use the DN of the user on
  this server is incorrect

-- We are potentially trampling existing intent and configuration. Imagine I have server A and B.
  On A I have "incoming manager B" and on B I have "incoming manager A". These agreements were
  manually configured and existing. Adding the replication manager on A would break my outgoing
  agreement to B now.

Fix Description: Remove the behaviour from the command. Some code cleanups
were also performed to use the get_arg which exists to help reduce boilerplate
throughout the codebase.

https://pagure.io/389-ds-base/issue/50399

Author: William Brown <william@blackhats.net.au>

Review by: ???

---

diff --git a/src/lib389/lib389/cli_conf/replication.py b/src/lib389/lib389/cli_conf/replication.py
index b25eba6..a062f1d 100644
--- a/src/lib389/lib389/cli_conf/replication.py
+++ b/src/lib389/lib389/cli_conf/replication.py
@@ -384,12 +384,8 @@ def get_cl(inst, basedn, log, args):
 
 
 def create_repl_manager(inst, basedn, log, args):
-    manager_cn = "replication manager"
-    repl_manager_password = ""
-    repl_manager_password_confirm = ""
-
-    if args.name:
-        manager_cn = args.name
+    manager_cn = _get_arg(args.name)
+    repl_manager_password = _get_arg(args.passwd, hidden=True, confirm=True)
 
     if is_a_dn(manager_cn):
         # A full DN was provided, make sure it uses "cn" for the RDN
@@ -401,55 +397,12 @@ def create_repl_manager(inst, basedn, log, args):
     else:
         manager_dn = "cn={},cn=config".format(manager_cn)
 
-    if args.passwd:
-        repl_manager_password = args.passwd
-    else:
-        # Prompt for password
-        while 1:
-            while repl_manager_password == "":
-                repl_manager_password = getpass("Enter replication manager password: ")
-            while repl_manager_password_confirm == "":
-                repl_manager_password_confirm = getpass("Confirm replication manager password: ")
-            if repl_manager_password_confirm == repl_manager_password:
-                break
-            else:
-                log.info("Passwords do not match!\n")
-                repl_manager_password = ""
-                repl_manager_password_confirm = ""
-
     manager = BootstrapReplicationManager(inst, dn=manager_dn)
-    try:
-        manager.create(properties={
-            'cn': manager_cn,
-            'userPassword': repl_manager_password
-        })
-        if args.suffix:
-            # Add supplier DN to config only if add succeeds
-            replicas = Replicas(inst)
-            replica = replicas.get(args.suffix)
-            try:
-                replica.add('nsds5ReplicaBindDN', manager_dn)
-            except ldap.TYPE_OR_VALUE_EXISTS:
-                pass
-        log.info("Successfully created replication manager: " + manager_dn)
-    except ldap.ALREADY_EXISTS:
-        log.info("Replication Manager ({}) already exists, recreating it...".format(manager_dn))
-        # Already there, but could have different password.  Delete and recreate
-        manager.delete()
-        manager.create(properties={
-            'cn': manager_cn,
-            'userPassword': repl_manager_password
-        })
-        if args.suffix:
-            # Add supplier DN to config only if add succeeds
-            replicas = Replicas(inst)
-            replica = replicas.get(args.suffix)
-            try:
-                replica.add('nsds5ReplicaBindDN', manager_dn)
-            except ldap.TYPE_OR_VALUE_EXISTS:
-                pass
-
-        log.info("Successfully created replication manager: " + manager_dn)
+    manager.ensure_state(properties={
+        'cn': manager_cn,
+        'userPassword': repl_manager_password
+    })
+    log.info("Successfully created replication manager: " + manager_dn)
 
 
 def del_repl_manager(inst, basedn, log, args):
@@ -934,10 +887,9 @@ def create_parser(subparsers):
     repl_add_manager_parser.set_defaults(func=create_repl_manager)
     repl_add_manager_parser.add_argument('--name', help="The NAME of the new replication manager entry.  For example, " +
                                                         "if the NAME is \"replication manager\" then the new manager " +
-                                                        "entry's DN would be \"cn=replication manager,cn=config\".")
+                                                        "entry's DN would be \"cn=replication manager,cn=config\".",
+                                                        default="replication manager")
     repl_add_manager_parser.add_argument('--passwd', help="Password for replication manager.  If not provided, you will be prompted for the password")
-    repl_add_manager_parser.add_argument('--suffix', help='The DN of the replication suffix whose replication ' +
-                                                          'configuration you want to add this new manager to (OPTIONAL)')
 
     repl_del_manager_parser = repl_subcommands.add_parser('delete-manager', help='Delete a replication manager entry')
     repl_del_manager_parser.set_defaults(func=del_repl_manager)