8000 feat(oauth2): add authorization server metadata endpoint and PKCE sup… · coder/coder@b50e322 · GitHub
[go: up one dir, main page]
Skip to content

Commit b50e322

Browse files
ThomasK33ThomasK33
committed
feat(oauth2): add authorization server metadata endpoint and PKCE support
- Add /.well-known/oauth-authorization-server metadata endpoint (RFC 8414) - Implement PKCE support with S256 method for enhanced security - Add resource parameter support (RFC 8707) for token binding - Add OAuth2-compliant error responses with proper error codes - Fix authorization UI to use POST-based consent instead of GET redirects - Add comprehensive OAuth2 test scripts and interactive test server - Update CLAUDE.md with OAuth2 development guidelines Database changes: - Add migration 000341: code_challenge, resource_uri, audience fields - Update audit table for new OAuth2 fields OAuth2 provider remains development-only (requires --dev flag). Change-Id: Ifbd0d9a543d545f9f56ecaa77ff2238542ff954a Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent 48bb534 commit b50e322

34 files changed

+1998
-242
lines changed

‎CLAUDE.md

Lines changed: 158 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,22 @@
22

33
Read [cursor rules](.cursorrules).
44

5+
## Quick Start Checklist for New Features
6+
7+
### Before Starting
8+
9+
- [ ] Run `git pull` to ensure you're on latest code
10+
- [ ] Check if feature touches database - you'll need migrations
11+
- [ ] Check if feature touches audit logs - update `enterprise/audit/table.go`
12+
13+
## Development Server
14+
15+
### Starting Development Mode
16+
17+
- Use `./scripts/develop.sh` to start Coder in development mode
18+
- This automatically builds and runs with `--dev` flag and proper access URL
19+
- Do NOT manually run `make build && ./coder server --dev` - use the script instead
20+
521
## Build/Test/Lint Commands
622

723
### Main Commands
@@ -34,6 +50,7 @@ Read [cursor rules](.cursorrules).
3450
- Use `gofumpt` for formatting
3551
- Create packages when used during implementation
3652
- Validate abstractions against implementations
53+
- **Test packages**: Use `package_test` naming (e.g., `identityprovider_test`) for black-box testing
3754

3855
### Error Handling
3956

@@ -63,11 +80,45 @@ Read [cursor rules](.cursorrules).
6380
- Keep message titles concise (~70 characters)
6481
- Use imperative, present tense in commit titles
6582

66-
## Database queries
83+
## Database Work
84+
85+
### Migration Guidelines
86+
87+
1. **Create migration files**:
88+
- Location: `coderd/database/migrations/`
89+
- Format: `{number}_{description}.{up|down}.sql`
90+
- Number must be unique and sequential
91+
- Always include both up and down migrations
92+
93+
2. **Update database queries**:
94+
- MUST DO! Any changes to database - adding queries, modifying queries should be done in the `coderd/database/queries/*.sql` files
95+
- MUST DO! Queries are grouped in files relating to context - e.g. `prebuilds.sql`, `users.sql`, `oauth2.sql`
96+
- After making changes to any `coderd/database/queries/*.sql` files you must run `make gen` to generate respective ORM changes
97+
98+
3. **Handle nullable fields**:
99+
- Use `sql.NullString`, `sql.NullBool`, etc. for optional database fields
100+
- Set `.Valid = true` when providing values
101+
- Example:
102+
103+
```go
104+
CodeChallenge: sql.NullString{
105+
String: params.codeChallenge,
106+
Valid: params.codeChallenge != "",
107+
}
108+
```
67109

68-
- MUST DO! Any changes to database - adding queries, modifying queries should be done in the `coderd\database\queries\*.sql` files. Use `make gen` to generate necessary changes after.
69-
- MUST DO! Queries are grouped in files relating to context - e.g. `prebuilds.sql`, `users.sql`, `provisionerjobs.sql`.
70-
- After making changes to any `coderd\database\queries\*.sql` files you must run `make gen` to generate respective ORM changes.
110+
4. **Audit table updates**:
111+
- If adding fields to auditable types, update `enterprise/audit/table.go`
112+
- Add each new field with appropriate action (ActionTrack, ActionIgnore, ActionSecret)
113+
- Run `make gen` to verify no audit errors
114+
115+
### Database Generation Process
116+
117+
1. Modify SQL files in `coderd/database/queries/`
118+
2. Run `make gen`
119+
3. If errors about audit table, update `enterprise/audit/table.go`
120+
4. Run `make gen` again
121+
5. Run `make lint` to catch any remaining issues
71122

72123
## Architecture
73124

