feat: support configuring the linuxdo endpoint via environment variables - #2237
Conversation
WalkthroughThe changes externalize hard-coded Linux DO OAuth endpoints to environment variables. Two new configuration options are added to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.env.example (1)
70-72: Maintain consistency with the file's commenting pattern.All other optional environment variables in this file are commented out (lines 1-69). The new Linux DO configuration should follow the same pattern to remain consistent.
Apply this diff:
# LinuxDo相关配置 -LINUX_DO_TOKEN_ENDPOINT=https://connect.linux.do/oauth2/token -LINUX_DO_USER_ENDPOINT=https://connect.linux.do/api/user +# LINUX_DO_TOKEN_ENDPOINT=https://connect.linux.do/oauth2/token +# LINUX_DO_USER_ENDPOINT=https://connect.linux.do/api/user
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.example(1 hunks)controller/linuxdo.go(2 hunks)
🔇 Additional comments (1)
controller/linuxdo.go (1)
132-132: Implementation looks good.The user endpoint configuration follows the same pattern as the token endpoint (line 87). The default value is appropriate and matches the configuration in
.env.example.
|
|
||
| // Get access token using Basic auth | ||
| tokenEndpoint := "https://connect.linux.do/oauth2/token" | ||
| tokenEndpoint := common.GetEnvOrDefaultString("LINUX_DO_TOKEN_ENDPOINT", "https://connect.linux.do/oauth2/token") |
There was a problem hiding this comment.
🧩 Analysis chain
Consider the security implications of configurable OAuth endpoints.
The implementation correctly externalizes the token endpoint. However, allowing OAuth endpoints to be configured via environment variables means that anyone with access to modify these variables could redirect authentication flows to malicious servers.
This is acceptable for deployment-time configuration in trusted environments, but ensure that:
- Access to environment variables is restricted to authorized personnel
- Documentation warns about the security implications
- Consider adding URL validation to ensure HTTPS is used
🌐 Web query:
Are there security best practices for configurable OAuth endpoints in Go applications?
💡 Result:
Yes — treat configurable OAuth endpoints as a high-risk configuration and apply defensive controls. Concise checklist for Go apps (with concrete mitigations):
- Require HTTPS and validate TLS (no plain HTTP). Enforce modern TLS ciphers and validate certs on outgoing requests. [1]
- Whitelist endpoints — do not accept arbitrary user-supplied discovery/redirect/resource URLs. Match redirect_uri exactly (no wildcards or relaxed rules). Store allowed endpoints in config or DB and validate at runtime. [1][2]
- Avoid untrusted dynamic discovery/registration unless the issuer is trusted; if you support it, restrict which hosts can be discovered and sanitize all returned endpoints to prevent SSRF, open-redirects, and unexpected callbacks. (Malicious-endpoint attacks are documented.) [3]
- Defend against SSRF/SSDT: validate/normalize URLs, restrict outbound targets by allowlist (hosts/IPs), block private-network IPs, and set short HTTP timeouts and size limits for requests to configured endpoints. [2][3]
- Enforce OAuth best practices: use Authorization Code + PKCE for public clients, short-lived access tokens, refresh-token rotation, scope least-privilege, and token introspection/revocation where appropriate. [1][2]
- Protect client credentials and tokens: store secrets in secure vaults or environment variables (never in code/repo), mark cookies HttpOnly/Secure/SameSite when used, avoid logging tokens, and rotate secrets periodically. [1][2]
- Authenticate and authorize endpoint changes: require strong admin auth/MFA, use RBAC for config edits, log and alert on configuration changes, and require review/approval for new endpoints. [2]
- Validate and sanitize all input returned by endpoints (JSON metadata, JWKS URLs, redirect URIs); verify JWT issuer/audience/signature and enforce expected token type and audience. Cache and validate JWKs with expiry/rotation checks. [1][2]
- Rate-limit and monitor: rate-limit auth/token endpoints and outbound discovery calls; collect telemetry and alert on unusual patterns (mass discovery, repeated failures, token reuse). [2]
- Use well-maintained libraries and follow platform guidance (e.g., golang.org/x/oauth2); keep dependencies up to date and audit them. Add unit/integration tests for config validation and adversarial endpoint values. [4]
Key references:
- RFC 6819 — OAuth 2.0 Threat Model & Security Considerations. [1]
- OWASP / API security guidance (whitelisting, SSRF, config hardening, least privilege). [2]
- "Malicious Endpoints" attacks on OIDC/OpenID Connect (discovery/dynamic registration risks). [3]
- Go oauth2 package repo / docs (use maintained libs). [4]
If you want, I can: (a) translate these into a Go implementation checklist/code snippets (URL validation, allowlist enforcement, SSRF-safe HTTP client), or (b) review a sample config file and point out issues. Which would you prefer?
🏁 Script executed:
#!/bin/bash
# Examine controller/linuxdo.go for OAuth endpoint handling
echo "=== Checking lines 70-150 in controller/linuxdo.go ==="
cat controller/linuxdo.go | head -150 | tail -80Length of output: 2318
🏁 Script executed:
#!/bin/bash
# Check GetEnvOrDefaultString implementation in common package
echo "=== Finding GetEnvOrDefaultString implementation ==="
find . -type f -name "*.go" | xargs grep -l "GetEnvOrDefaultString" | head -5
echo ""
echo "=== Common package location ==="
find . -path "*/common/*.go" -type f | head -3Length of output: 230
🏁 Script executed:
#!/bin/bash
# Examine GetEnvOrDefaultString implementation
echo "=== common/env.go content ==="
cat common/env.goLength of output: 903
Add URL validation and HTTPS enforcement for OAuth endpoints.
The implementation exposes a critical security vulnerability: configurable OAuth endpoints are accepted from environment variables with no validation. This allows anyone with env var access to redirect authentication flows to malicious servers and intercept credentials.
Required fixes (lines 87 & 132 in controller/linuxdo.go):
- Enforce HTTPS for all endpoints (reject
http://) - Validate endpoint URLs match an allowlist of trusted hosts (e.g.,
connect.linux.doonly) - Add SSRF defenses: block requests to private/internal IPs (127.0.0.1, 10.0.0.0/8, etc.)
This aligns with OAuth 2.0 security best practices (RFC 6819) and OWASP guidance. Deployment flexibility should not come at the cost of credential theft vulnerability.
🤖 Prompt for AI Agents
In controller/linuxdo.go around lines 87 and 132, the environment-provided OAuth
endpoints are used without validation; update both places to (1) parse the
environment value as a URL and require scheme == "https", (2) enforce an
allowlist for the host (exact match or safe subdomain policy, e.g.
"connect.linux.do" only), (3) reject raw IP literals and perform DNS resolution
of the hostname and validate all resolved IPs are not in private, loopback,
link-local, or other reserved ranges (e.g. 127.0.0.0/8, 10.0.0.0/8,
172.16.0.0/12, 192.168.0.0/16, ::1, fc00::/7, fe80::/10), and (4) return an
error (and fail startup or skip the override) when any of these checks fail so
only HTTPS allowlisted public endpoints are accepted.
…ttings feat: support configuring the linuxdo endpoint via environment variables
…ttings feat: support configuring the linuxdo endpoint via environment variables
2 participants
#2197
Summary by CodeRabbit