Allow authentication using RBAC instead of API Keys #427
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| AZURE_AUTH_TYPE= | ||
| AZURE_SEARCH_SERVICE= | ||
| AZURE_SEARCH_INDEX= | ||
| AZURE_SEARCH_KEY= | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,9 @@ resource account 'Microsoft.CognitiveServices/accounts@2023-05-01' = { | |
| publicNetworkAccess: publicNetworkAccess | ||
|
There was a problem hiding this comment. Can you do a PR to update this too? https://github.com/Azure/azure-dev/blob/main/templates/common/infra/bicep/core/ai/cognitiveservices.bicep There was a problem hiding this comment. What updates are required? Do you just mean to pull from main? |
||
| } | ||
| sku: sku | ||
| identity: { | ||
| type: 'SystemAssigned' | ||
| } | ||
| } | ||
|
|
||
| @batchSize(1) | ||
|
|
@@ -41,3 +44,5 @@ output id string = account.id | |
| output name string = account.name | ||
| output skuName string = account.sku.name | ||
| output key string = account.listKeys().key1 | ||
| output identityPrincipalId string = account.identity.principalId | ||
|
There was a problem hiding this comment. This should match the name of the other core files when they provide an id, which is just principalId |
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ param sku object = { | |
|
|
||
| param authOptions object = {} | ||
| param semanticSearch string = 'disabled' | ||
| param authType string = 'rbac' | ||
|
There was a problem hiding this comment. Can you do a PR to update this? https://github.com/Azure/azure-dev/blob/main/templates/common/infra/bicep/core/search/search-services.bicep There was a problem hiding this comment. What updates are required? Do you just mean to pull from main? |
||
|
|
||
| resource search 'Microsoft.Search/searchServices@2021-04-01-preview' = { | ||
| name: name | ||
|
|
@@ -17,8 +18,8 @@ resource search 'Microsoft.Search/searchServices@2021-04-01-preview' = { | |
| type: 'SystemAssigned' | ||
| } | ||
| properties: { | ||
| authOptions: authType == 'keys' ? authOptions : null | ||
| disableLocalAuth: authType == 'keys' ? false : true | ||
| disabledDataExfiltrationOptions: [] | ||
| encryptionWithCmk: { | ||
| enforcement: 'Unspecified' | ||
|
|
@@ -41,3 +42,4 @@ output endpoint string = 'https://${name}.search.windows.net/' | |
| output name string = search.name | ||
| output skuName string = sku.name | ||
| output adminKey string = search.listAdminKeys().primaryKey | ||
| output identityPrincipalId string = search.identity.principalId | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,12 @@ param appServicePlanName string = '' | |
| param backendServiceName string = '' | ||
| param resourceGroupName string = '' | ||
|
|
||
| @allowed([ | ||
| 'keys' | ||
| 'rbac' | ||
| ]) | ||
| param authType string = 'rbac' | ||
|
|
||
| param searchServiceName string = '' | ||
| param searchServiceResourceGroupName string = '' | ||
| param searchServiceResourceGroupLocation string = location | ||
|
|
@@ -115,10 +121,11 @@ module backend 'core/host/appservice.bicep' = { | |
| authClientId: authClientId | ||
| authIssuerUri: authIssuerUri | ||
| appSettings: { | ||
| AZURE_AUTH_TYPE: authType | ||
| // search | ||
| AZURE_SEARCH_INDEX: searchIndexName | ||
| AZURE_SEARCH_SERVICE: searchService.outputs.name | ||
| AZURE_SEARCH_KEY: authType == 'keys' ? searchService.outputs.adminKey : null | ||
| AZURE_SEARCH_USE_SEMANTIC_SEARCH: searchUseSemanticSearch | ||
| AZURE_SEARCH_SEMANTIC_SEARCH_CONFIG: searchSemanticSearchConfig | ||
| AZURE_SEARCH_TOP_K: searchTopK | ||
|
|
@@ -131,7 +138,7 @@ module backend 'core/host/appservice.bicep' = { | |
| AZURE_OPENAI_RESOURCE: openAi.outputs.name | ||
| AZURE_OPENAI_MODEL: openAIModel | ||
| AZURE_OPENAI_MODEL_NAME: openAIModelName | ||
| AZURE_OPENAI_KEY: authType == 'keys' ? openAi.outputs.key : null | ||
| AZURE_OPENAI_TEMPERATURE: openAITemperature | ||
| AZURE_OPENAI_TOP_P: openAITopP | ||
| AZURE_OPENAI_MAX_TOKENS: openAIMaxTokens | ||
|
|
@@ -189,6 +196,7 @@ module searchService 'core/search/search-services.bicep' = { | |
| aadAuthFailureMode: 'http401WithBearerChallenge' | ||
| } | ||
| } | ||
| authType: authType | ||
| sku: { | ||
| name: !empty(searchServiceSkuName) ? searchServiceSkuName : 'standard' | ||
| } | ||
|
|
@@ -250,6 +258,27 @@ module searchServiceContribRoleUser 'core/security/role.bicep' = { | |
| } | ||
| } | ||
|
|
||
|
|
||
| module openAiContributorRoleUser 'core/security/role.bicep' = if (authType == 'rbac') { | ||
| scope: searchServiceResourceGroup | ||
| name: 'openai-contributor-role-user' | ||
| params: { | ||
| principalId: principalId | ||
| roleDefinitionId: 'a001fd3d-188f-4b5d-821b-7da978bf7442' | ||
| principalType: 'User' | ||
| } | ||
| } | ||
|
|
||
| module cognitiveServicesContributorRoleUser 'core/security/role.bicep' = if (authType == 'rbac') { | ||
| scope: searchServiceResourceGroup | ||
| name: 'cognitive-services-contributor-role-user' | ||
| params: { | ||
| principalId: principalId | ||
| roleDefinitionId: '25fbc0a9-bd7c-42a3-aa1a-3b75d497ee68' | ||
| principalType: 'User' | ||
| } | ||
| } | ||
|
|
||
| // SYSTEM IDENTITIES | ||
| module openAiRoleBackend 'core/security/role.bicep' = { | ||
| scope: openAiResourceGroup | ||
|
|
@@ -271,6 +300,46 @@ module searchRoleBackend 'core/security/role.bicep' = { | |
| } | ||
| } | ||
|
|
||
| module searchRoleOpenAi 'core/security/role.bicep' = if (authType == 'rbac') { | ||
|
There was a problem hiding this comment. Any new roles you create will also need to be assigned to the user's principalId, see "user roles" above. |
||
| scope: searchServiceResourceGroup | ||
| name: 'search-role-openai' | ||
| params: { | ||
| principalId: openAi.outputs.identityPrincipalId | ||
| roleDefinitionId: '1407120a-92aa-4202-b7e9-c0e197c71c8f' | ||
| principalType: 'ServicePrincipal' | ||
| } | ||
| } | ||
|
|
||
| module searchServiceRoleOpenAi 'core/security/role.bicep' = if (authType == 'rbac') { | ||
| scope: searchServiceResourceGroup | ||
| name: 'search-service-role-openai' | ||
| params: { | ||
| principalId: openAi.outputs.identityPrincipalId | ||
| roleDefinitionId: '7ca78c08-252a-4471-8644-bb5ff32d4ba0' | ||
| principalType: 'ServicePrincipal' | ||
| } | ||
| } | ||
|
|
||
| module openAiContributorRoleSearch 'core/security/role.bicep' = if (authType == 'rbac') { | ||
| scope: searchServiceResourceGroup | ||
| name: 'openai-contributor-role-search' | ||
| params: { | ||
| principalId: searchService.outputs.identityPrincipalId | ||
| roleDefinitionId: 'a001fd3d-188f-4b5d-821b-7da978bf7442' | ||
| principalType: 'ServicePrincipal' | ||
| } | ||
| } | ||
|
|
||
| module cognitiveServicesContributorRoleSearch 'core/security/role.bicep' = if (authType == 'rbac') { | ||
| scope: searchServiceResourceGroup | ||
| name: 'cognitive-services-contributor-role-search' | ||
| params: { | ||
| principalId: searchService.outputs.identityPrincipalId | ||
| roleDefinitionId: '25fbc0a9-bd7c-42a3-aa1a-3b75d497ee68' | ||
| principalType: 'ServicePrincipal' | ||
| } | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. In outputs below we shouldn't be outputting keys. You will get a bicep linter warning. Best to ask the user to manually set them with in env vars at this time until we have a secure way to store them. There was a problem hiding this comment. Phew, glad to see the keys are out of this output finally! That's been a TODO (in my head) for a long time. Another option btw is to provide a script that grabs the keys, which is what I've had to do for another repo which unfortunately requires keys: But probably okay not to do that here, since we now have the keyless option. |
||
| // For doc prep | ||
| module docPrepResources 'docprep.bicep' = { | ||
| name: 'docprep-resources${resourceToken}' | ||
|
|
@@ -297,7 +366,6 @@ output AZURE_SEARCH_INDEX string = searchIndexName | |
| output AZURE_SEARCH_SERVICE string = searchService.outputs.name | ||
| output AZURE_SEARCH_SERVICE_RESOURCE_GROUP string = searchServiceResourceGroup.name | ||
| output AZURE_SEARCH_SKU_NAME string = searchService.outputs.skuName | ||
| output AZURE_SEARCH_USE_SEMANTIC_SEARCH bool = searchUseSemanticSearch | ||
| output AZURE_SEARCH_SEMANTIC_SEARCH_CONFIG string = searchSemanticSearchConfig | ||
| output AZURE_SEARCH_TOP_K int = searchTopK | ||
|
|
@@ -314,7 +382,6 @@ output AZURE_OPENAI_ENDPOINT string = openAi.outputs.endpoint | |
| output AZURE_OPENAI_MODEL string = openAIModel | ||
| output AZURE_OPENAI_MODEL_NAME string = openAIModelName | ||
| output AZURE_OPENAI_SKU_NAME string = openAi.outputs.skuName | ||
| output AZURE_OPENAI_EMBEDDING_NAME string = '${embeddingDeploymentName}' | ||
| output AZURE_OPENAI_TEMPERATURE int = openAITemperature | ||
| output AZURE_OPENAI_TOP_P int = openAITopP | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather the flow of the application take care of all the settings and not require the user to take manual action.
We are experimenting with adding an AZURE_AUTH_TYPE parameter that can get set to rbac or keys and the bicep does the appropriate thing based on that.
It's not finalized, but should be soon.