Skip to content

auth: add OIDC token auth - #1820

Merged
methane merged 5 commits into
go-sql-driver:masterfrom
methane:oidc-token-auth
Oct 7, 2026
Merged

methane merged 5 commits into
go-sql-driver:masterfrom
methane:oidc-token-auth

Conversation

@methane

@methane methane commented Oct 6, 2026

Copy link
Copy Markdown
Member

Description

This pull request adds OpenID Connect (OIDC) authentication support to the MySQL driver for Go, allowing applications to authenticate using OIDC tokens. It introduces a new configuration option, enforces proper TLS usage, and updates error handling to ensure security and clarity when using OIDC. The README is updated with documentation and usage examples.

OpenID Connect (OIDC) Authentication:

  • Added support for OIDC authentication by introducing the OIDCToken option in the Config struct and a new openIDConnectAuthPlugin for handling OIDC tokens. [1] [2] [3]
  • Enforced that OIDC authentication requires driver-managed TLS and proper server verification, and added related error constants such as ErrOpenIDConnectTLS and ErrOpenIDConnectToken. [1] [2]

Connection and Authentication Flow:

  • Updated the connection logic to select the OIDC plugin when a token is present, check for required server capabilities, and reject authentication switches when using OIDC. [1] [2] [3] [4]

Security Improvements:

  • Redacted OIDC tokens from error messages to prevent accidental exposure in logs or error outputs.

Documentation:

  • Updated the README.md with a new section explaining OIDC authentication, including usage examples and security notes. [1] [2]

Constants and Imports:

  • Added the OIDC plugin name constant and necessary imports to support the new authentication flow. [1] [2]

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

@methane methane mentioned this pull request Oct 6, 2026
5 tasks done
@coveralls

coveralls commented Oct 6, 2026 •

Copy link
Copy Markdown

Coverage Status

coverage: 85.144% (-0.04%) from 85.179% — methane:oidc-token-auth into go-sql-driver:master

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 53a2395f-6439-4276-bcbc-f7f2f75728b8
📥 Commits

Reviewing files that changed from the base of the PR and between 6a73d88 and c6ea1fa.

📒 Files selected for processing (1)
  • oidc_test.go

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


Walkthrough

This change adds OpenID Connect token configuration and authentication over driver-managed TLS. It checks server capabilities, rejects authentication switches, redacts tokens from server error messages, and adds tests and README guidance for configuration and token refresh.

Changes

OpenID Connect authentication

Layer / File(s) Summary
Token configuration and usage
dsn.go, errors.go, README.md, oidc_test.go
Adds the OIDCToken option and related errors. The README describes configuration and token refresh. Tests cover configuration, cloning, DSN behavior, and refreshed tokens.
TLS authentication flow
const.go, connector.go, auth_oidc.go, oidc_test.go
Selects the OIDC plugin when a token is configured, requires driver-managed TLS and server capabilities, and sends the length-encoded token. Tests cover protocol framing and TLS validation.
Authentication response safeguards
auth.go, oidc_test.go
Rejects authentication-switch packets when an OIDC token is configured and redacts the token from MySQL server error messages. Tests cover malformed replies and authentication errors.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Config
  participant ConnectorConnect
  participant MySQLServer
  participant openIDConnectAuthPlugin
  Config->>ConnectorConnect: Provide OIDC token
  MySQLServer->>ConnectorConnect: Send greeting and capabilities
  ConnectorConnect->>openIDConnectAuthPlugin: Initialize with token and TLS context
  openIDConnectAuthPlugin->>ConnectorConnect: Return length-encoded token
  ConnectorConnect->>MySQLServer: Send authentication response
Loading

Merge Risk: ⚪ Minimal · up to c6ea1

This change adds OIDC token authentication guarded by TLS and server-capability checks. No concrete merge-blocking risk was identified in the supplied context. The author has not reported test results, so normal CI should still run before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c6ea1

OIDC authentication is explicitly enabled and prevents plaintext fallback, server-selected authentication switches, and direct token echoes in authentication errors. Server identity verification and token refresh remain application responsibilities. No concrete bypass was established in the inspected flow, but production verification policies and token permissions were not available.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The new sensitive sink is delivery of the application's bearer token to the configured database peer. If server verification is disabled without a replacement policy, an impersonating peer could receive that token. Potential reuse across services or tenants depends on token audiences, permissions, and downstream acceptance, which were not supplied.

Trust Boundaries and Controls

  • observed — The server's greeting cannot select OIDC or redirect a configured OIDC exchange into another authentication plugin. Token configuration selects the plugin explicitly, both required plugin capabilities are checked, switches are rejected, and unexpected continuations fail through simpleAuth.
  • observed — Verification-disabled TLS modes predate this PR. The new bearer-token path inherits those application-configurable policies; its documentation requires server authentication or proper replacement verification. This is not evidence that every accepted TLS configuration authenticates the intended server.

Resilience and Maintainability Implications

  • observed — NewConnector clones caller configuration, and callback-driven refresh uses another clone for each connection. Token replacement therefore does not change the connector's stored token through this normal flow. Concurrent token acquisition remains the application's responsibility.

Hardening Proposals

  • proposed — For deployments requiring a fail-closed verified-server contract, consider an explicit OIDC verification policy that rejects built-in verification-disabled TLS modes while accommodating deliberate custom verification. This would strengthen the current application-owned policy, not repair an established bypass of that documented contract.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: adding OIDC token authentication.
Description check ✅ Passed The description explains the OIDC authentication changes, security requirements, connection flow, error handling, and documentation updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@tokoko

tokoko commented Oct 6, 2026

Copy link
Copy Markdown

thanks for picking this up. one question I'm not sure about though: why can't the token be supplied through the DSN when the password can? Passwd is already parsed from and written back by ParseDSN/FormatDSN, and a short-lived JWT doesn't seem more sensitive than a long-lived password.

Without a DSN path, anything that reaches the driver through sql.Open("mysql", dsn) can't use OIDC at all: tools that only take a connection string, and wrappers that build a DSN internally. For example, my own case is the ADBC MySQL driver, which passes a DSN string around and round-trips it through FormatDSN, so it would need restructuring to use this.

@shogo82148

Copy link
Copy Markdown
Contributor

I think issuing a JWT in BeforeConnect is a reasonable design.
Since JWTs are short-lived tokens, they need to be generated right before establishing a connection with the server.

sql.Open("mysql", dsn) does not actually establish a connection with the server. Since the connection is actually established when a query is executed, some time may have passed since the dsn was constructed. In that case, the JWT would have expired.

@shogo82148 shogo82148 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.

LGTM

@methane
methane merged commit 032a849 into go-sql-driver:master Oct 7, 2026
27 checks passed
@methane
methane deleted the oidc-token-auth branch October 7, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants