Merge branch 'marrobi/issue3796' of github.com:marrobi/AzureTRE into …

…marrobi/issue3796

CHANGELOG.md

@@ -9,6 +9,8 @@ FEATURES:
 ENHANCEMENTS:
 * Security updates aligning to Dependabot, MS Defender for Cloud and Synk ([#3796](https://github.com/microsoft/AzureTRE/issues/3796))
 BUG FIXES:
+* Fix issue where updates fail as read only is not configured consistently on schema fields ([#3691](https://github.com/microsoft/AzureTRE/issues/3691))
+* When geting avaialble address spaces allow those allocated to deleted workspaces to be reassigned ([#3691](https://github.com/microsoft/AzureTRE/issues/3691))
 * Update Python packages, and fix breaking changes ([#3764](https://github.com/microsoft/AzureTRE/issues/3764))
 * Enabling support for more than 20 users/groups in Workspace API ([#3759](https://github.com/microsoft/AzureTRE/pull/3759  ))
 * Airlock Import Review workspace uses dedicated DNS zone to prevent conflict with core ([#3767](https://github.com/microsoft/AzureTRE/pull/3767))

airlock_processor/BlobCreatedTrigger/__init__.py

@@ -32,10 +32,11 @@ def main(msg: func.ServiceBusMessage,
             logging.error("environment variable 'ENABLE_MALWARE_SCANNING' does not exists. Cannot continue.")
             raise
 
-        if enable_malware_scanning:
+        if enable_malware_scanning and constants.STORAGE_ACCOUNT_NAME_IMPORT_INPROGRESS in topic:
             # If malware scanning is enabled, the fact that the blob was created can be dismissed.
             # It will be consumed by the malware scanning service
             logging.info('Malware scanning is enabled. no action to perform.')
+            send_delete_event(dataDeletionEvent, json_body, request_id)
             return
         else:
             logging.info('Malware scanning is disabled. Completing the submitted stage (moving to in_review).')
@@ -66,6 +67,10 @@ def main(msg: func.ServiceBusMessage,
             event_time=datetime.datetime.utcnow(),
             data_version=constants.STEP_RESULT_EVENT_DATA_VERSION))
 
+    send_delete_event(dataDeletionEvent, json_body, request_id)
+
+
+def send_delete_event(dataDeletionEvent: func.Out[func.EventGridOutputEvent], json_body, request_id):
     # check blob metadata to find the blob it was copied from
     blob_client = get_blob_client_from_blob_info(
         *get_blob_info_from_topic_and_subject(topic=json_body["topic"], subject=json_body["subject"]))

airlock_processor/ScanResultTrigger/__init__.py

@@ -14,7 +14,7 @@ def main(msg: func.ServiceBusMessage,
 
     logging.info("Python ServiceBus queue trigger processed message - Malware scan result arrived!")
     body = msg.get_body().decode('utf-8')
-    logging.info('Python ServiceBus queue trigger processed message: %s', body)
+    logging.info(f'Python ServiceBus queue trigger processed message: {body}')
     status_message = None
 
     try:
@@ -34,7 +34,7 @@ def main(msg: func.ServiceBusMessage,
     try:
         json_body = json.loads(body)
         blob_uri = json_body["data"]["blobUri"]
-        verdict = json_body["data"]["verdict"]
+        verdict = json_body["data"]["scanResultType"]
     except KeyError as e:
         logging.error("body was not as expected {}", e)
         raise e
@@ -46,10 +46,10 @@ def main(msg: func.ServiceBusMessage,
     # Otherwise, move the request to the blocked stage
     completed_step = constants.STAGE_SUBMITTED
     if verdict == constants.NO_THREATS:
-        logging.info('No malware were found in request id %s, moving to %s stage', request_id, constants.STAGE_IN_REVIEW)
+        logging.info(f'No malware were found in request id {request_id}, moving to {constants.STAGE_IN_REVIEW} stage')
         new_status = constants.STAGE_IN_REVIEW
     else:
-        logging.info('Malware was found in request id %s, moving to %s stage', request_id, constants.STAGE_BLOCKING_INPROGRESS)
+        logging.info(f'Malware was found in request id {request_id}, moving to {constants.STAGE_BLOCKING_INPROGRESS} stage')
         new_status = constants.STAGE_BLOCKING_INPROGRESS
         status_message = verdict
 

airlock_processor/_version.py

@@ -1 +1 @@
-__version__ = "0.7.4"
+__version__ = "0.7.0"

api_app/api/routes/workspaces.py

@@ -7,7 +7,7 @@
 
 from api.dependencies.database import get_repository
 from api.dependencies.workspaces import get_operation_by_id_from_path, get_workspace_by_id_from_path, get_deployed_workspace_by_id_from_path, get_deployed_workspace_service_by_id_from_path, get_workspace_service_by_id_from_path, get_user_resource_by_id_from_path
-from db.errors import MajorVersionUpdateDenied, TargetTemplateVersionDoesNotExist, UserNotAuthorizedToUseTemplate, VersionDowngradeDenied
+from db.errors import InvalidInput, MajorVersionUpdateDenied, TargetTemplateVersionDoesNotExist, UserNotAuthorizedToUseTemplate, VersionDowngradeDenied
 from db.repositories.operations import OperationRepository
 from db.repositories.resource_templates import ResourceTemplateRepository
 from db.repositories.resources_history import ResourceHistoryRepository
@@ -107,6 +107,8 @@ async def create_workspace(workspace_create: WorkspaceInCreate, response: Respon
     except UserNotAuthorizedToUseTemplate as e:
         logging.exception("User not authorized to use template")
         raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e))
+    except InvalidInput as e:
+        raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e))
 
     operation = await save_and_deploy_resource(
         resource=workspace,

api_app/db/repositories/workspaces.py

@@ -140,11 +140,11 @@ async def validate_address_space(self, address_space):
         if (address_space is None):
             raise InvalidInput("Missing 'address_space' from properties.")
 
-        allocated_networks = [x.properties["address_space"] for x in await self.get_workspaces()]
+        allocated_networks = [x.properties["address_space"] for x in await self.get_active_workspaces()]
         return is_network_available(allocated_networks, address_space)
 
     async def get_new_address_space(self, cidr_netmask: int = 24):
-        workspaces = await self.get_workspaces()
+        workspaces = await self.get_active_workspaces()
         networks = [[x.properties.get("address_space")] for x in workspaces]
         networks = networks + [x.properties.get("address_spaces", []) for x in workspaces]
         networks = [i for s in networks for i in s if i is not None]

api_app/services/schema_service.py

@@ -50,9 +50,17 @@ def enrich_template(original_template, extra_properties, is_update: bool = False
     # this will help the UI render fields appropriately and know what it can send in a PATCH
     if is_update:
         for prop in template["properties"].values():
-            if "updateable" not in prop.keys() or prop["updateable"] is not True:
+            if not prop.get("updateable", False):
                 prop["readOnly"] = True
 
+        if "allOf" in template:
+            for conditional_property in template["allOf"]:
+                for condition in ["then", "else"]:
+                    if condition in conditional_property and "properties" in conditional_property[condition]:
+                        for prop in conditional_property[condition]["properties"].values():
+                            if not prop.get("updateable", False):
+                                prop["readOnly"] = True
+
     # if there is an 'allOf' property which is empty, the validator fails - so remove the key
     if "allOf" in template and template["allOf"] is None:
         template.pop("allOf")

api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py

@@ -209,29 +209,29 @@ async def test_get_address_space_based_on_size_with_custom_address_space_and_mis
 
 
 @pytest.mark.asyncio
-@patch('db.repositories.workspaces.WorkspaceRepository.get_workspaces')
+@patch('db.repositories.workspaces.WorkspaceRepository.get_active_workspaces')
 @patch('core.config.RESOURCE_LOCATION', "useast2")
 @patch('core.config.TRE_ID', "9876")
 @patch('core.config.CORE_ADDRESS_SPACE', "10.1.0.0/22")
 @patch('core.config.TRE_ADDRESS_SPACE', "10.0.0.0/12")
-async def test_get_address_space_based_on_size_with_address_space_only(get_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
+async def test_get_address_space_based_on_size_with_address_space_only(get_active_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
     workspace_with_address_space = copy.deepcopy(workspace)
     workspace_with_address_space.properties["address_space"] = "10.1.4.0/24"
 
-    get_workspaces_mock.return_value = [workspace_with_address_space]
+    get_active_workspaces_mock.return_value = [workspace_with_address_space]
     workspace_to_create = basic_workspace_request
     address_space = await workspace_repo.get_address_space_based_on_size(workspace_to_create.properties)
 
     assert "10.1.5.0/24" == address_space
 
 
 @pytest.mark.asyncio
-@patch('db.repositories.workspaces.WorkspaceRepository.get_workspaces')
+@patch('db.repositories.workspaces.WorkspaceRepository.get_active_workspaces')
 @patch('core.config.RESOURCE_LOCATION', "useast2")
 @patch('core.config.TRE_ID', "9876")
 @patch('core.config.CORE_ADDRESS_SPACE', "10.1.0.0/22")
 @patch('core.config.TRE_ADDRESS_SPACE', "10.0.0.0/12")
-async def test_get_address_space_based_on_size_with_address_space_and_address_spaces(get_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
+async def test_get_address_space_based_on_size_with_address_space_and_address_spaces(get_active_workspaces_mock, workspace_repo, basic_workspace_request, workspace):
     workspace_with_address_space = copy.deepcopy(workspace)
     workspace_with_address_space.properties["address_space"] = "10.1.4.0/24"
 
@@ -242,7 +242,7 @@ async def test_get_address_space_based_on_size_with_address_space_and_address_sp
     workspace_with_both.properties["address_spaces"] = ["10.1.7.0/24", "10.1.8.0/24"]
     workspace_with_both.properties["address_space"] = "10.1.7.0/24"
 
-    get_workspaces_mock.return_value = [workspace_with_address_space, workspace_with_address_spaces, workspace_with_both]
+    get_active_workspaces_mock.return_value = [workspace_with_address_space, workspace_with_address_spaces, workspace_with_both]
     workspace_to_create = basic_workspace_request
     address_space = await workspace_repo.get_address_space_based_on_size(workspace_to_create.properties)
 

config.sample.yaml

@@ -31,7 +31,7 @@ tre:
   core_app_service_plan_sku: P1v2
   resource_processor_vmss_sku: Standard_B2s
   enable_swagger: true
-  enable_airlock_malware_scanning: false
+  enable_airlock_malware_scanning: true
 
   # TODO: move to RP default with https://github.com/microsoft/AzureTRE/issues/2948
   workspace_app_service_plan_sku: P1v2

core/terraform/airlock/eventgrid_topics.tf

@@ -161,6 +161,36 @@ resource "azurerm_private_endpoint" "eg_data_deletion" {
   }
 }
 
+resource "azurerm_eventgrid_topic" "scan_result" {
+  count               = var.enable_malware_scanning ? 1 : 0
+  name                = local.scan_result_topic_name
+  location            = var.location
+  resource_group_name = var.resource_group_name
+  # This is mandatory for the scan result to be published since private networks are not supported yet
+  public_network_access_enabled = true
+
+  identity {
+    type = "SystemAssigned"
+  }
+
+  tags = merge(var.tre_core_tags, {
+    Publishers = "Airlock Processor;"
+  })
+
+  lifecycle { ignore_changes = [tags] }
+}
+
+resource "azurerm_role_assignment" "servicebus_sender_scan_result" {
+  count                = var.enable_malware_scanning ? 1 : 0
+  scope                = var.airlock_servicebus.id
+  role_definition_name = "Azure Service Bus Data Sender"
+  principal_id         = azurerm_eventgrid_topic.scan_result[0].identity[0].principal_id
+
+  depends_on = [
+    azurerm_eventgrid_topic.scan_result
+  ]
+}
+
 # System topic
 resource "azurerm_eventgrid_system_topic" "import_inprogress_blob_created" {
   name                   = local.import_inprogress_sys_topic_name
@@ -395,6 +425,23 @@ resource "azurerm_eventgrid_event_subscription" "data_deletion" {
   ]
 }
 
+resource "azurerm_eventgrid_event_subscription" "scan_result" {
+  count = var.enable_malware_scanning ? 1 : 0
+  name  = local.scan_result_eventgrid_subscription_name
+  scope = azurerm_eventgrid_topic.scan_result[0].id
+
+  service_bus_queue_endpoint_id = azurerm_servicebus_queue.scan_result.id
+
+  delivery_identity {
+    type = "SystemAssigned"
+  }
+
+  depends_on = [
+    azurerm_eventgrid_topic.scan_result,
+    azurerm_role_assignment.servicebus_sender_scan_result
+  ]
+}
+
 resource "azurerm_eventgrid_event_subscription" "import_inprogress_blob_created" {
   name  = local.import_inprogress_eventgrid_subscription_name
   scope = azurerm_storage_account.sa_import_in_progress.id

core/terraform/airlock/locals.tf

@@ -23,6 +23,7 @@ locals {
   status_changed_topic_name = "evgt-airlock-status-changed-${local.topic_name_suffix}"
   notification_topic_name   = "evgt-airlock-notification-${local.topic_name_suffix}"
   data_deletion_topic_name  = "evgt-airlock-data-deletion-${local.topic_name_suffix}"
+  scan_result_topic_name    = "evgt-airlock-scan-result-${local.topic_name_suffix}"
 
   step_result_queue_name    = "airlock-step-result"
   status_changed_queue_name = "airlock-status-changed"
@@ -35,6 +36,7 @@ locals {
   step_result_eventgrid_subscription_name       = "evgs-airlock-update-status"
   status_changed_eventgrid_subscription_name    = "evgs-airlock-status-changed"
   data_deletion_eventgrid_subscription_name     = "evgs-airlock-data-deletion"
+  scan_result_eventgrid_subscription_name       = "evgs-airlock-scan-result"
   import_inprogress_eventgrid_subscription_name = "evgs-airlock-import-in-progress-blob-created"
   import_rejected_eventgrid_subscription_name   = "evgs-airlock-import-rejected-blob-created"
   import_blocked_eventgrid_subscription_name    = "evgs-airlock-import-blocked-blob-created"

core/terraform/airlock/main.tf

@@ -5,6 +5,10 @@ terraform {
       source  = "hashicorp/azurerm"
       version = ">= 3.16"
     }
+    azapi = {
+      source  = "Azure/azapi"
+      version = ">= 1.9.0"
+    }
     local = {
       source  = "hashicorp/local"
       version = ">= 2.2"

core/terraform/airlock/storage_accounts.tf

@@ -111,6 +111,32 @@ resource "azurerm_storage_account" "sa_import_in_progress" {
   lifecycle { ignore_changes = [tags] }
 }
 
+
+# Enable Airlock Malware Scanning on Core TRE
+resource "azapi_resource_action" "enable_defender_for_storage" {
+  count       = var.enable_malware_scanning ? 1 : 0
+  type        = "Microsoft.Security/defenderForStorageSettings@2022-12-01-preview"
+  resource_id = "${azurerm_storage_account.sa_import_in_progress.id}/providers/Microsoft.Security/defenderForStorageSettings/current"
+  method      = "PUT"
+
+  body = jsonencode({
+    properties = {
+      isEnabled = true
+      malwareScanning = {
+        onUpload = {
+          isEnabled     = true
+          capGBPerMonth = 5000
+        },
+        scanResultsEventGridTopicResourceId = azurerm_eventgrid_topic.scan_result[0].id
+      }
+      sensitiveDataDiscovery = {
+        isEnabled = false
+      }
+      overrideSubscriptionLevelSettings = true
+    }
+  })
+}
+
 resource "azurerm_private_endpoint" "stg_import_inprogress_pe" {
   name                = "pe-stg-import-inprogress-blob-${var.tre_id}"
   location            = var.location

core/terraform/main.tf

@@ -17,11 +17,19 @@ terraform {
       source  = "hashicorp/http"
       version = "~> 3.2.0"
     }
+    azapi = {
+      source  = "Azure/azapi"
+      version = "~> 1.9.0"
+    }
   }
 
   backend "azurerm" {}
 }
 
+provider "azapi" {
+  use_msi = var.arm_use_msi
+}
+
 provider "azurerm" {
   features {
     key_vault {

core/terraform/variables.tf

@@ -130,6 +130,13 @@ variable "arm_environment" {
   description = "Used as an environment variable in the VMSS to set the Azure cloud for Terraform"
 }
 
+variable "arm_use_msi" {
+  type        = bool
+  default     = false
+  description = "Used as an environment variable to determine if Terraform should use a managed identity"
+}
+
+
 variable "stateful_resources_locked" {
   type        = bool
   default     = true
@@ -161,9 +168,6 @@ variable "public_deployment_ip_address" {
   default     = ""
 }
 
-# Important note: it is NOT enough to simply enable the malware scanning on. Further, manual, steps are required
-# in order to actually set up the scanner. Setting this property to True without supplying a scanner will result
-# in airlock requests being stuck in the in-progress stage.
 variable "enable_airlock_malware_scanning" {
   type        = bool
   default     = false

core/version.txt

@@ -1 +1 @@
-__version__ = "0.8.9"
+__version__ = "0.9.0"

docs/azure-tre-overview/airlock.md

@@ -66,7 +66,8 @@ This storage location is external for import (`stalimex`) or internal for export
 The user will be able to upload a file to the provided storage location, using any tool of their preference: [Azure Storage Explorer](https://azure.microsoft.com/en-us/features/storage-explorer/) or [AzCopy](https://docs.microsoft.com/en-us/azure/storage/common/storage-use-azcopy-v10) which is a command line tool.
 
 The user Submits the request (TRE API call) starting the data movement (to the `stalimip` - import in-progress or `stalexip` - export in-progress). The airlock request is now in state **Submitted**.
-If enabled, the Security Scanning is started. In the case that security flaws are found, the request state becomes **Blocking In-progress** while the data is moved to blocked storage  (either import blocked `stalimblocked` or export blocked `stalexblocked`). In this case, the request is finalized with the state **Blocked By Scan**.
+If enabled, the Malware Scanning is started. The scan is done using Microsoft Defender for Storage, which is described in details [here](https://learn.microsoft.com/en-us/azure/defender-for-cloud/defender-for-storage-introduction).
+In the case that security flaws are found, the request state becomes **Blocking In-progress** while the data is moved to blocked storage  (either import blocked `stalimblocked` or export blocked `stalexblocked`). In this case, the request is finalized with the state **Blocked By Scan**.
 If the Security Scanning does not identify any security flaws, the request state becomes **In-Review**. Simultaneously, a notification is sent to the Airlock Manager user. The user needs to ask for the container URL using the TRE API (SAS token + URL with READ permission).
 
 > The Security Scanning can be disabled, changing the request state from **Submitted** straight to **In-Review**.