Merge pull request #255 from microsoft/sfi-code-fix

feat: Identity based Authentication implemented & Sfi code QL fixes for Comp Analysis

Author: Prajwal-Microsoft

Deployment/bicep/modules/azurecognitiveservice.bicep

@@ -15,6 +15,7 @@ resource cognitiveService 'Microsoft.CognitiveServices/accounts@2022-03-01' = {
   }
   kind: 'FormRecognizer'
   properties: {
+    customSubDomainName: cognitiveServiceName
   }
 }
 

Deployment/bicep/modules/azureopenaiservice.bicep

@@ -12,6 +12,7 @@ resource openAIService 'Microsoft.CognitiveServices/accounts@2023-05-01' = {
     name: 'S0'
   }
   properties: {
+    customSubDomainName: openAIServiceName
     // Add any specific properties if needed
   }
 }

Deployment/bicep/modules/azuresearch.bicep

@@ -13,6 +13,7 @@ resource searchService 'Microsoft.Search/searchServices@2023-11-01' = {
   properties: {
     replicaCount: 1
     partitionCount: 1
+    disableLocalAuth: true
   }
 }
 

Deployment/scripts/deployAzureResources.ps1

@@ -180,7 +180,6 @@ class DeploymentResult {
     [string]$StorageAccountConnectionString
     [string]$AzSearchServiceName
     [string]$AzSearchServicEndpoint
-    [string]$AzSearchAdminKey
     [string]$LogicAppDocumentProcessWatcherName
     [string]$LogicAppBenchmarkProcessWatcherName
     [string]$LogicAppGapAnalysisProcessWatcherName
@@ -190,7 +189,6 @@ class DeploymentResult {
     [string]$AksName
     [string]$ContainerRegistryName
     [string]$AzCognitiveServiceName
-    [string]$AzCognitiveServiceKey
     [string]$AzCognitiveServiceEndpoint
     [string]$AzOpenAiServiceName
     [string]$AzGPT4oModelName
@@ -200,7 +198,6 @@ class DeploymentResult {
     [string]$AzGPTEmbeddingModelName
     [string]$AzGPTEmbeddingModelId
     [string]$AzOpenAiServiceEndpoint
-    [string]$AzOpenAiServiceKey
     [string]$AzCosmosDBName
     [string]$AzCosmosDBConnectionString
 
@@ -213,7 +210,6 @@ class DeploymentResult {
         # Azure Search
         $this.AzSearchServiceName = ""
         $this.AzSearchServicEndpoint = ""
-        $this.AzSearchAdminKey = ""
         # Logic Apps
         $this.LogicAppDocumentProcessWatcherName = ""
         $this.LogicAppBenchmarkProcessWatcherName = ""
@@ -228,11 +224,9 @@ class DeploymentResult {
         # Cognitive Service - Azure AI Intelligence Document Service
         $this.AzCognitiveServiceName = ""
         $this.AzCognitiveServiceEndpoint = ""
-        $this.AzCognitiveServiceKey = ""
         # Open AI Service
         $this.AzOpenAiServiceName = ""
         $this.AzOpenAiServiceEndpoint = ""
-        $this.AzOpenAiServiceKey = ""
         # Model - GPT4o
         $this.AzGPT4oModelName = ""
         $this.AzGPT4oModelId = ""
@@ -341,12 +335,6 @@ try {
     $deploymentResult.AzLogicAppGapAnalysisProcessWatcherUrl = Get-LogicAppTriggerUrl -subscriptionId $subscriptionID -resourceGroupName $deploymentResult.ResourceGroupName -logicAppName $deploymentResult.LogicAppGapAnalysisProcessWatcherName -triggerName $triggerName
     # Get MongoDB connection string
     $deploymentResult.AzCosmosDBConnectionString = az cosmosdb keys list --name $deploymentResult.AzCosmosDBName --resource-group $deploymentResult.ResourceGroupName --type connection-strings --query "connectionStrings[0].connectionString" -o tsv
-    # Get Azure Cognitive Service API Key
-    $deploymentResult.AzCognitiveServiceKey = az cognitiveservices account keys list --name $deploymentResult.AzCognitiveServiceName --resource-group $deploymentResult.ResourceGroupName --query "key1" -o tsv
-    # Get Azure Search Service Admin Key
-    $deploymentResult.AzSearchAdminKey = az search admin-key show --service-name $deploymentResult.AzSearchServiceName --resource-group $deploymentResult.ResourceGroupName --query "primaryKey" -o tsv
-    # Get Azure Open AI Service API Key
-    $deploymentResult.AzOpenAiServiceKey = az cognitiveservices account keys list --name $deploymentResult.AzOpenAiServiceName --resource-group $deploymentResult.ResourceGroupName --query "key1" -o tsv
 
     ######################################################################################################################
     # Step 3 : Update App Configuration files with Secrets and information for AI Service and Kernel Memory Service.
@@ -358,7 +346,6 @@ try {
 
     $aiServicePlaceholders = @{
         '{{ azureopenaiendpoint }}' = $deploymentResult.AzOpenAiServiceEndpoint
-        '{{ azureopenaiapikey }}' = $deploymentResult.AzOpenAiServiceKey
         '{{ azuregpt4omodelname }}' = $deploymentResult.AzGPT4oModelName
         '{{ azuregpt4omodelId }}' = $deploymentResult.AzGPT4oModelId
         '{{ gpt4-32Kembeddingmodelname }}' = $deploymentResult.AzGPTEmbeddingModelName
@@ -385,12 +372,9 @@ try {
         '{{ blobstorageName }}' = $deploymentResult.StorageAccountName
         '{{ blobstorageconnectionstring }}' = $deploymentResult.StorageAccountConnectionString
         '{{ azuresearchendpoint }}' = $deploymentResult.AzSearchServicEndpoint
-        '{{ azuresearchadminkey }}' = $deploymentResult.AzSearchAdminKey
         '{{ azureopenaiendpoint }}' = $deploymentResult.AzOpenAiServiceEndpoint
-        '{{ azureopenaiapikey }}' = $deploymentResult.AzOpenAiServiceKey
         '{{ azuregpt4omodelname }}' = $deploymentResult.AzGPT4oModelName
         '{{ embeddingmodelname }}' = $deploymentResult.AzGPTEmbeddingModelName
-        '{{ azurecognitiveservicapikey }}' = $deploymentResult.AzCognitiveServiceKey
         '{{ azurecognitiveserviceendpoint }}' = $deploymentResult.AzCognitiveServiceEndpoint
     }
 
@@ -515,12 +499,30 @@ try {
     $systemAssignedIdentity = $(az vmss identity assign --resource-group $vmssResourceGroupName --name $vmssName --query systemAssignedIdentity --output tsv)
 
     # Assign the role for aks system assigned managed identity to Azure blob Storage Data contributor role with the scope of the storage account
+    Write-Host "Assign the role for aks system assigned managed identity to Azure blob Storage Data contributor role" -ForegroundColor Green
     az role assignment create --role "Storage Blob Data Contributor" --assignee $systemAssignedIdentity --scope "/subscriptions/$subscriptionID/resourceGroups/$($deploymentResult.ResourceGroupName)/providers/Microsoft.Storage/storageAccounts/$($deploymentResult.StorageAccountName)"
 
     # Assigne the role for aks system assigned managed identity to Azure queue data contributor role with the scope of the storage account
+    Write-Host "Assign the role for aks system assigned managed identity to Azure queue data contributor role" -ForegroundColor Green
     az role assignment create --role "Storage Queue Data Contributor" --assignee $systemAssignedIdentity --scope "/subscriptions/$subscriptionID/resourceGroups/$($deploymentResult.ResourceGroupName)/providers/Microsoft.Storage/storageAccounts/$($deploymentResult.StorageAccountName)"
 
+    # Assign the role for aks system assigned managed identity to Azure Queue Data Contributor role with the scope of Storage Account
+    Write-Host "Assign the role for aks system assigned managed identity to App Cognitive Services OpenAI User role" -ForegroundColor Green
+    az role assignment create --assignee $systemAssignedIdentity --role "Cognitive Services OpenAI User" --scope "/subscriptions/$subscriptionID/resourceGroups/$($deploymentResult.ResourceGroupName)/providers/Microsoft.CognitiveServices/accounts/$($deploymentResult.AzOpenAiServiceName)"
 
+    # Assign the role for aks system assigned managed identity to Azure Queue Data Contributor role with the scope of Storage Account
+    Write-Host "Assign the role for aks system assigned managed identity to App Search Index Data Contributor role" -ForegroundColor Green
+    az role assignment create --assignee $systemAssignedIdentity --role "Search Index Data Contributor" --scope "/subscriptions/$subscriptionID/resourceGroups/$($deploymentResult.ResourceGroupName)/providers/Microsoft.Search/searchServices/$($deploymentResult.AzSearchServiceName)"
+    
+    # Assign the role for aks system assigned managed identity to Azure Queue Data Contributor role with the scope of Storage Account
+    Write-Host "Assign the role for aks system assigned managed identity to App Search Service Contributor role" -ForegroundColor Green
+    az role assignment create --assignee $systemAssignedIdentity --role "Search Service Contributor" --scope "/subscriptions/$subscriptionID/resourceGroups/$($deploymentResult.ResourceGroupName)/providers/Microsoft.Search/searchServices/$($deploymentResult.AzSearchServiceName)"
+
+    # Assign the role for aks system assigned managed identity to Azure Queue Data Contributor role with the scope of Storage Account
+    Write-Host "Assign the role for aks system assigned managed identity to App Cognitive Services User role" -ForegroundColor Green
+    
+    az role assignment create --assignee $systemAssignedIdentity --role "Cognitive Services User" --scope "/subscriptions/$subscriptionID/resourceGroups/$($deploymentResult.ResourceGroupName)/providers/Microsoft.CognitiveServices/accounts/$($deploymentResult.AzCognitiveServiceName)"
+    
     # Update aks nodepools to updated new role
     try {
         Write-Host "Upgrading node pools..." -ForegroundColor Cyan

Services/appconfig/aiservice/appsettings.dev.json.template

@@ -9,7 +9,6 @@
       "ModelName": "{{ azuregpt4omodelname }}",
       "ModelId": "{{ azuregpt4omodelId }}",
       "EndPoint": "{{ azureopenaiendpoint }}",
-      "Key": "{{ azureopenaiapikey }}",
       "EmbeddingModelName": "{{ gpt4-32Kembeddingmodelname }}"
     },
     "ConnectionStrings": {

Services/appconfig/kernelmemory/appsettings.development.json.template

@@ -124,9 +124,8 @@
         "VirtualHost": "/"
       },
       "AzureAISearch": {
-        "Auth": "ApiKey",
+        "Auth": "AzureIdentity",
         "Endpoint": "{{ azuresearchendpoint }}",
-        "APIKey": "{{ azuresearchadminkey }}"
       },
       "Postgres": {
         "ConnectionString": "Host=localhost;Port=5432;Username=public;Password=",
@@ -155,18 +154,16 @@
       },
       "AzureOpenAIText": {
         "APIType": "ChatCompletion",
-        "Auth": "ApiKey",
+        "Auth": "AzureIdentity",
         "Endpoint": "{{ azureopenaiendpoint }}",
-        "APIKey": "{{ azureopenaiapikey }}",
         "Deployment": "{{ azuregpt4omodelname }}",
         "MaxRetries": 10,
         "MaxTokenTotal": 32768,
         "TextModelMaxTokenTotal": 32768 
       },
       "AzureOpenAIEmbedding": {
-        "Auth": "ApiKey",
+        "Auth": "AzureIdentity",
         "Endpoint": "{{ azureopenaiendpoint }}",
-        "APIKey": "{{ azureopenaiapikey }}",
         "Deployment": "{{ embeddingmodelname }}",
         "MaxTokenTotal": 8191,
         "APIType": "EmbeddingGeneration"
@@ -187,8 +184,7 @@
         "MaxTokenTotal": 4096
       },
       "AzureAIDocIntel": {
-        "Auth": "ApiKey",
-        "APIKey": "{{ azurecognitiveservicapikey }}",
+        "Auth": "AzureIdentity",
         "Endpoint": "{{ azurecognitiveserviceendpoint }}"
       }
     }

Services/src/esg-ai-doc-analysis/CFS.SK.Sustainability.AI.Host/ESRSHttpServiceMapper.cs

@@ -115,7 +115,7 @@ public static void UseESRSEndpoint(this WebApplication app)
                     var content = new StringContent(serializedParamJson, Encoding.UTF8, "application/json");
 
                     //Invoke Process Watcher
-                    var response = await appContext.httpClient.PostAsync(config["BenchmarkProcessing:processwatcherUrl"], content);
+                    var response = await appContext.httpClient.PostAsync(config["BenchmarkProcessing:processwatcherUrl"], content); // CodeQL [SM03781] We are reading this value from appsettings.json, this is not an user input
 
                     return Results.Accepted(locationUrl, benchmarkServiceRequest);
                 }

Services/src/esg-ai-doc-analysis/CFS.SK.Sustainability.AI/DocumentManager.cs

@@ -177,7 +177,7 @@ async public Task<DocumentServiceResult> RegisterDocumentFromFileUrl(RegisterDoc
             //Download file from URL then take only fileName from URL
             //the file location URL in the document will be mixed with SAS token to get it.
             //sample url - https://microsoft.seismic.com/app?ContentId=d6e9f9bb-70d4-4845-a2ad-dd25ecc343d6#/doccenter/a5266a70-9230-4c1e-a553-c5bddcb7a896/doc/%252Fdde0caec0e-9236-f21b-2991-5868e63d3984%252FdfYTZjNDRiZDMtMzEwZS1kNWZkLTNjOGEtNjliYWJjMjhmMmUw%252CPT0%253D%252CUGl0Y2ggRGVjaw%253D%253D%252Flffb13c1f1-d960-4bbe-8685-000afbf5a67f//?mode=view&parentPath=sessionStorage
-            
+            serviceRequest.FileUrl = FileNameValidator.ValidateAndReturnSafeFileName(serviceRequest.FileUrl);
             var fileBytes = await this.httpClient.GetByteArrayAsync(serviceRequest.FileUrl);
             //write ByteArray to Stream
             MemoryStream downloadedFileMemoryStream = new MemoryStream(fileBytes);

Services/src/esg-ai-doc-analysis/CFS.SK.Sustainability.AI/ESRSGapAnalysisManager.cs

@@ -25,7 +25,6 @@
 using Azure.Identity;
 using System.Text.RegularExpressions;
 
-
 namespace CFS.SK.Sustainability.AI
 {
     public class ESRSGapAnalysisManager : SemanticKernelLogicBase<ESRSGapAnalysisManager>
@@ -36,10 +35,6 @@ public class ESRSGapAnalysisManager : SemanticKernelLogicBase<ESRSGapAnalysisMan
         private readonly GapAnalysisJobRepository _gapAnalysisJobRepository;
         private readonly IConfiguration _config;
 
-        // Regex to validate file names (only allows letters, digits, dot, dash, underscore)
-        private static readonly Regex ValidFileNameRegex = 
-        new Regex(@"^[A-Za-z0-9._\-]+$", RegexOptions.Compiled);
-
         public ESRSGapAnalysisManager(ApplicationContext appContext,
                                         ILogger<ESRSGapAnalysisManager> logger,
                                         ESRSDisclosureRetriever esrsDisclosureRetriever,
@@ -209,7 +204,7 @@ public async Task<GapAnalysisJob> RegisterJob(GapAnalysisServiceRequest jobReque
             var fileName = $"GAPAnalysisReport-{sanitizedDisclosureNumber}-{jobId}";
 
             // validate file name
-            fileName = ValidateAndReturnSafeFileName(fileName);
+            fileName = FileNameValidator.ValidateAndReturnSafeFileName(fileName);
             var blobClient = blobContainerClient.GetBlobClient($"{fileName}.md");
             using (Stream stream = new MemoryStream(Encoding.UTF8.GetBytes(analysis_resultString)))
             {
@@ -223,7 +218,7 @@ public async Task<GapAnalysisJob> RegisterJob(GapAnalysisServiceRequest jobReque
             //Create BlobClient
             var htmlFileName = $"{fileName}.html";
             // validate html file name
-            htmlFileName = ValidateAndReturnSafeFileName(htmlFileName);
+            htmlFileName = FileNameValidator.ValidateAndReturnSafeFileName(htmlFileName);
             blobClient = blobContainerClient.GetBlobClient(htmlFileName);
             byte[] byteArray_html = MarkdownHtmlConverter.Convert(analysis_resultString);
 
@@ -241,7 +236,7 @@ public async Task<GapAnalysisJob> RegisterJob(GapAnalysisServiceRequest jobReque
             //Convert HTML to PDF
             //Need to extra work for convert html to pdf
             var pdfFileName = $"{fileName}.pdf";
-            pdfFileName = ValidateAndReturnSafeFileName(pdfFileName); // Validate pdffilename
+            pdfFileName = FileNameValidator.ValidateAndReturnSafeFileName(pdfFileName); // Validate pdffilename
             HtmlPdfConverter.Convert(htmlFileName, pdfFileName);
 
             //if Pdf file is exist then upload to the blob
@@ -271,7 +266,7 @@ public async Task<GapAnalysisJob> RegisterJob(GapAnalysisServiceRequest jobReque
             }
 
             var metaDataFileName = $"GAPAnalysisReport-{sanitizedDisclosureNumber}-{jobId}-meta.json";
-            metaDataFileName = ValidateAndReturnSafeFileName(metaDataFileName);// Validate metadatafilename
+            metaDataFileName = FileNameValidator.ValidateAndReturnSafeFileName(metaDataFileName);// Validate metadatafilename
             blobClient = blobContainerClient.GetBlobClient(metaDataFileName);
 
             using (Stream stream = new MemoryStream(Encoding.UTF8.GetBytes(JsonSerializer.Serialize(disclosureDescription))))
@@ -310,22 +305,5 @@ public async Task<GapAnalysisJob> RegisterJob(GapAnalysisServiceRequest jobReque
             return gapAnalysis_response;
             //return result.GetValue<string>();
         }
-       
-        // Validate simple file name to prevent path traversal attacks
-        // Returns the validated filename to help static analysis tools track sanitization
-        private static string ValidateAndReturnSafeFileName(string name)
-        {
-            if (string.IsNullOrWhiteSpace(name))
-                throw new ArgumentException("File name is empty or null.");
-
-            if (name.Contains("..") || name.Contains("/") || name.Contains("\\"))
-                throw new ArgumentException("Invalid file name (contains path components or traversal).");
-            
-            // Whitelist chars: letters, digits, dash, underscore, dot
-            if (!ValidFileNameRegex.IsMatch(name))
-                throw new ArgumentException("Invalid file name.");
-
-            return name;
-        }
     }
 }