-
Notifications
You must be signed in to change notification settings - Fork 1
Update credential usage across all languages #15
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
Changes from all commits
d62592d
1b7d853
8a78719
0f19550
83da7ca
22deb30
e873ccc
8bb07bd
24950e4
3b4ac77
5831e9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,6 @@ | |||||
| using System.Text; | ||||||
| using System.Text.Json; | ||||||
| using Azure.Core; | ||||||
| using Azure.Identity; | ||||||
| using Npgsql; | ||||||
|
|
||||||
| namespace Microsoft.Azure.PostgreSQL.Auth; | ||||||
|
|
@@ -25,12 +24,11 @@ public static class EntraIdExtension | |||||
| /// Configures the NpgsqlDataSourceBuilder to use Entra ID authentication synchronously. | ||||||
| /// </summary> | ||||||
| /// <param name="dataSourceBuilder">The NpgsqlDataSourceBuilder to configure.</param> | ||||||
| /// <param name="credential">The TokenCredential to use for authentication. If null, DefaultAzureCredential is used.</param> | ||||||
| /// <param name="credential">The TokenCredential to use for authentication.</param> | ||||||
| /// <param name="cancellationToken">A cancellation token that can be used to cancel the operation.</param> | ||||||
| /// <returns>The configured NpgsqlDataSourceBuilder.</returns> | ||||||
| public static NpgsqlDataSourceBuilder UseEntraAuthentication(this NpgsqlDataSourceBuilder dataSourceBuilder, TokenCredential? credential = default, CancellationToken cancellationToken = default) | ||||||
| public static NpgsqlDataSourceBuilder UseEntraAuthentication(this NpgsqlDataSourceBuilder dataSourceBuilder, TokenCredential credential, CancellationToken cancellationToken = default) | ||||||
| { | ||||||
| credential ??= new DefaultAzureCredential(); | ||||||
|
|
||||||
| if (dataSourceBuilder.ConnectionStringBuilder.Username == null) | ||||||
| { | ||||||
|
|
@@ -60,12 +58,11 @@ public static NpgsqlDataSourceBuilder UseEntraAuthentication(this NpgsqlDataSour | |||||
| /// Configures the NpgsqlDataSourceBuilder to use Entra ID authentication asynchronously. | ||||||
| /// </summary> | ||||||
| /// <param name="dataSourceBuilder">The NpgsqlDataSourceBuilder to configure.</param> | ||||||
| /// <param name="credential">The TokenCredential to use for authentication. If null, DefaultAzureCredential is used.</param> | ||||||
| /// <param name="credential">The TokenCredential to use for authentication.</param> | ||||||
| /// <param name="cancellationToken">A cancellation token that can be used to cancel the operation.</param> | ||||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the configured NpgsqlDataSourceBuilder.</returns> | ||||||
| public static async Task<NpgsqlDataSourceBuilder> UseEntraAuthenticationAsync(this NpgsqlDataSourceBuilder dataSourceBuilder, TokenCredential? credential = default, CancellationToken cancellationToken = default) | ||||||
| public static async Task<NpgsqlDataSourceBuilder> UseEntraAuthenticationAsync(this NpgsqlDataSourceBuilder dataSourceBuilder, TokenCredential credential, CancellationToken cancellationToken = default) | ||||||
| { | ||||||
| credential ??= new DefaultAzureCredential(); | ||||||
|
|
||||||
|
||||||
| ArgumentNullException.ThrowIfNull(credential); |
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.
Same as above
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -26,7 +26,6 @@ | |||||||
|
|
||||||||
| <ItemGroup> | ||||||||
| <PackageReference Include="Azure.Core" Version="1.44.1" /> | ||||||||
| <PackageReference Include="Azure.Identity" Version="1.13.1" /> | ||||||||
| <PackageReference Include="Npgsql" Version="8.0.5" /> | ||||||||
|
||||||||
| <PackageReference Include="Npgsql" Version="8.0.5" /> | |
| <PackageReference Include="Npgsql" Version="8.0.5" /> | |
| <PackageReference Include="Azure.Identity" Version="1.10.3" /> |
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.
Azure.Identity is included as a dependency in GettingStarted.csproj, which is the project file for the sample code for .NET.
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.
Missing null validation for the required
credentialparameter. Since the parameter is no longer nullable (changed fromTokenCredential?toTokenCredential), callers could still passnullexplicitly, which would cause aNullReferenceExceptionwhen the credential is used later.Add explicit validation:
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.
No, callers can't pass
nullexplicitly. They will get a compiler warning.