Skip to content

Commit 4696754

Browse files
committed
Fix authentication and other azure deployment issues
1 parent 045d2d2 commit 4696754

File tree

6 files changed

+157
-17
lines changed

6 files changed

+157
-17
lines changed

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,8 +363,8 @@ Example role assignment commands (PowerShell friendly). Replace placeholders wit
363363
#$searchId = "/subscriptions/<sub>/resourceGroups/<rg>/providers/Microsoft.Search/searchServices/<search-name>"
364364
#$openaiId = "/subscriptions/<sub>/resourceGroups/<rg>/providers/Microsoft.CognitiveServices/accounts/<openai-name>"
365365
366-
az role assignment create --assignee-object-id $objectId --role "Storage Blob Data Reader" --scope $storageId
367-
az role assignment create --assignee-object-id $objectId --role "Search Index Data Reader" --scope $searchId
366+
az role assignment create --assignee-object-id $objectId --role "Storage Blob Data Contributor" --scope $storageId
367+
az role assignment create --assignee-object-id $objectId --role "Search Index Data Contributor" --scope $searchId
368368
az role assignment create --assignee-object-id $objectId --role "Cognitive Services OpenAI Contributor" --scope $openaiId
369369
```
370370

src/backend/core/azure_client_factory.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,18 @@ async def close(self) -> None:
6666
await self.document_intelligence_client.close()
6767
except Exception:
6868
logger.debug("Error closing document_intelligence_client", exc_info=True)
69+
# Try to close openai client if it exposes an async close or close method
70+
try:
71+
if hasattr(self.openai_client, 'aclose'):
72+
await getattr(self.openai_client, 'aclose')()
73+
elif hasattr(self.openai_client, 'close'):
74+
# some clients may expose sync close
75+
try:
76+
getattr(self.openai_client, 'close')()
77+
except Exception:
78+
logger.debug("Error calling openai_client.close()", exc_info=True)
79+
except Exception:
80+
logger.debug("Error closing openai client", exc_info=True)
6981

7082

7183
class ClientFactory:
@@ -143,6 +155,13 @@ async def get_session_clients(cls, session_id: str, auth_mode: AuthMode) -> Sess
143155
except Exception as e:
144156
logger.error("DefaultAzureCredential failed to acquire storage token", extra={"session_id": session_id, "error": str(e)} , exc_info=True)
145157

158+
# Diagnostic: attempt to acquire a token for Azure Cognitive Search to detect missing data-plane permissions
159+
try:
160+
search_token = await credential.get_token("https://search.azure.com/.default")
161+
logger.info("Acquired token for Azure Cognitive Search", extra={"session_id": session_id, "search_token_exp": getattr(search_token, 'expires_on', None)})
162+
except Exception as e:
163+
logger.warning("Could not acquire token for Azure Cognitive Search. This may indicate missing AAD scopes or permissions.", extra={"session_id": session_id, "error": str(e)})
164+
146165
# OpenAI bearer token provider
147166
token_provider = get_bearer_token_provider(
148167
credential,
@@ -241,6 +260,11 @@ async def get_session_clients(cls, session_id: str, auth_mode: AuthMode) -> Sess
241260
**client_kwargs,
242261
)
243262

263+
try:
264+
logger.debug("SearchIndexClient created", extra={"endpoint": config.search_service.endpoint, "credential_type": type(search_cred).__name__})
265+
except Exception:
266+
logger.debug("Could not log search index client creation", exc_info=True)
267+
244268
def get_search_client(index_name: str) -> SearchClient:
245269
return SearchClient(
246270
endpoint=config.search_service.endpoint,

src/backend/data_ingestion/process_file.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,8 +1393,36 @@ async def _index_documents(self, index_name, documents):
13931393
return
13941394

13951395
try:
1396+
# Log diagnostic info about the search client and credential
1397+
try:
1398+
cred = getattr(self.search_client, '_credential', None)
1399+
cred_type = type(cred).__name__ if cred is not None else 'None'
1400+
except Exception:
1401+
cred_type = 'unknown'
1402+
try:
1403+
endpoint = getattr(self.search_client, 'endpoint', None) or getattr(self.search_client, '_endpoint', None)
1404+
except Exception:
1405+
endpoint = None
1406+
logger.info("Uploading documents to search index", extra={"index": index_name, "document_count": len(documents), "search_credential_type": cred_type, "endpoint": endpoint})
1407+
13961408
await self.search_client.upload_documents(documents=documents)
13971409
print(f"Indexed {len(documents)} documents.")
1410+
except HttpResponseError as e:
1411+
# Detailed logging for HTTP errors from the search service
1412+
status = getattr(e, 'status_code', None) or getattr(e, 'status', None)
1413+
message = str(e)
1414+
logger.error("Search service returned HTTP error during upload", extra={"index": index_name, "status": status, "error": message})
1415+
# If index missing, attempt recreate and retry
1416+
missing = isinstance(e, ResourceNotFoundError) or (hasattr(e, 'message') and 'not found' in str(e).lower())
1417+
if missing:
1418+
desired = await self._build_index_schema(index_name)
1419+
await self._ensure_index_exists(index_name, desired)
1420+
await self.search_client.upload_documents(documents=documents)
1421+
print(f"Indexed {len(documents)} documents after recreating index.")
1422+
else:
1423+
# Surface forbidden and other errors with more detail
1424+
print(f"Error indexing documents (http): status={status} message={message}")
1425+
raise
13981426
except Exception as e:
13991427
# Retry once if index missing
14001428
missing = isinstance(e, ResourceNotFoundError) or (hasattr(e, 'message') and 'not found' in str(e).lower())

src/backend/handlers/citation_file_handler.py

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from typing import Dict, Any
55
from aiohttp import web
66
from azure.storage.blob.aio import ContainerClient, BlobServiceClient
7+
from azure.identity.aio import DefaultAzureCredential
78
from azure.storage.blob import generate_blob_sas, BlobSasPermissions
89
from azure.core.exceptions import AzureError, ResourceNotFoundError
910

@@ -333,17 +334,61 @@ async def _get_file_url(
333334
except Exception:
334335
logger.debug("Credential diagnostic check failed", extra={"request_id": request_id}, exc_info=True)
335336

336-
user_delegation_key = await blob_service_client.get_user_delegation_key(
337-
key_start_time=start_time, key_expiry_time=expiry_time
338-
)
339-
sas_token = generate_blob_sas(
340-
account_name=blob_client.account_name or "",
341-
container_name=container_client.container_name,
342-
blob_name=normalized_blob_name,
343-
user_delegation_key=user_delegation_key,
344-
permission=BlobSasPermissions(read=True),
345-
expiry=datetime.utcnow() + timedelta(minutes=self.sas_duration_minutes),
346-
)
337+
# If the existing blob_service_client is not AAD/Bearer-capable (for example,
338+
# constructed with an account key), the Storage service will reject a
339+
# get_user_delegation_key call. In that case, create a temporary
340+
# AAD-backed BlobServiceClient using DefaultAzureCredential to request
341+
# the user delegation key. This is deliberate and only used when
342+
# auth_mode == MANAGED_IDENTITY and the provided client cannot issue
343+
# a Bearer token. We log the behavior so it is visible in production.
344+
temp_cred = None
345+
temp_blob_service_client = None
346+
try:
347+
cred = getattr(blob_service_client, 'credential', None)
348+
needs_temp_aad = not (cred is not None and hasattr(cred, 'get_token'))
349+
350+
if needs_temp_aad:
351+
logger.info(
352+
"BlobServiceClient not AAD-capable; creating temporary AAD-backed client for user-delegation key",
353+
extra={"request_id": request_id, "blob_account": blob_client.account_name}
354+
)
355+
temp_cred = DefaultAzureCredential()
356+
# Use the same account URL as the provided service client
357+
account_url = getattr(blob_service_client, 'url', None)
358+
if not account_url:
359+
# Fallback to constructing from account name
360+
account_name = getattr(blob_client, 'account_name', None) or getattr(blob_service_client, 'account_name', None)
361+
account_url = f"https://{account_name}.blob.core.windows.net"
362+
363+
temp_blob_service_client = BlobServiceClient(account_url=account_url, credential=temp_cred)
364+
user_delegation_key = await temp_blob_service_client.get_user_delegation_key(
365+
key_start_time=start_time, key_expiry_time=expiry_time
366+
)
367+
else:
368+
user_delegation_key = await blob_service_client.get_user_delegation_key(
369+
key_start_time=start_time, key_expiry_time=expiry_time
370+
)
371+
372+
sas_token = generate_blob_sas(
373+
account_name=blob_client.account_name or "",
374+
container_name=container_client.container_name,
375+
blob_name=normalized_blob_name,
376+
user_delegation_key=user_delegation_key,
377+
permission=BlobSasPermissions(read=True),
378+
expiry=datetime.utcnow() + timedelta(minutes=self.sas_duration_minutes),
379+
)
380+
finally:
381+
# Close temporary clients/credentials if created to avoid leaks
382+
if temp_blob_service_client is not None:
383+
try:
384+
await temp_blob_service_client.close()
385+
except Exception:
386+
logger.debug("Failed closing temporary BlobServiceClient", extra={"request_id": request_id}, exc_info=True)
387+
if temp_cred is not None:
388+
try:
389+
await temp_cred.close()
390+
except Exception:
391+
logger.debug("Failed closing temporary DefaultAzureCredential", extra={"request_id": request_id}, exc_info=True)
347392
except Exception as ade:
348393
logger.error("Managed Identity auth selected but user-delegation key request failed", extra={"request_id": request_id, "error": str(ade)})
349394
# Surface error to caller (no silent fallback allowed)

src/backend/handlers/upload_handler.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,8 @@ async def handle_process_document(self, request):
277277
"""Handle document processing with enhanced logging and monitoring"""
278278
process_start = time.time()
279279
upload_id = None
280+
# Track clients that are created for this request and must be closed explicitly
281+
transient_clients = []
280282

281283
try:
282284
data = await request.json()
@@ -317,16 +319,21 @@ async def handle_process_document(self, request):
317319
if not self.clients:
318320
await self.initialize_clients()
319321
bundle_clients = self.clients
322+
transient_clients = []
320323
else:
321324
config = get_config()
322325
# map bundle to same keys used in self.clients
326+
# The SearchClient returned by bundle.get_search_client is request-scoped and should be closed
327+
search_client = bundle.get_search_client(config.search_service.index_name)
328+
transient_clients = [search_client]
329+
323330
bundle_clients = {
324331
'document': bundle.document_intelligence_client,
325332
'text_embedding': type('AOAIWrapper', (), {'embed': lambda self, input: bundle.openai_client.embeddings.create(input=input, model=config.azure_openai.embedding_deployment)})(),
326333
'image_embedding': type('AOAIWrapper', (), {'embed': lambda self, input: bundle.openai_client.embeddings.create(input=input, model=config.azure_openai.embedding_deployment)})(),
327334
'openai': bundle.openai_client,
328335
'instructor_openai': instructor.from_openai(bundle.openai_client),
329-
'search': bundle.get_search_client(config.search_service.index_name),
336+
'search': search_client,
330337
'search_index': bundle.search_index_client,
331338
'blob_service': bundle.blob_service_client,
332339
'blob_container': bundle.blob_service_client.get_container_client(config.storage.artifacts_container)
@@ -521,6 +528,41 @@ async def process_file(self, *args, **kwargs):
521528
"timestamp": datetime.datetime.now().isoformat()
522529
})
523530
print(f"Document processing error for {upload_id}: {e}")
531+
finally:
532+
# Close any clients in bundle_clients that are not the handler-level cached clients
533+
try:
534+
if bundle_clients:
535+
for key, client_obj in list(bundle_clients.items()):
536+
try:
537+
# Skip if this client is the same object as the handler-level cached client
538+
if getattr(self, 'clients', None) and self.clients.get(key) is client_obj:
539+
continue
540+
541+
if client_obj is None:
542+
continue
543+
544+
# Prefer async close
545+
if hasattr(client_obj, 'aclose') and callable(getattr(client_obj, 'aclose')):
546+
try:
547+
await client_obj.aclose()
548+
logger.debug("Closed transient async client", extra={"client_key": key})
549+
except Exception:
550+
logger.debug("Error closing client aclose()", extra={"client_key": key}, exc_info=True)
551+
elif hasattr(client_obj, 'close') and callable(getattr(client_obj, 'close')):
552+
try:
553+
result = client_obj.close()
554+
import asyncio as _asyncio
555+
if _asyncio.iscoroutine(result):
556+
await result
557+
logger.debug("Awaited async close() on client", extra={"client_key": key})
558+
else:
559+
logger.debug("Called sync close() on client", extra={"client_key": key})
560+
except Exception:
561+
logger.debug("Error calling client's close()", extra={"client_key": key}, exc_info=True)
562+
except Exception:
563+
logger.debug("Error while attempting to close a client", exc_info=True)
564+
except Exception:
565+
logger.debug("Error in final cleanup of clients", exc_info=True)
524566

525567
def update_processing_progress(self, upload_id, step, message, progress=None, details=None, increments=None):
526568
"""Update processing progress with detailed information"""

src/backend/middleware/__init__.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,17 +223,18 @@ async def __call__(self, request: web.Request, handler: Callable) -> web.Respons
223223
def _add_security_headers(self, response: web.Response) -> None:
224224
"""Add comprehensive security headers to response."""
225225
# Comprehensive CSP policy that allows necessary resources while maintaining security
226+
# Updated CSP: allow trusted CDNs (e.g. Monaco loader on cdn.jsdelivr.net) while keeping strict defaults.
226227
csp_policy = (
227228
"default-src 'self'; "
228-
"script-src 'self' 'unsafe-inline' 'unsafe-eval'; " # Allow inline scripts and eval for React/Vite
229+
"script-src 'self' 'unsafe-inline' 'unsafe-eval' https://cdn.jsdelivr.net https://unpkg.com; " # Allow Monaco loader from CDNs
229230
"style-src 'self' 'unsafe-inline' data:; " # Allow inline styles and data URLs for fonts
230231
"img-src 'self' *.blob.core.windows.net *.azureedge.net data: blob: https:; " # Allow Azure storage and common image sources
231232
"font-src 'self' data: https: *.gstatic.com *.googleapis.com; " # Allow web fonts
232-
"connect-src 'self' *.blob.core.windows.net *.azure.com *.azureedge.net *.openai.azure.com wss: ws:; " # Allow API connections
233+
"connect-src 'self' https://cdn.jsdelivr.net *.blob.core.windows.net *.azure.com *.azureedge.net *.openai.azure.com wss: ws: https:; " # Allow API connections and CDN module loads
233234
"media-src 'self' *.blob.core.windows.net data: blob:; " # Allow media files from storage
234235
"object-src 'none'; " # Block plugins for security
235236
"frame-src 'self'; " # Allow same-origin frames
236-
"worker-src 'self' blob:; " # Allow web workers
237+
"worker-src 'self' blob: https://cdn.jsdelivr.net; " # Allow web workers and worker scripts from CDN
237238
"child-src 'self' blob:; " # Allow child contexts
238239
"manifest-src 'self'; " # Allow web app manifest
239240
"form-action 'self'; " # Restrict form submissions

0 commit comments

Comments
 (0)