Skip to content

feat: support custom oauth & oidc providers#2357

Open
cemalkilic wants to merge 15 commits intomasterfrom
cemal/feat-custom-oauth-oidc-providers
Open

feat: support custom oauth & oidc providers#2357
cemalkilic wants to merge 15 commits intomasterfrom
cemal/feat-custom-oauth-oidc-providers

Conversation

@cemalkilic
Copy link
Contributor

Summary

Add configurable custom OAuth/OIDC providers (phase 1) so projects can integrate self‑hosted/regional identity providers without requiring code changes.

Problem

Current OAuth/OIDC providers are hardcoded, require provider-specific code and env vars, and block customers who need self‑hosted or custom IdPs (e.g. GitHub Enterprise, LINE, internal OIDC servers).

Solution

Introduce database‑backed oauth_providers with custom:{identifier} IDs, OIDC discovery + OAuth2 manual configuration, admin CRUD APIs, and tier‑gated quotas, reusing existing /authorize and /callback flows with JWT state + PKCE.

Things to review

How to encrypt client secrets?

@cemalkilic cemalkilic requested a review from a team as a code owner January 28, 2026 11:15
}

// Resolve hostname to IP addresses
ips, err := net.LookupIP(hostname)
Copy link
Contributor

Choose a reason for hiding this comment

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

🟠 Severity: HIGH

DNS Rebinding Attack (TOCTOU): The URL validation performs DNS resolution at validation time, but the actual HTTP request happens later. An attacker can exploit DNS rebinding by having their DNS server return a safe IP during validation, then quickly change to a private IP (e.g., 169.254.169.254) before the actual request. The ssrfProtectedTransport attempts to mitigate this but only re-validates the URL string, not the resolved IP. Consider implementing IP-pinning where validated IPs are cached and reused.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: To properly fix the DNS rebinding vulnerability, implement IP pinning: 1) Modify ValidateOAuthURL to return the resolved and validated IP addresses along with the error, 2) Update ssrfProtectedTransport to accept and store pinned IPs during validation, 3) Implement a custom net.Dialer in the transport that only connects to the pinned IPs (bypassing the hostname resolution). This ensures the HTTP connection uses the exact IPs that were validated, preventing DNS rebinding attacks. The implementation should resolve the hostname once during validation, validate all returned IPs, then configure the transport's DialContext to connect directly to one of those validated IPs using the Host header for TLS SNI.

@coveralls
Copy link

coveralls commented Jan 28, 2026

Pull Request Test Coverage Report for Build 21715046627

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 698 of 1189 (58.7%) changed or added relevant lines in 10 files are covered.
  • 96 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.5%) to 68.321%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/models/errors.go 2 5 40.0%
internal/api/provider/provider.go 16 25 64.0%
internal/api/token_oidc.go 1 29 3.45%
internal/utilities/url_validator.go 79 136 58.09%
internal/api/provider/custom_oauth.go 138 222 62.16%
internal/api/external.go 3 94 3.19%
internal/models/custom_oauth_provider.go 133 224 59.38%
internal/api/custom_oauth_admin.go 314 442 71.04%
Files with Coverage Reduction New Missed Lines %
internal/conf/configuration.go 4 99.34%
internal/models/refresh_token.go 6 70.25%
internal/observability/request-tracing.go 13 88.1%
internal/tokens/service.go 73 80.11%
Totals Coverage Status
Change from base Build 21407255753: -0.5%
Covered Lines: 15593
Relevant Lines: 22823

💛 - Coveralls

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.

2 participants