Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added new feature to integrate azure services using managed identities #442

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

FreddyAyala
Copy link

This pull request introduces support for Managed Identities in the Azure Chat Solution Accelerator, enhancing security and simplifying secret management. Key changes include updates to documentation, infrastructure templates, and deployment configurations.

Documentation Updates:

  • Added a new section on using Managed Identities for the Azure Chat Solution Accelerator, detailing security advantages, services using Managed Identities, and deployment instructions. (docs/10.managed-identities.md)

Infrastructure Updates:

  • Introduced a new parameter disableLocalAuth in infra/main.bicep to toggle authentication by key, enforcing RBAC using Managed Identities. (infra/main.bicep) [1] [2]
  • Updated infra/main.json to include the disableLocalAuth parameter and its usage across various Azure services configurations. (infra/main.json) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]

Deployment Configuration:

  • Modified the deployment instructions to ensure the parameter disableLocalAuth is set to true for using Managed Identities and updated environment variables accordingly. (infra/main.json) [1] [2]

These changes collectively enhance the security posture of the Azure Chat deployment by leveraging Managed Identities, while also simplifying secret management and access control.

@FreddyAyala
Copy link
Author

FreddyAyala commented Oct 6, 2024

Hey there @thivy @davidxw ,
I've spent the last few days adding a new feature that enables the use of managed identities with the accelerator, except for Azure Speech, which I couldn't get to work reliably with managed identities and TypeScript.
As you might know, the FSI initiative is locking down tenants and enforcing the use of managed identities for internal tenants, particularly for CosmosDB. This change broke our solution, so I took the time to modify the infrastructure code and application services to support managed identities. This enhancement allows us to eliminate the risks associated with key sharing and deploy the solution in locked-down tenants.
Please take a look when you have a chance. I've conducted extensive testing to ensure everything works correctly.

@pyrox82
Copy link

pyrox82 commented Nov 12, 2024

Hi, any updates on this? I would really like to use this solution with managed identities.

Thank you in advance.

@davidxw davidxw requested a review from Copilot December 9, 2024 22:55

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 6 out of 13 changed files in this pull request and generated no suggestions.

Files not reviewed (7)
  • infra/main.bicep: Language not supported
  • infra/main.json: Language not supported
  • infra/resources.bicep: Language not supported
  • src/package.json: Language not supported
  • src/features/common/services/document-intelligence.ts: Evaluated as low risk
  • src/features/common/services/openai.ts: Evaluated as low risk
  • src/features/chat-page/chat-services/chat-document-service.ts: Evaluated as low risk
Comments skipped due to low confidence (2)

src/features/common/services/ai-search.ts:24

  • Rename function to follow camelCase convention: getCredential.
export const GetCredential = () => {

src/features/common/services/azure-storage.ts:20

  • [nitpick] The error message should specify the exact environment variable name, e.g., "AZURE_STORAGE_ACCOUNT_NAME or AZURE_STORAGE_ACCOUNT_KEY environment variable is not set".
throw new Error("Azure Storage Account not configured correctly, check environment variables.");
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants