mirror of
https://github.com/coder/coder.git
synced 2026-11-10 03:02:26 +08:00
## Summary
This PR implements critical MCP OAuth2 compliance features for Coder's authorization server, adding PKCE support, resource parameter handling, and OAuth2 server metadata discovery. This brings Coder's OAuth2 implementation significantly closer to production readiness for MCP (Model Context Protocol)
integrations.
## What's Added
### OAuth2 Authorization Server Metadata (RFC 8414)
- Add `/.well-known/oauth-authorization-server` endpoint for automatic client discovery
- Returns standardized metadata including supported grant types, response types, and PKCE methods
- Essential for MCP client compatibility and OAuth2 standards compliance
### PKCE Support (RFC 7636)
- Implement Proof Key for Code Exchange with S256 challenge method
- Add `code_challenge` and `code_challenge_method` parameters to authorization flow
- Add `code_verifier` validation in token exchange
- Provides enhanced security for public clients (mobile apps, CLIs)
### Resource Parameter Support (RFC 8707)
- Add `resource` parameter to authorization and token endpoints
- Store resource URI and bind tokens to specific audiences
- Critical for MCP's resource-bound token model
### Enhanced OAuth2 Error Handling
- Add OAuth2-compliant error responses with proper error codes
- Use standard error format: `{"error": "code", "error_description": "details"}`
- Improve error consistency across OAuth2 endpoints
### Authorization UI Improvements
- Fix authorization flow to use POST-based consent instead of GET redirects
- Remove dependency on referer headers for security decisions
- Improve CSRF protection with proper state parameter validation
## Why This Matters
**For MCP Integration:** MCP requires OAuth2 authorization servers to support PKCE, resource parameters, and metadata discovery. Without these features, MCP clients cannot securely authenticate with Coder.
**For Security:** PKCE prevents authorization code interception attacks, especially critical for public clients. Resource binding ensures tokens are only valid for intended services.
**For Standards Compliance:** These are widely adopted OAuth2 extensions that improve interoperability with modern OAuth2 clients.
## Database Changes
- **Migration 000343:** Adds `code_challenge`, `code_challenge_method`, `resource_uri` to `oauth2_provider_app_codes`
- **Migration 000343:** Adds `audience` field to `oauth2_provider_app_tokens` for resource binding
- **Audit Updates:** New OAuth2 fields properly tracked in audit system
- **Backward Compatibility:** All changes maintain compatibility with existing OAuth2 flows
## Test Coverage
- Comprehensive PKCE test suite in `coderd/identityprovider/pkce_test.go`
- OAuth2 metadata endpoint tests in `coderd/oauth2_metadata_test.go`
- Integration tests covering PKCE + resource parameter combinations
- Negative tests for invalid PKCE verifiers and malformed requests
## Testing Instructions
```bash
# Run the comprehensive OAuth2 test suite
./scripts/oauth2/test-mcp-oauth2.sh
Manual Testing with Interactive Server
# Start Coder in development mode
./scripts/develop.sh
# In another terminal, set up test app and run interactive flow
eval $(./scripts/oauth2/setup-test-app.sh)
./scripts/oauth2/test-manual-flow.sh
# Opens browser with OAuth2 flow, handles callback automatically
# Clean up when done
./scripts/oauth2/cleanup-test-app.sh
Individual Component Testing
# Test metadata endpoint
curl -s http://localhost:3000/.well-known/oauth-authorization-server | jq .
# Test PKCE generation
./scripts/oauth2/generate-pkce.sh
# Run specific test suites
go test -v ./coderd/identityprovider -run TestVerifyPKCE
go test -v ./coderd -run TestOAuth2AuthorizationServerMetadata
```
### Breaking Changes
None. All changes maintain backward compatibility with existing OAuth2 flows.
---
Change-Id: Ifbd0d9a543d545f9f56ecaa77ff2238542ff954a
Signed-off-by: Thomas Kosiewski <tk@coder.com>
8.7 KiB
8.7 KiB
Coder Development Guidelines
Read cursor rules.
Quick Start Checklist for New Features
Before Starting
- Run
git pullto ensure you're on latest code - Check if feature touches database - you'll need migrations
- Check if feature touches audit logs - update
enterprise/audit/table.go
Development Server
Starting Development Mode
- Use
./scripts/develop.shto start Coder in development mode - This automatically builds and runs with
--devflag and proper access URL - Do NOT manually run
make build && ./coder server --dev- use the script instead
Build/Test/Lint Commands
Main Commands
make buildormake build-fat- Build all "fat" binaries (includes "server" functionality)make build-slim- Build "slim" binariesmake test- Run Go testsmake test RUN=TestFunctionNameorgo test -v ./path/to/package -run TestFunctionName- Test singlemake test-postgres- Run tests with Postgres databasemake test-race- Run tests with Go race detectormake test-e2e- Run end-to-end testsmake lint- Run all lintersmake fmt- Format all codemake gen- Generates mocks, database queries and other auto-generated files
Frontend Commands (site directory)
pnpm build- Build frontendpnpm dev- Run development serverpnpm check- Run code checkspnpm format- Format frontend codepnpm lint- Lint frontend codepnpm test- Run frontend tests
Code Style Guidelines
Go
- Follow Effective Go and Go's Code Review Comments
- Use
gofumptfor formatting - Create packages when used during implementation
- Validate abstractions against implementations
- Test packages: Use
package_testnaming (e.g.,identityprovider_test) for black-box testing
Error Handling
- Use descriptive error messages
- Wrap errors with context
- Propagate errors appropriately
- Use proper error types
- (
xerrors.Errorf("failed to X: %w", err))
Naming
- Use clear, descriptive names
- Abbreviate only when obvious
- Follow Go and TypeScript naming conventions
Comments
- Document exported functions, types, and non-obvious logic
- Follow JSDoc format for TypeScript
- Use godoc format for Go code
Commit Style
- Follow Conventional Commits 1.0.0
- Format:
type(scope): message - Types:
feat,fix,docs,style,refactor,test,chore - Keep message titles concise (~70 characters)
- Use imperative, present tense in commit titles
Database Work
Migration Guidelines
-
Create migration files:
- Location:
coderd/database/migrations/ - Format:
{number}_{description}.{up|down}.sql - Number must be unique and sequential
- Always include both up and down migrations
- Location:
-
Update database queries:
- MUST DO! Any changes to database - adding queries, modifying queries should be done in the
coderd/database/queries/*.sqlfiles - MUST DO! Queries are grouped in files relating to context - e.g.
prebuilds.sql,users.sql,oauth2.sql - After making changes to any
coderd/database/queries/*.sqlfiles you must runmake gento generate respective ORM changes
- MUST DO! Any changes to database - adding queries, modifying queries should be done in the
-
Handle nullable fields:
-
Use
sql.NullString,sql.NullBool, etc. for optional database fields -
Set
.Valid = truewhen providing values -
Example:
CodeChallenge: sql.NullString{ String: params.codeChallenge, Valid: params.codeChallenge != "", }
-
-
Audit table updates:
- If adding fields to auditable types, update
enterprise/audit/table.go - Add each new field with appropriate action (ActionTrack, ActionIgnore, ActionSecret)
- Run
make gento verify no audit errors
- If adding fields to auditable types, update
-
In-memory database (dbmem) updates:
- When adding new fields to database structs, ensure
dbmemimplementation copies all fields - Check
coderd/database/dbmem/dbmem.gofor Insert/Update methods - Missing fields in dbmem can cause tests to fail even if main implementation is correct
- When adding new fields to database structs, ensure
Database Generation Process
- Modify SQL files in
coderd/database/queries/ - Run
make gen - If errors about audit table, update
enterprise/audit/table.go - Run
make genagain - Run
make lintto catch any remaining issues
Architecture
Core Components
- coderd: Main API service connecting workspaces, provisioners, and users
- provisionerd: Execution context for infrastructure-modifying providers
- Agents: Services in remote workspaces providing features like SSH and port forwarding
- Workspaces: Cloud resources defined by Terraform
Adding New API Endpoints
- Define types in
codersdk/package - Add handler in appropriate
coderd/file - Register route in
coderd/coderd.go - Add tests in
coderd/*_test.gofiles - Update OpenAPI by running
make gen
Sub-modules
Template System
- Templates define infrastructure for workspaces using Terraform
- Environment variables pass context between Coder and templates
- Official modules extend development environments
RBAC System
- Permissions defined at site, organization, and user levels
- Object-Action model protects resources
- Built-in roles: owner, member, auditor, templateAdmin
- Permission format:
<sign>?<level>.<object>.<id>.<action>
Database
- PostgreSQL 13+ recommended for production
- Migrations managed with
migrate - Database authorization through
dbauthzpackage
Frontend
The frontend is contained in the site folder.
For building Frontend refer to this document
Common Patterns
OAuth2/Authentication Work
- Types go in
codersdk/oauth2.goor similar - Handlers go in
coderd/oauth2.goorcoderd/identityprovider/ - Database fields need migration + audit table updates
- Always support backward compatibility
OAuth2 Development
OAuth2 Provider Implementation
When working on OAuth2 provider features:
-
OAuth2 Spec Compliance:
- Follow RFC 6749 for token responses
- Use
expires_in(seconds) notexpiry(timestamp) in token responses - Return proper OAuth2 error format:
{"error": "code", "error_description": "details"}
-
Error Response Format:
- Create OAuth2-compliant error responses for token endpoint
- Use standard error codes:
invalid_client,invalid_grant,invalid_request - Avoid generic error responses for OAuth2 endpoints
-
Testing OAuth2 Features:
- Use scripts in
./scripts/oauth2/for testing - Run
./scripts/oauth2/test-mcp-oauth2.shfor comprehensive tests - Manual testing: use
./scripts/oauth2/test-manual-flow.sh
- Use scripts in
-
PKCE Implementation:
- Support both with and without PKCE for backward compatibility
- Use S256 method for code challenge
- Properly validate code_verifier against stored code_challenge
-
UI Authorization Flow:
- Use POST requests for consent, not GET with links
- Avoid dependency on referer headers for security decisions
- Support proper state parameter validation
OAuth2 Error Handling Pattern
// Define specific OAuth2 errors
var (
errInvalidPKCE = xerrors.New("invalid code_verifier")
)
// Use OAuth2-compliant error responses
type OAuth2Error struct {
Error string `json:"error"`
ErrorDescription string `json:"error_description,omitempty"`
}
// Return proper OAuth2 errors
if errors.Is(err, errInvalidPKCE) {
writeOAuth2Error(ctx, rw, http.StatusBadRequest, "invalid_grant", "The PKCE code verifier is invalid")
return
}
Testing Patterns
- Use table-driven tests for comprehensive coverage
- Mock external dependencies
- Test both positive and negative cases
- Use
testutil.WaitLongfor timeouts in tests
Testing Scripts
OAuth2 Test Scripts
Located in ./scripts/oauth2/:
test-mcp-oauth2.sh- Full automated test suitesetup-test-app.sh- Create test OAuth2 appcleanup-test-app.sh- Remove test appgenerate-pkce.sh- Generate PKCE parameterstest-manual-flow.sh- Manual browser testing
Always run the full test suite after OAuth2 changes:
./scripts/oauth2/test-mcp-oauth2.sh
Troubleshooting
Common Issues
- "Audit table entry missing action" - Update
enterprise/audit/table.go - "package should be X_test" - Use
package_testnaming for test files - SQL type errors - Use
sql.Null*types for nullable fields - Missing newlines - Ensure files end with newline character
- Tests passing locally but failing in CI - Check if
dbmemimplementation needs updating - OAuth2 endpoints returning wrong error format - Ensure OAuth2 endpoints return RFC 6749 compliant errors