[PostgreSQL] Add Microsoft.Azure.PostgreSQL.Auth library #55231
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new library Microsoft.Azure.PostgreSQL.Auth that provides Entra ID (formerly Azure AD) authentication support for Npgsql PostgreSQL connections. The library enables passwordless authentication using OAuth 2.0 tokens through Azure's identity platform.
Changes:
- Added EntraIdExtension class with UseEntraAuthentication and UseEntraAuthenticationAsync extension methods
- Implemented JWT token parsing to extract usernames from Entra ID tokens
- Created comprehensive integration tests using PostgreSQL test containers
- Added sample code and documentation
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| EntraIdExtension.cs | Core implementation providing Entra ID authentication extension methods for Npgsql |
| Microsoft.Azure.PostgreSQL.Auth.csproj (src) | Project file for the main library with package metadata |
| Microsoft.Azure.PostgreSQL.Auth.csproj (tests) | Test project configuration |
| EntraIdExtensionTests.cs | Comprehensive integration tests for Entra authentication functionality |
| TestUtilities.cs | Test helpers including JWT token generation and test credentials |
| CreateDbConnectionNpgsql.cs | Sample demonstrating library usage |
| GettingStarted.csproj | Sample project configuration |
| dotnet.sln | Solution file organizing projects |
| README.md | Library documentation and usage guide |
| CHANGELOG.md | Version history and changes |
| appsettings.sample.json | Sample configuration file |
|
|
||
| var dataSourceBuilder = new NpgsqlDataSourceBuilder(connectionString); | ||
|
|
||
| // Here, we use the appropriate extension method provided by NpgsqlDataSourceBuilderExtensions.cs |
There was a problem hiding this comment.
The comment references "NpgsqlDataSourceBuilderExtensions.cs" but the actual class name is "EntraIdExtension". This could confuse developers trying to understand the code.
| // Here, we use the appropriate extension method provided by NpgsqlDataSourceBuilderExtensions.cs | |
| // Here, we use the appropriate extension method provided by the EntraIdExtension class |
| var act = async () => await builder.UseEntraAuthenticationAsync(credential); | ||
|
|
||
| await act.Should().ThrowAsync<Exception>(); |
There was a problem hiding this comment.
The test validates that an exception is thrown for invalid JWT token format, but doesn't verify the error message. Since the test name mentions "ThrowMeaningfulErrorForInvalidJwtTokenFormat", consider adding an assertion to check that the error message is actually meaningful and helpful to users.
| /// <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, CancellationToken cancellationToken = default) |
There was a problem hiding this comment.
The method doesn't validate that the input parameters are not null. Following Azure SDK conventions (see Azure.Identity examples), public API methods should validate their parameters and throw ArgumentNullException when null parameters are passed. Add null checks for dataSourceBuilder and credential parameters.
| /// <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, CancellationToken cancellationToken = default) |
There was a problem hiding this comment.
The method doesn't validate that the input parameters are not null. Following Azure SDK conventions (see Azure.Identity examples), public API methods should validate their parameters and throw ArgumentNullException when null parameters are passed. Add null checks for dataSourceBuilder and credential parameters.
| } | ||
| else | ||
| { | ||
| throw new Exception("Could not determine username from token claims"); |
There was a problem hiding this comment.
This code throws a generic Exception instead of a more specific exception type. Based on Azure SDK conventions (e.g., Azure.Identity throws InvalidOperationException for similar scenarios), consider using InvalidOperationException or FormatException here for better error handling and debugging.
| <RepositoryUrl>https://github.com/Azure/postgres-entra-auth.git</RepositoryUrl> | ||
| <PackageProjectUrl>https://github.com/Azure/postgres-entra-auth</PackageProjectUrl> | ||
| <RepositoryType>git</RepositoryType> | ||
| <PackageReleaseNotes>https://github.com/Azure/postgres-entra-auth/releases</PackageReleaseNotes> |
There was a problem hiding this comment.
The PackageReleaseNotes URL points to "https://github.com/Azure/postgres-entra-auth/releases" which appears to be a different repository. This should point to the azure-sdk-for-net releases or an appropriate changelog location.
| <PackageReleaseNotes>https://github.com/Azure/postgres-entra-auth/releases</PackageReleaseNotes> | |
| <PackageReleaseNotes>https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/postgresql/Microsoft.Azure.PostgreSQL.Auth/CHANGELOG.md</PackageReleaseNotes> |
| var act = () => builder.UseEntraAuthentication(credential); | ||
|
|
||
| act.Should().Throw<Exception>(); |
There was a problem hiding this comment.
The test validates that an exception is thrown for invalid JWT token format, but doesn't verify the error message. Since the test name mentions "ThrowMeaningfulErrorForInvalidJwtTokenFormat", consider adding an assertion to check that the error message is actually meaningful and helpful to users.
| - | ||
|
|
||
| ### Fixed | ||
| - Remove dependency on DefaultAzureCredential in source library |
There was a problem hiding this comment.
The CHANGELOG has a "Fixed" entry stating "Remove dependency on DefaultAzureCredential in source library" but this is a new library being added. The "Fixed" section should be empty or this should be in a different section, as there was no previous version to fix.
| - | |
| ### Fixed | |
| - Remove dependency on DefaultAzureCredential in source library | |
| - Remove dependency on DefaultAzureCredential in source library | |
| ### Fixed | |
| - |
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.Azure.PostgreSQL.Entra", "src\Microsoft\Azure\PostgreSQL\Auth\Microsoft.Azure.PostgreSQL.Auth.csproj", "{3E862DB4-B843-4361-94B5-8CF34402B511}" | ||
| EndProject | ||
| Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "tests", "tests", "{0AB3BF05-4346-4AA6-1389-037BE0695223}" | ||
| EndProject | ||
| Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Microsoft", "Microsoft", "{8FEB4F0F-C974-64A2-0863-8577ABAC15AD}" | ||
| EndProject | ||
| Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Azure", "Azure", "{AC05A953-B9EF-C104-E53F-E15EBB9C3478}" | ||
| EndProject | ||
| Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "PostgreSQL", "PostgreSQL", "{7164C26A-6C7C-D37D-98D2-1150AFE094DD}" | ||
| EndProject | ||
| Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Auth", "Auth", "{290860F1-0C73-540D-3A79-AA6C3ABBD9C3}" | ||
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.Azure.PostgreSQL.Entra.Tests", "tests\Microsoft\Azure\PostgreSQL\Auth\Microsoft.Azure.PostgreSQL.Auth.csproj", "{750B2A4F-9EF5-4CC5-8EF9-A93F4A1748F6}" |
There was a problem hiding this comment.
The solution file references the project with name "Microsoft.Azure.PostgreSQL.Entra" on line 25 and "Microsoft.Azure.PostgreSQL.Entra.Tests" on line 37, but the actual project files are named "Microsoft.Azure.PostgreSQL.Auth.csproj". This naming inconsistency will cause the solution file to incorrectly reference these projects. The project names in the solution should match the actual .csproj file names.
| </PackageDescription> | ||
| <PackageTags>azure;entra;PostgreSQL;Npgsql</PackageTags> | ||
| <RepositoryUrl>https://github.com/Azure/postgres-entra-auth.git</RepositoryUrl> | ||
| <PackageProjectUrl>https://github.com/Azure/postgres-entra-auth</PackageProjectUrl> |
There was a problem hiding this comment.
The PackageProjectUrl points to "https://github.com/Azure/postgres-entra-auth" which appears to be a different repository. This should point to the azure-sdk-for-net repository or the appropriate documentation page for this package.
| <PackageProjectUrl>https://github.com/Azure/postgres-entra-auth</PackageProjectUrl> | |
| <PackageProjectUrl>https://github.com/Azure/azure-sdk-for-net/tree/main/sdk/postgresql/Microsoft.Azure.PostgreSQL.Auth</PackageProjectUrl> |
jsquire
left a comment
jsquire
left a comment
There was a problem hiding this comment.
This is not ready for review. There's a lot of work needed to update this to follow the conventions of this repository and integrate with the engineering system.
Please reference the Azure SDK onboarding guide (Microsoft internal) and the Azure SDK for .NET Contributing guide. For support, please use the Azure SDK onboarding assistance channel (Microsoft internal)
2 participants
Contributing to the Azure SDK
Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.
For specific information about pull request etiquette and best practices, see this section.