Deployment fix for --auto_create_cli_app flag bug

docs/getting-started.md

@@ -30,7 +30,7 @@ On a host with the [Azure CLI logged
 in](https://docs.microsoft.com/en-us/cli/azure/authenticate-azure-cli?view=azure-cli-latest),
 do the following:
 
-```
+```console
 unzip onefuzz-deployment-$VERSION.zip
 pip install -r requirements.txt
 chmod +x deploy.py
@@ -40,7 +40,7 @@ chmod +x deploy.py
 When running `deploy.py` the first time for an instance, you will be prompted
 to follow a manual step to initialize your CLI config.
 
-The `$NSG_CONFIG_FILE` is a required parameter that specifies the 'allow rules' for the OneFuzz Network Security Group. A default `config.json` is provided in the deployment zip.
+The `$NSG_CONFIG_FILE` is a required parameter that specifies the 'allow rules' for the OneFuzz Network Security Group as well as other basic OneFuzz settings. A `config.json` file is provided with default NSG values. 
 This 'allow' config resembles the following:
 
 ```json
@@ -52,7 +52,45 @@ This 'allow' config resembles the following:
 }
 ```
 
-Future updates can be made to this configuration via the OneFuzz CLI. 
+>#### Note: 
+> - Line #5 in the example `config.json` inside of the deployment.zip has the parameter for `"cli_client_id": "",` 
+>   - You'll need to add your CLI app registration ID to this parameter's value for deployments and upgrade deployments
+>    **unless** you're deploying and passing the `--auto_create_cli_app` flag to create a new App ID during the deployment.
+>   - If you wanted to create a new App ID at deployment and use this flag, you need to delete this line to remove the `cli_client_id` key from your config file.
+
+**Example deployment config.json:**
+
+```json
+{
+    "tenant_id": "05c88c2c-55f6-4a51-81db-cdbbf759fa75",
+    "tenant_domain": "azurewebsites.net",
+    "multi_tenant_domain": "",
+    "cli_client_id": "6e5d9a35-39ca-4978-8fe3-5b84b0b8806a",
+    "proxy_nsg_config": {
+        "allowed_ips": [
+            "*"
+        ],
+        "allowed_service_tags": []
+    }
+}
+```
+
+**Example config.json for a deployment where `--auto_create_cli_app` is being used:**
+
+```json
+{
+    "tenant_id": "e6424a5f-2625-42a4-8d94-c9677a4d96fc",
+    "tenant_domain": "azurewebsites.net",
+    "multi_tenant_domain": "",
+    "proxy_nsg_config": {
+        "allowed_ips": [
+            "*"
+        ],
+        "allowed_service_tags": []
+    }
+}
+```
+Future updates can be made to this configuration via the OneFuzz CLI.
 
 ## Install the CLI
 
@@ -61,7 +99,7 @@ from the [Latest Release of OneFuzz](https://github.com/microsoft/onefuzz/releas
 
 If you're using the SDK, install via:
 
-```
+```console
 pip install ./onefuzz*.whl
 ```
 

src/deployment/deploy.py

@@ -398,36 +398,41 @@ def setup_rbac(self) -> None:
 
         (password_id, password) = self.create_password(app["id"])
 
-        cli_app = get_application(
-            app_id=uuid.UUID(self.cli_app_id),
-            subscription_id=self.get_subscription_id(),
-        )
+        try:
+            cli_app = get_application(
+                app_id=uuid.UUID(self.cli_app_id),
+                subscription_id=self.get_subscription_id(),
+            )
+        except Exception as err:
+            cli_app = None
+            logger.info(
+                "Could not find the default CLI application under the current "
+                "subscription."
+            )
+            logger.debug(f"Error finding CLI application due to: {err}")
+        if self.auto_create_cli_app:
+            logger.info("auto_create_cli_app specified, creating a new CLI application")
+            app_info = register_application(
+                "onefuzz-cli",
+                self.application_name,
+                OnefuzzAppRole.CliClient,
+                self.get_subscription_id(),
+            )
 
-        if not cli_app:
-            if self.auto_create_cli_app:
-                logger.info(
-                    "Could not find the default CLI application under the current "
-                    "subscription and auto_create specified, creating a new one"
-                )
-                app_info = register_application(
-                    "onefuzz-cli",
-                    self.application_name,
-                    OnefuzzAppRole.CliClient,
-                    self.get_subscription_id(),
+            try:
+                cli_app = get_application(
+                    app_id=app_info.client_id,
+                    subscription_id=self.get_subscription_id(),
                 )
-
-                self.cli_config = {
-                    "client_id": app_info.client_id,
-                    "authority": self.authority,
-                }
-            else:
+                self.cli_app_id = str(app_info.client_id)
+                logger.info(f"New CLI app created - cli_app_id : {self.cli_app_id}")
+            except Exception as err:
                 logger.error(
-                    "error deploying. could not find specified CLI app registrion."
-                    "use flag --auto_create_cli_app to automatically create CLI registration"
-                    "or specify a correct app id with --cli_app_id."
+                    f"Unable to determine new 'cli_app_id' for new app registration: {err} "
                 )
                 sys.exit(1)
-        else:
+
+        if cli_app:
             onefuzz_cli_app = cli_app
             authorize_application(uuid.UUID(onefuzz_cli_app["appId"]), app["appId"])
 
@@ -467,8 +472,15 @@ def setup_rbac(self) -> None:
                 OnefuzzAppRole.ManagedNode,
             )
 
-        self.results["client_id"] = app["appId"]
-        self.results["client_secret"] = password
+            self.results["client_id"] = app["appId"]
+            self.results["client_secret"] = password
+        else:
+            logger.error(
+                "error deploying. could not find specified CLI app registrion."
+                "use flag --auto_create_cli_app to automatically create CLI registration"
+                "or specify a correct app id with --cli_app_id."
+            )
+            sys.exit(1)
 
     def update_existing_app_registration(
         self, app: Dict[str, Any], app_roles: List[Dict[str, Any]]
@@ -777,7 +789,10 @@ def parse_config(self) -> None:
                 config_template = json.load(template_handle)
 
             try:
-                config = Config(config_template)
+                if self.auto_create_cli_app:
+                    config = Config(config_template, True)
+                else:
+                    config = Config(config_template)
                 self.rules = parse_rules(config)
 
                 ## Values provided via the CLI will override what's in the config.json
@@ -789,8 +804,9 @@ def parse_config(self) -> None:
                     self.tenant_domain = config.tenant_domain
                 if self.multi_tenant_domain == "":
                     self.multi_tenant_domain = config.multi_tenant_domain
-                if self.cli_app_id == "":
-                    self.cli_app_id = config.cli_client_id
+                if not self.cli_app_id:
+                    if not self.auto_create_cli_app:
+                        self.cli_app_id = config.cli_client_id
 
             except Exception as ex:
                 logging.info(

src/deployment/deploylib/configuration.py

@@ -56,7 +56,8 @@ class Config:
     allowed_ips: List[str]
     allowed_service_tags: List[str]
 
-    def __init__(self, config: Any):
+    def __init__(self, config: Any, new_app: bool = False):
+        self.new_app_id = new_app
         self.parse_nsg_json(config)
         self.parse_endpoint_json(config)
 
@@ -113,25 +114,28 @@ def parse_nsg_json(self, config: Any) -> None:
         self.allowed_service_tags = proxy_config["allowed_service_tags"]
 
     def parse_endpoint_json(self, config: Any) -> None:
-        if "cli_client_id" not in config:
-            raise Exception(
-                "CLI client_id not provided as valid key. Please Provide Valid Config."
-            )
-
-        if (
-            not isinstance(config["cli_client_id"], str)
-            or config["cli_client_id"] == ""
-        ):
-            raise Exception(
-                "client_id is not a string. Please provide valid client_id."
-            )
-
-        try:
-            UUID(config["cli_client_id"])
-        except ValueError:
-            raise Exception(
-                "client_id is not a valid UUID. Please provide valid client_id."
-            )
+        if not self.new_app_id:
+            if "cli_client_id" not in config:
+                raise Exception(
+                    "CLI client_id not provided as valid key. Please Provide Valid Config."
+                )
+
+            if (
+                not isinstance(config["cli_client_id"], str)
+                or config["cli_client_id"] == ""
+            ):
+                raise Exception(
+                    "client_id is not a string. Please provide valid client_id."
+                )
+
+            try:
+                UUID(config["cli_client_id"])
+            except ValueError:
+                raise Exception(
+                    "client_id is not a valid UUID. Please provide valid client_id."
+                )
+
+            self.cli_client_id = config["cli_client_id"]
 
         if "tenant_id" not in config:
             raise Exception(
@@ -166,7 +170,6 @@ def parse_endpoint_json(self, config: Any) -> None:
                 "multi_tenant_domain is not a string. Please provide valid multi_tenant_domain. If the instance is not multi-tenant, please provide an empty string."
             )
 
-        self.cli_client_id = config["cli_client_id"]
         self.tenant_id = config["tenant_id"]
         self.tenant_domain = config["tenant_domain"]
         self.multi_tenant_domain = config["multi_tenant_domain"]