[.NET][JS] Update OAuth sample with FIC authentication#438
[.NET][JS] Update OAuth sample with FIC authentication#438ceciliaavila wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthroughThis set of changes updates both C# and JavaScript bot authentication samples to support Federated Identity Credentials (FIC) alongside existing authentication methods. Project files, configuration files, and environment variables are revised to reflect new authentication parameters and dependencies. Documentation is extensively rewritten to provide detailed guidance for configuring FIC, including Azure resource setup, managed identities, and federated credential registration. Several code updates introduce or revise the use of federated credential factories for authentication. Some solution and project files are cleaned up, and minor corrections are made to titles, namespaces, and JSON syntax. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Bot (App Service)
participant Azure AD (Entra ID)
participant Managed Identity
participant Federated Credential Factory
User->>Bot (App Service): Sends message
Bot (App Service)->>Federated Credential Factory: Requests credentials (AppId, ClientId, TenantId)
Federated Credential Factory->>Managed Identity: Authenticate via federated identity
Managed Identity->>Azure AD (Entra ID): Exchange token using federated credentials
Azure AD (Entra ID)-->>Managed Identity: Returns access token
Managed Identity-->>Federated Credential Factory: Returns token
Federated Credential Factory-->>Bot (App Service): Provides credentials
Bot (App Service)->>User: Processes and responds to message
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
samples/javascript_nodejs/86.bot-authentication-fic/README.md (2)
32-44: Address Markdown formatting inconsistenciesThere are inconsistent indentation levels in the unordered lists, which might affect readability.
Consider fixing the list indentation to make the document more readable and consistent:
- Use 2 spaces for top-level list items
- Use 4 spaces for nested list items
- Ensure consistent indentation for items at the same nesting level
🧰 Tools
🪛 LanguageTool
[uncategorized] ~32-~32: Possible missing article found.
Context: ...d in the previous step. - Enter name for the credential and click on Add. ...(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~44-~44: You might be missing the article “the” here.
Context: ...dd the User managed identity created in previous step to the Azure App Service under Con...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
37-37: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
38-38: Unordered list indentation
Expected: 4; Actual: 5(MD007, ul-indent)
39-39: Inconsistent indentation for list items at the same level
Expected: 5; Actual: 6(MD005, list-indent)
39-39: Unordered list indentation
Expected: 4; Actual: 6(MD007, ul-indent)
40-40: Inconsistent indentation for list items at the same level
Expected: 5; Actual: 6(MD005, list-indent)
40-40: Unordered list indentation
Expected: 4; Actual: 6(MD007, ul-indent)
41-41: Inconsistent indentation for list items at the same level
Expected: 5; Actual: 6(MD005, list-indent)
41-41: Unordered list indentation
Expected: 4; Actual: 6(MD007, ul-indent)
42-42: Inconsistent indentation for list items at the same level
Expected: 5; Actual: 6(MD005, list-indent)
42-42: Unordered list indentation
Expected: 4; Actual: 6(MD007, ul-indent)
43-43: Unordered list indentation
Expected: 2; Actual: 3(MD007, ul-indent)
44-44: Unordered list indentation
Expected: 4; Actual: 5(MD007, ul-indent)
32-32: Add missing articles in sentencesAdd "a" and "the" where they're missing in these sentences.
- Enter name for the credential and click on Add. + Enter a name for the credential and click on Add. - Add the User managed identity created in previous step to the Azure App Service + Add the User managed identity created in the previous step to the Azure App ServiceAlso applies to: 44-44
🧰 Tools
🪛 LanguageTool
[uncategorized] ~32-~32: Possible missing article found.
Context: ...d in the previous step. - Enter name for the credential and click on Add. ...(AI_HYDRA_LEO_MISSING_THE)
samples/csharp_dotnetcore/18.bot-authentication/README.md (4)
32-40: Grammar and clarity improvements needed.The setup instructions are comprehensive but have a few grammatical issues:
- - Create a MS Entra ID application to register a bot resource in Azure and a separate MS Entra ID application to function as the identity provider. + - Create an MS Entra ID application to register a bot resource in Azure and a separate MS Entra ID application to function as the identity provider.🧰 Tools
🪛 LanguageTool
[misspelling] ~32-~32: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...otbuilder-samples.git ``` - Create a MS Entra ID application to register a b...(EN_A_VS_AN)
48-49: Fix punctuation in URL reference.There's an unnecessary colon before the URL.
- - Update the **Messaging endpoint** on the **Configuration** tab to: https://{default-domain}/api/messages. + - Update the **Messaging endpoint** on the **Configuration** tab to https://{default-domain}/api/messages.🧰 Tools
🪛 LanguageTool
[typographical] ~48-~48: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...endpoint** on the Configuration tab to: https://{default-domain}/api/messages....(RP_COLON)
80-80: Grammar correction needed.Missing the definite article "the" in the description.
- | MicrosoftAppType | SingleTenant | Currently, FIC feature is supported in single tenant bots only. | + | MicrosoftAppType | SingleTenant | Currently, the FIC feature is supported in single tenant bots only. |🧰 Tools
🪛 LanguageTool
[uncategorized] ~80-~80: You might be missing the article “the” here.
Context: ...e | SingleTenant | Currently, FIC feature is supported in single tenant b...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
91-91: Fix hyphenation in "Right-click".The term "Right-click" should be hyphenated.
- - Right click the project and select **Publish**. + - Right-click the project and select **Publish**.🧰 Tools
🪛 LanguageTool
[grammar] ~91-~91: The verb “Right-click” is spelled with a hyphen.
Context: ...g the Visual Studio Publish tool. - Right click the project and select Publish. - C...(CLICK_HYPHEN)
samples/javascript_nodejs/18.bot-authentication/README.md (5)
43-50: Grammar improvements needed.The setup instructions have a few grammatical issues:
- - Create a MS Entra ID application to register a bot resource in Azure and a separate MS Entra ID application to function as the identity provider. + - Create an MS Entra ID application to register a bot resource in Azure and a separate MS Entra ID application to function as the identity provider. - - Add the User managed identity created in previous step to the Azure App Service under Configuration -> Identity -> User Assigned Managed Identity. + - Add the User managed identity created in the previous step to the Azure App Service under Configuration -> Identity -> User Assigned Managed Identity.🧰 Tools
🪛 LanguageTool
[misspelling] ~43-~43: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...`bash npm install ``` - Create a MS Entra ID application to register a b...(EN_A_VS_AN)
[uncategorized] ~48-~48: You might be missing the article “the” here.
Context: ...dd the User managed identity created in previous step to the Azure App Service under Con...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
51-51: Grammar correction needed.Missing the definite article "the" in the description.
- - This should be Single tenant (Currently, FIC feature is supported in single tenant bots only). + - This should be Single tenant (Currently, the FIC feature is supported in single tenant bots only).🧰 Tools
🪛 LanguageTool
[uncategorized] ~51-~51: You might be missing the article “the” here.
Context: ...his should be Single tenant (Currently, FIC feature is supported in single tenant b...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
57-57: Grammar improvement needed.Missing an indefinite article.
- - Enter name for the credential and click on **Add**. + - Enter a name for the credential and click on **Add**.🧰 Tools
🪛 LanguageTool
[uncategorized] ~57-~57: Possible missing article found.
Context: ...d in the previous step. - Enter name for the credential and click on Add...(AI_HYDRA_LEO_MISSING_THE)
62-62: Correct grammatical error in list reference.The word "followings" should be "following".
- - Select SingleTenant and for the Redirect URL, use one of the followings: [OAuth URL support in Azure AI Bot Service](https://learn.microsoft.com/en-us/azure/bot-service/ref-oauth-redirect-urls?view=azure-bot-service-4.0). + - Select SingleTenant and for the Redirect URL, use one of the following: [OAuth URL support in Azure AI Bot Service](https://learn.microsoft.com/en-us/azure/bot-service/ref-oauth-redirect-urls?view=azure-bot-service-4.0).🧰 Tools
🪛 LanguageTool
[uncategorized] ~62-~62: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...nd for the Redirect URL, use one of the followings: [OAuth URL support in Azure AI Bot Ser...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
100-101: Fix formatting in section header.There's a missing line break between "Deploy the bot to Azure" and "To learn more...". This makes it look like a single run-on sentence.
-## Deploy the bot to Azure -To learn more about deploying a bot to Azure, see [Deploy your bot to Azure](https://aka.ms/azuredeployment) for a complete list of deployment instructions. +## Deploy the bot to Azure + +To learn more about deploying a bot to Azure, see [Deploy your bot to Azure](https://aka.ms/azuredeployment) for a complete list of deployment instructions.🧰 Tools
🪛 LanguageTool
[grammar] ~100-~100: Did you mean “too Azure To”?
Context: ...e deployed to Azure. ## Deploy the bot to Azure To learn more about deploying a bot to Azu...(TOO_ADJECTIVE_TO)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
samples/csharp_dotnetcore/18.bot-authentication/AuthenticationBot.csproj(1 hunks)samples/csharp_dotnetcore/18.bot-authentication/README.md(3 hunks)samples/csharp_dotnetcore/18.bot-authentication/Startup.cs(2 hunks)samples/csharp_dotnetcore/18.bot-authentication/appsettings.json(1 hunks)samples/csharp_dotnetcore/86.bot-authentication-fic/AdapterWithErrorHandler.cs(1 hunks)samples/csharp_dotnetcore/86.bot-authentication-fic/AuthFederatedCredBot.csproj(1 hunks)samples/csharp_dotnetcore/86.bot-authentication-fic/Controllers/BotController.cs(1 hunks)samples/csharp_dotnetcore/86.bot-authentication-fic/README.md(3 hunks)samples/csharp_dotnetcore/86.bot-authentication-fic/Startup.cs(0 hunks)samples/csharp_dotnetcore/86.bot-authentication-fic/appsettings.json(1 hunks)samples/csharp_dotnetcore/86.bot-authentication-fic/wwwroot/default.htm(1 hunks)samples/csharp_dotnetcore/csharp_dotnetcore.sln(0 hunks)samples/javascript_nodejs/18.bot-authentication/.env(1 hunks)samples/javascript_nodejs/18.bot-authentication/README.md(3 hunks)samples/javascript_nodejs/18.bot-authentication/index.js(2 hunks)samples/javascript_nodejs/18.bot-authentication/package.json(1 hunks)samples/javascript_nodejs/86.bot-authentication-fic/README.md(2 hunks)samples/javascript_nodejs/86.bot-authentication-fic/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- samples/csharp_dotnetcore/86.bot-authentication-fic/Startup.cs
- samples/csharp_dotnetcore/csharp_dotnetcore.sln
🧰 Additional context used
🧬 Code Graph Analysis (1)
samples/csharp_dotnetcore/18.bot-authentication/Startup.cs (3)
samples/csharp_dotnetcore/84.bot-authentication-certificate/Startup.cs (2)
Startup(19-90)Startup(23-26)samples/csharp_dotnetcore/85.bot-authentication-sni/Startup.cs (2)
Startup(20-91)Startup(24-27)samples/csharp_dotnetcore/02.echo-bot/Startup.cs (2)
Startup(17-63)Startup(19-22)
🪛 LanguageTool
samples/javascript_nodejs/86.bot-authentication-fic/README.md
[uncategorized] ~32-~32: Possible missing article found.
Context: ...d in the previous step. - Enter name for the credential and click on Add. ...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~44-~44: You might be missing the article “the” here.
Context: ...dd the User managed identity created in previous step to the Azure App Service under Con...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
samples/csharp_dotnetcore/18.bot-authentication/README.md
[misspelling] ~32-~32: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...otbuilder-samples.git ``` - Create a MS Entra ID application to register a b...
(EN_A_VS_AN)
[typographical] ~48-~48: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...endpoint** on the Configuration tab to: https://{default-domain}/api/messages....
(RP_COLON)
[uncategorized] ~80-~80: You might be missing the article “the” here.
Context: ...e | SingleTenant | Currently, FIC feature is supported in single tenant b...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~91-~91: The verb “Right-click” is spelled with a hyphen.
Context: ...g the Visual Studio Publish tool. - Right click the project and select Publish. - C...
(CLICK_HYPHEN)
samples/javascript_nodejs/18.bot-authentication/README.md
[misspelling] ~43-~43: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...`bash npm install ``` - Create a MS Entra ID application to register a b...
(EN_A_VS_AN)
[uncategorized] ~48-~48: You might be missing the article “the” here.
Context: ...dd the User managed identity created in previous step to the Azure App Service under Con...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~51-~51: You might be missing the article “the” here.
Context: ...his should be Single tenant (Currently, FIC feature is supported in single tenant b...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~57-~57: Possible missing article found.
Context: ...d in the previous step. - Enter name for the credential and click on Add...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~62-~62: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...nd for the Redirect URL, use one of the followings: [OAuth URL support in Azure AI Bot Ser...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[grammar] ~100-~100: Did you mean “too Azure To”?
Context: ...e deployed to Azure. ## Deploy the bot to Azure To learn more about deploying a bot to Azu...
(TOO_ADJECTIVE_TO)
🪛 markdownlint-cli2 (0.17.2)
samples/javascript_nodejs/86.bot-authentication-fic/README.md
37-37: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
38-38: Unordered list indentation
Expected: 4; Actual: 5
(MD007, ul-indent)
39-39: Inconsistent indentation for list items at the same level
Expected: 5; Actual: 6
(MD005, list-indent)
39-39: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
40-40: Inconsistent indentation for list items at the same level
Expected: 5; Actual: 6
(MD005, list-indent)
40-40: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
41-41: Inconsistent indentation for list items at the same level
Expected: 5; Actual: 6
(MD005, list-indent)
41-41: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
42-42: Inconsistent indentation for list items at the same level
Expected: 5; Actual: 6
(MD005, list-indent)
42-42: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
43-43: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
44-44: Unordered list indentation
Expected: 4; Actual: 5
(MD007, ul-indent)
🔇 Additional comments (42)
samples/javascript_nodejs/18.bot-authentication/.env (2)
3-3: Verify environment variable alignment with code
Confirm that the sample’s startup code now readsprocess.env.MicrosoftAppClientId(instead ofMicrosoftAppPassword) and thatMicrosoftAppIdisn’t accidentally left unused or conflicting.
5-5: Skip: newline consistency
The added newline at the end of the file follows POSIX conventions and has no functional impact.samples/csharp_dotnetcore/86.bot-authentication-fic/appsettings.json (1)
5-5: JSON syntax correction
Removing the trailing comma after"MicrosoftAppTenantId"makes the JSON valid.samples/csharp_dotnetcore/86.bot-authentication-fic/Controllers/BotController.cs (1)
8-8: Standardize controller namespace
Changing toMicrosoft.BotBuilderSamples.Controllersaligns with sample conventions. Ensure the project’s default namespace in the.csprojmatches this update to prevent build warnings.samples/csharp_dotnetcore/86.bot-authentication-fic/AuthFederatedCredBot.csproj (1)
4-4: Framework bump to .NET 8.0
Upgrading tonet8.0ensures alignment with other samples targeting the latest .NET version. Ensure your local development environment has the .NET 8 SDK installed.samples/javascript_nodejs/86.bot-authentication-fic/package.json (1)
4-4: Update package description terminology
The description has been corrected to reference "Federated Identity Credentials (FIC)" in line with the sample updates.samples/csharp_dotnetcore/86.bot-authentication-fic/AdapterWithErrorHandler.cs (1)
13-13: Simplification of AdapterWithErrorHandler constructor makes error handling more streamlined.The constructor has been modified to remove the
ConversationStateparameter, which aligns with the sample's streamlined approach for FIC authentication where state management services have been removed from the dependency injection container in Startup.cs.samples/csharp_dotnetcore/86.bot-authentication-fic/wwwroot/default.htm (1)
7-7: Updated title reflects the sample's purpose.The document title has been changed from "bot_authentication_fic" to "AuthFederatedCredBot", making it more descriptive and consistent with the sample's focus on Federated Identity Credentials authentication.
samples/csharp_dotnetcore/86.bot-authentication-fic/README.md (7)
1-3: Improved terminology consistency in title and description.The terminology has been corrected from "Federation Identity Credentials" to "Federated Identity Credentials" and clarifies that FIC is currently supported only in single tenant bots.
5-5: Corrected phrasing for better technical accuracy.Changed "federated identity certificate configuration" to "federated identity credentials configuration", which more accurately describes the authentication mechanism being used.
41-41: Added important single tenant requirement.Clarified that FIC feature is supported only in single tenant bots, which is critical information for users trying to implement this authentication method.
43-45: Improved federated credential configuration instructions.The instructions now more clearly explain the process of linking managed identity to the App Registration and specify the correct "Managed Identity" scenario selection.
54-57: Consistent configuration guidance for existing bots.The instructions for existing bots have been aligned with the new bot setup process, ensuring consistent guidance for all users regardless of whether they're creating a new bot or configuring an existing one.
64-64: Fixed app setting type value.Corrected the MicrosoftAppType value to "SingleTenant" without a space, which matches the expected format in the configuration.
72-72: Clarified deployment requirements.The note about local running limitations has been reworded for clarity, emphasizing that bots using Federated Credentials must be deployed to Azure.
samples/javascript_nodejs/18.bot-authentication/package.json (1)
19-20: Updated Bot Framework SDK dependencies.The botbuilder and botbuilder-dialogs packages have been upgraded from version 4.23.0 to 4.23.1, ensuring compatibility with the Federated Identity Credentials authentication features introduced in this PR.
samples/csharp_dotnetcore/18.bot-authentication/appsettings.json (1)
4-4: Configuration updated to support FIC authenticationThe
MicrosoftAppClientIdconfiguration key has been added, replacing the previousMicrosoftAppPasswordkey. This change aligns with the PR objective of transitioning from client secret authentication to Federated Identity Credentials (FIC).samples/csharp_dotnetcore/18.bot-authentication/AuthenticationBot.csproj (1)
10-11: Upgraded Bot Builder dependenciesBot Builder dependencies have been updated from version 4.22.4 to 4.23.0, which supports the new Federated Identity Credentials authentication functionality. This upgrade aligns with the PR objectives.
samples/javascript_nodejs/18.bot-authentication/index.js (3)
16-16: Imported FederatedServiceClientCredentialsFactory for FIC authenticationAdded import to support the new FIC authentication implementation.
31-36: Implemented FIC authentication using environment variablesThe code correctly initializes the FederatedServiceClientCredentialsFactory with the necessary environment variables to support Federated Identity Credentials. The use of the nullish coalescing operator (
??) is a good practice to provide fallback empty strings for undefined values.
38-38: Updated BotFrameworkAuthentication to use FICThe ConfigurationBotFrameworkAuthentication now uses the FederatedServiceClientCredentialsFactory, completing the transition to FIC authentication.
samples/javascript_nodejs/86.bot-authentication-fic/README.md (5)
1-5: Improved terminology accuracy for FICCorrected terminology from "Federation Identity Certificate" to "Federated Identity Credentials" and clarified that the feature is limited to single tenant bots only.
26-26: Emphasized single tenant requirementClearly specified that FIC feature is only supported for single tenant bots, which is an important limitation for implementers to understand.
28-31: Corrected FIC configuration instructionsUpdated the instructions to properly specify "Managed Identity" as the correct federated credential scenario instead of "Customer Managed Keys".
Also applies to: 38-41
48-48: Updated example app type valueThe example now correctly shows "SingleTenant" as the value for MicrosoftAppType, reinforcing the single tenant requirement for FIC.
56-56: Clarified deployment requirementsRefined the explanation that bots using Federated Credentials must be deployed to Azure and cannot run locally.
samples/csharp_dotnetcore/18.bot-authentication/Startup.cs (3)
9-9: LGTM: Required namespace import added.The Microsoft.Extensions.Configuration namespace is correctly added to support the configuration injection pattern implemented in this update.
17-22: LGTM: Standard ASP.NET Core configuration injection pattern implemented.This follows the recommended pattern for accessing configuration in ASP.NET Core applications:
- Constructor injection of IConfiguration
- Public property to expose the configuration
This enables the configuration-based setup of the FederatedServiceClientCredentialsFactory that follows.
32-38: LGTM: Federated Identity Credentials factory correctly configured.The implementation properly registers a FederatedServiceClientCredentialsFactory singleton that will be used for bot authentication. It correctly pulls the required parameters from configuration:
- MicrosoftAppId: The bot's app registration ID
- MicrosoftAppClientId: The User Managed Identity ID
- MicrosoftAppTenantId: The bot's app tenant ID
This registration enables the bot to use Federated Identity Credentials for authentication instead of client secrets.
samples/csharp_dotnetcore/18.bot-authentication/README.md (7)
11-12: LGTM: Clear documentation about authentication options.The note clearly indicates that the sample supports both Federated Identity Credentials (FIC) and ClientSecret authentication methods, with a link to relevant documentation.
22-22: LGTM: SDK version requirement updated.Correctly specifies the minimum version requirement for Bot Framework SDK to support the Federated Identity Credentials feature.
57-62: LGTM: Detailed technical guidance for federated credentials.The instructions provide essential information about creating the subject identifier for federated credentials, including helpful links for encoding tenant IDs.
77-85: LGTM: Well-structured configuration table.The configuration table clearly lists all required settings in the appsettings.json file, with descriptions that help users understand each value's purpose.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~80-~80: You might be missing the article “the” here.
Context: ...e | SingleTenant | Currently, FIC feature is supported in single tenant b...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
86-86: LGTM: Important deployment constraint clearly stated.Critical information about deployment requirements is clearly highlighted - bots using Federated Credentials must be deployed to Azure and cannot run locally.
88-96: LGTM: Clear deployment instructions.The deployment instructions provide a straightforward, step-by-step process for publishing the bot to Azure using Visual Studio.
🧰 Tools
🪛 LanguageTool
[grammar] ~91-~91: The verb “Right-click” is spelled with a hyphen.
Context: ...g the Visual Studio Publish tool. - Right click the project and select Publish. - C...(CLICK_HYPHEN)
108-108: LGTM: Updated documentation links.Added relevant link to the documentation on implementing authentication with Federated Identity Credentials.
samples/javascript_nodejs/18.bot-authentication/README.md (6)
11-11: LGTM: Clear documentation about authentication options.The note clearly indicates that the sample supports both Federated Identity Credentials and ClientSecret authentication methods, with a link to relevant documentation.
21-21: LGTM: SDK version requirement updated.Correctly specifies the minimum version requirement for Bot Framework SDK to support Federated Identity Credentials.
71-73: LGTM: Helpful technical guidance for encoding identifiers.The note provides valuable information about encoding tenant IDs for the federated credential subject identifier.
86-97: LGTM: Clear environment variable documentation.The documentation clearly lists all required environment variables with their values, making it easy for developers to configure their applications.
98-98: LGTM: Important deployment constraint clearly stated.Critical information about deployment requirements is clearly highlighted - bots using Federated Credentials must be deployed to Azure and cannot run locally.
112-113: LGTM: Updated documentation links.Updated links to the latest documentation on authentication, including the new reference for Federated Identity Credentials implementation.
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="3.1.1" /> | ||
| <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="6.0.7" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align package version with target framework
Referencing Microsoft.AspNetCore.Mvc.NewtonsoftJson v6.0.7 on a net8.0 target can cause compatibility warnings. Consider bumping this dependency to the 8.x series (e.g., 8.0.0) to match the SDK.
- <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="6.0.7" />
+ <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="8.0.0" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="6.0.7" /> | |
| <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="8.0.0" /> |
|
/promoted 4046 |
2 participants
#minor
Description
This PR updates the .NET and JS 18.bot-authentication samples replacing the client secret with Federated Identity Credentials authentication.
Aditionally, it updates the README files with the instruction to configure the Azure resources.
Detailed Changes
csharp_dotnetcore/18.bot-authenticationto use FIC as the authentication method.csharp_dotnetcore/86.bot-authentication-ficsample to remove unused code.javascript_nodejs/18.bot-authenticationto use FIC as the authentication method.javascript_nodejs/86.bot-authentication-ficsample.Testing
These images show the samples working after following the configuration steps.
