Skip to content

Adding resource principal and instance principal auth methods for OCI Database MCP server#323

Open
shyam2511 wants to merge 1 commit into
oracle:mainfrom
shyam2511:ip-and-rp-auth
Open

Adding resource principal and instance principal auth methods for OCI Database MCP server#323
shyam2511 wants to merge 1 commit into
oracle:mainfrom
shyam2511:ip-and-rp-auth

Conversation

@shyam2511

@shyam2511 shyam2511 commented Jun 15, 2026

Copy link
Copy Markdown
Member

Database MCP server

Description

This change adds multiple authentication methods to the OCI Database MCP server:

  • Existing security-token authentication (default)
  • Instance Principal authentication (--auth-method instance_principal)
  • Resource Principal authentication (--auth-method resource_principal) using the RPST flow

It adds CLI arguments for auth selection and resource-principal values, branches OCI client initialization by auth method, and documents local and SSH-remote configuration patterns.

Resource-principal token and RPST exchange HTTP calls now use explicit connect/read timeouts to prevent startup from hanging indefinitely.

Version is bumped from 1.0.7 to 1.1.0.

Motivation and context

Previously, the server relied on security-token config-file authentication. This limited OCI-native deployments where instance principal or resource principal authentication is preferable for automated workloads.

This update supports those deployment patterns while retaining token authentication as the default.

Dependencies required for this change

  • cryptography — required for RPST request signing and session-key generation.
  • requests — required for resource-principal token retrieval and RPST exchange HTTP calls.

Validation

  • Added mocked unit coverage for token authentication, instance-principal signer construction, resource-principal initialization, RPST request construction, fallback behavior, endpoint failures, timeouts, missing inputs, and CLI/environment precedence.
  • make test project=oci-database-mcp-server passes: 160 tests, 92.94% coverage.
  • make lint passes.

Fixes #

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

@oracle-contributor-agreement oracle-contributor-agreement Bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jun 15, 2026
@krisrice

Copy link
Copy Markdown
Member

Automated review result: needs security/manual review.

Reason:

  • This PR changes authentication for the OCI Database MCP server by adding instance principal and a custom resource principal/RPST flow, including private key loading, token exchange calls, runtime auth selection, and new credential-related command-line inputs.

Manual review should verify:

  • Server-side authorization and least-privilege scope.
  • Parameter validation and bounded output/pagination.
  • Secret/customer-data redaction in logs, errors, and returned content.
  • Tests for auth failure, validation failure, side effects, and redaction where applicable.

No automated approval or merge was performed.

@dustin-sale

Copy link
Copy Markdown
Contributor

A few review items to address:

  • [P1] pyproject.toml, uv.lock — Rebase required before merge.

    • Evidence: GitHub reports the PR as dirty; a local merge-tree test against current main produced content conflicts in both files.
    • Impact: The PR cannot merge as-is.
    • Suggested direction: Rebase onto current main, resolve dependency/lockfile changes, then rerun validation.
  • [P1] server.py:382 and server.py:450 — RPST HTTP calls have no timeout.

    • Evidence: Resource-principal initialization runs before mcp.run(), but its token and RPST exchange requests omit a timeout.
    • Impact: An unreachable or stalled endpoint can hang MCP server startup indefinitely.
    • Suggested direction: Set explicit connect/read timeouts, handle timeout failures cleanly, and test them.
  • [P1] server.py auth implementation — No tests cover the new authentication behavior.

    • Evidence: The PR adds 337 lines to server.py but does not modify the unit-test file; local coverage lists the new RPST, instance-principal, and CLI paths as missed.
    • Impact: The feature’s primary flows—including signer construction, invalid input, endpoint failures, and CLI/env precedence—can regress unnoticed.
    • Suggested direction: Add mocked unit tests for each auth mode, RPST request construction/failures/timeouts, and main() argument handling.
  • [P2] pyproject.toml — PR description inaccurately says no direct dependencies were added.

    • Evidence: cryptography and requests are newly declared project dependencies.
    • Impact: Dependency, licensing, and release review lack an accurate PR summary.
    • Suggested direction: Correct the PR description and explain why each new dependency is required.

Key things are adding the timeouts and expanding test coverage for the new auth paths. The PR should also be updated to include the two new dependencies.

This is also a good opportunity to add a CHANGELOG.md for oci-database-mcp-server, doesn't look like there is one yet.

@dustin-sale dustin-sale left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Items have been addressed from my prior comment. PR approved.

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

Labels

OCA Verified All contributors have signed the Oracle Contributor Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants