Skip to content

Conversation

@jumacabo
Copy link
Contributor

@jumacabo jumacabo commented Oct 30, 2025

What does this PR do?

Add support for PostgreSQL authentication, by adding 2 new parameters named auth-type and password to the following commands: postgres_database_list, postgres_table_list, postgres_database_query and postgres_table_schema_get.
Password parameter is optional an only required if we are using PostgreSQL authentication (MicrosoftEntra does not require it).

GitHub issue number?

#236

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages (cleanup guide)
    • Added comprehensive tests for new/modified functionality
    • Updated servers/Azure.Mcp.Server/CHANGELOG.md and/or servers/Fabric.Mcp.Server/CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
  • For MCP tool changes:
    • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
    • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
    • Validate README.md changes using script at eng/scripts/Process-PackageReadMe.ps1. See Package README
    • Updated command list in /servers/Azure.Mcp.Server/docs/azmcp-commands.md and/or /docs/fabric-commands.md
    • Run .\eng\scripts\Update-AzCommandsMetadata.ps1 to update tool metadata in azmcp-commands.md (required for CI)
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
    • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation in the PR description
  • Extra steps for Azure MCP Server tool changes:
    • Updated test prompts in /servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
    • 👉 For Community (non-Microsoft team member) PRs:
      • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
      • Manual tests run: added comment /azp run mcp - pullrequest - live to run Live Test Pipeline

@jumacabo jumacabo added bug Something isn't working tools-Postgres PostgreSQL labels Oct 30, 2025
@jumacabo jumacabo self-assigned this Oct 30, 2025
@jumacabo jumacabo marked this pull request as ready for review October 30, 2025 11:47
@jumacabo jumacabo requested review from a team, kk-src and xiangyan99 as code owners October 30, 2025 11:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for password-based authentication in addition to the existing Entra ID authentication for PostgreSQL tools. The change allows users to connect to PostgreSQL servers using either authentication method based on the username format.

Key changes:

  • Added optional password parameter to all PostgreSQL service methods and commands
  • Modified authentication logic to use password authentication when username doesn't contain "@", otherwise uses Entra ID
  • Updated all test mocks to include the new password parameter (passing null)

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
IPostgresService.cs Updated interface to add optional password parameter to all service methods
PostgresService.cs Implemented conditional authentication logic based on username format; added password parameter to methods
BasePostgresOptions.cs Added Password property to base options class
PostgresOptionDefinitions.cs Defined new password option for CLI
BaseDatabaseCommand.cs Registered password option and bound it in option parsing
DatabaseListCommand.cs Added password option registration and binding
TableSchemaGetCommand.cs Updated service call to pass password parameter
TableListCommand.cs Updated service call to pass password parameter
DatabaseQueryCommand.cs Updated service call to pass password parameter
TableSchemaGetCommandTests.cs Updated test mocks to include null password parameter
TableListCommandTests.cs Updated test mocks to include null password parameter
DatabaseQueryCommandTests.cs Updated test mocks to include null password parameter
DatabaseListCommandTests.cs Updated test mocks to include null password parameter
PostgresServiceParameterizedQueryTests.cs Updated test method calls to include null password parameter

@jumacabo jumacabo marked this pull request as draft October 30, 2025 12:31
@joshfree joshfree moved this from Untriaged to In Progress in Azure MCP Server Oct 31, 2025
@jumacabo jumacabo marked this pull request as ready for review October 31, 2025 17:29
@JasonYeMSFT
Copy link
Member

How does this change address the problem in the linked issue? In the linked issue, it says "This PostgreSQL server is configured with both PostgreSQL authentication and Microsoft Entra authentication methods." It doesn't seen that the root cause of the failure was missing PostgreSQL authentication support.

@xiangyan99
Copy link
Member

How does this change address the problem in the linked issue? In the linked issue, it says "This PostgreSQL server is configured with both PostgreSQL authentication and Microsoft Entra authentication methods." It doesn't seen that the root cause of the failure was missing PostgreSQL authentication support.

w/o the change, we don't really use the password provided by user. We always useEntra ID.

)
{
Description = $"The authentication type to access PostgreSQL server. " +
$"Supported values are '{AuthTypes.MicrosoftEntra}' or '{AuthTypes.PostgreSQL}'",
Copy link
Member

Choose a reason for hiding this comment

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

Should we make it optional and default to MicrosoftEntra?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working tools-Postgres PostgreSQL

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

4 participants