@@ -78,6 +129,14 @@ Read [cursor rules](.cursorrules).
78129
- **Agents**: Services in remote workspaces providing features like SSH and port forwarding
79130
- **Workspaces**: Cloud resources defined by Terraform
80131

132+
### Adding New API Endpoints
133+
134+
1. **Define types** in `codersdk/` package
135+
2. **Add handler** in appropriate `coderd/` file
136+
3. **Register route** in `coderd/coderd.go`
137+
4. **Add tests** in `coderd/*_test.go` files
138+
5. **Update OpenAPI** by running `make gen`
139+
81140
## Sub-modules
82141

83142
### Template System
@@ -104,3 +163,98 @@ Read [cursor rules](.cursorrules).
104163
The frontend is contained in the site folder.
105164

106165
For building Frontend refer to [this document](docs/about/contributing/frontend.md)
166+
167+
## Common Patterns
168+
169+
### OAuth2/Authentication Work
170+
171+
- Types go in `codersdk/oauth2.go` or similar
172+
- Handlers go in `coderd/oauth2.go` or `coderd/identityprovider/`
173+
- Database fields need migration + audit table updates
174+
- Always support backward compatibility
175+
176+
## OAuth2 Development
177+
178+
### OAuth2 Provider Implementation
179+
180+
When working on OAuth2 provider features:
181+
182+
1. **OAuth2 Spec Compliance**:
183+
- Follow RFC 6749 for token responses
184+
- Use `expires_in` (seconds) not `expiry` (timestamp) in token responses
185+
- Return proper OAuth2 error format: `{"error": "code", "error_description": "details"}`
186+
187+
2. **Error Response Format**:
188+
- Create OAuth2-compliant error responses for token endpoint
189+
- Use standard error codes: `invalid_client`, `invalid_grant`, `invalid_request`
190+
- Avoid generic error responses for OAuth2 endpoints
191+
192+
3. **Testing OAuth2 Features**:
193+
- Use scripts in `./scripts/oauth2/` for testing
194+
- Run `./scripts/oauth2/test-mcp-oauth2.sh` for comprehensive tests
195+
- Manual testing: use `./scripts/oauth2/test-manual-flow.sh`
196+
197+
4. **PKCE Implementation**:
198+
- Support both with and without PKCE for backward compatibility
199+
- Use S256 method for code challenge
200+
- Properly validate code_verifier against stored code_challenge
201+
202+
5. **UI Authorization Flow**:
203+
- Use POST requests for consent, not GET with links
204+
- Avoid dependency on referer headers for security decisions
205+
- Support proper state parameter validation
206+
207+
### OAuth2 Error Handling Pattern
208+
209+
```go
210+
// Define specific OAuth2 errors
211+
var (
212+
errInvalidPKCE = xerrors.New("invalid code_verifier")
213+
)
214+
215+
// Use OAuth2-compliant error responses
216+
type OAuth2Error struct {
217+
Error string `json:"error"`
218+
ErrorDescription string `json:"error_description,omitempty"`
219+
}
220+
221+
// Return proper OAuth2 errors
222+
if errors.Is(err, errInvalidPKCE) {
223+
writeOAuth2Error(ctx, rw, http.StatusBadRequest, "invalid_grant", "The PKCE code verifier is invalid")
224+
return
225+
}
226+
```
227+
228+
### Testing Patterns
229+
230+
- Use table-driven tests for comprehensive coverage
231+
- Mock external dependencies
232+
- Test both positive and negative cases
233+
- Use `testutil.WaitLong` for timeouts in tests
234+
235+
## Testing Scripts
236+
237+
### OAuth2 Test Scripts
238+
239+
Located in `./scripts/oauth2/`:
240+
241+
- `test-mcp-oauth2.sh` - Full automated test suite
242+
- `setup-test-app.sh` - Create test OAuth2 app
243+
- `cleanup-test-app.sh` - Remove test app
244+
- `generate-pkce.sh` - Generate PKCE parameters
245+
- `test-manual-flow.sh` - Manual browser testing
246+
247+
Always run the full test suite after OAuth2 changes:
248+
249+
```bash
250+
./scripts/oauth2/test-mcp-oauth2.sh
251+
```
252+
253+
## Troubleshooting
254+
255+
### Common Issues
256+
257+
1. **"Audit table entry missing action"** - Update `enterprise/audit/table.go`
258+
2. **"package should be X_test"** - Use `package_test` naming for test files
259+
3. **SQL type errors** - Use `sql.Null*` types for nullable fields
260+
4. **Missing newlines** - Ensure files end with newline character

‎coderd/apidoc/docs.go

Lines changed: 125 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
0