coderd/oauth2_security_test.go
Rama main · réplica de lectura del repositorio original.
package coderd_test
import (
"errors"
"fmt"
"net/http"
"strings"
"sync"
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest"
"github.com/coder/coder/v2/codersdk"
)
// TestOAuth2ClientIsolation tests that OAuth2 clients cannot access other clients' data
func TestOAuth2ClientIsolation(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
_ = coderdtest.CreateFirstUser(t, client)
oauth2providertest.EnableDCR(t, client)
ctx := t.Context()
// Create two separate OAuth2 clients with unique identifiers
client1Name := fmt.Sprintf("test-client-1-%s-%d", t.Name(), time.Now().UnixNano())
client1Req := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://client1.example.com/callback"},
ClientName: client1Name,
ClientURI: "https://client1.example.com",
}
client1Resp, err := client.PostOAuth2ClientRegistration(ctx, client1Req)
require.NoError(t, err)
client2Name := fmt.Sprintf("test-client-2-%s-%d", t.Name(), time.Now().UnixNano())
client2Req := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://client2.example.com/callback"},
ClientName: client2Name,
ClientURI: "https://client2.example.com",
}
client2Resp, err := client.PostOAuth2ClientRegistration(ctx, client2Req)
require.NoError(t, err)
t.Run("ClientsCannotAccessOtherClientData", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
// Client 1 should not be able to access Client 2's data using Client 1's token
_, err := client.GetOAuth2ClientConfiguration(ctx, client2Resp.ClientID, client1Resp.RegistrationAccessToken)
require.Error(t, err)
var httpErr *codersdk.Error
require.ErrorAs(t, err, &httpErr)
require.Equal(t, http.StatusUnauthorized, httpErr.StatusCode())
// Client 2 should not be able to access Client 1's data using Client 2's token
_, err = client.GetOAuth2ClientConfiguration(ctx, client1Resp.ClientID, client2Resp.RegistrationAccessToken)
require.Error(t, err)
require.ErrorAs(t, err, &httpErr)
require.Equal(t, http.StatusUnauthorized, httpErr.StatusCode())
})
t.Run("ClientsCannotUpdateOtherClients", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
// Client 1 should not be able to update Client 2 using Client 1's token
updateReq := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://malicious.example.com/callback"},
ClientName: "Malicious Update",
}
_, err := client.PutOAuth2ClientConfiguration(ctx, client2Resp.ClientID, client1Resp.RegistrationAccessToken, updateReq)
require.Error(t, err)
var httpErr *codersdk.Error
require.ErrorAs(t, err, &httpErr)
require.Equal(t, http.StatusUnauthorized, httpErr.StatusCode())
})
t.Run("ClientsCannotDeleteOtherClients", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
// Client 1 should not be able to delete Client 2 using Client 1's token
err := client.DeleteOAuth2ClientConfiguration(ctx, client2Resp.ClientID, client1Resp.RegistrationAccessToken)
require.Error(t, err)
var httpErr *codersdk.Error
require.ErrorAs(t, err, &httpErr)
require.Equal(t, http.StatusUnauthorized, httpErr.StatusCode())
// Verify Client 2 still exists and is accessible with its own token
config, err := client.GetOAuth2ClientConfiguration(ctx, client2Resp.ClientID, client2Resp.RegistrationAccessToken)
require.NoError(t, err)
require.Equal(t, client2Resp.ClientID, config.ClientID)
})
}
// TestOAuth2RegistrationTokenSecurity tests security aspects of registration access tokens
func TestOAuth2RegistrationTokenSecurity(t *testing.T) {
t.Parallel()
// Single instance shared across all sub-tests. Each registers
// independent OAuth2 apps with unique client names.
client := coderdtest.New(t, nil)
_ = coderdtest.CreateFirstUser(t, client)
oauth2providertest.EnableDCR(t, client)
t.Run("InvalidTokenFormats", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
// Register a client to use for testing
clientName := fmt.Sprintf("test-client-%s-%d", t.Name(), time.Now().UnixNano())
regReq := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
ClientName: clientName,
}
regResp, err := client.PostOAuth2ClientRegistration(ctx, regReq)
require.NoError(t, err)
invalidTokens := []string{
"", // Empty token
"invalid", // Too short
"not-base64-!@#$%^&*", // Invalid characters
strings.Repeat("a", 1000), // Too long
"Bearer " + regResp.RegistrationAccessToken, // With Bearer prefix (incorrect)
}
for i, token := range invalidTokens {
t.Run(fmt.Sprintf("InvalidToken_%d", i), func(t *testing.T) {
t.Parallel()
_, err := client.GetOAuth2ClientConfiguration(ctx, regResp.ClientID, token)
require.Error(t, err)
var httpErr *codersdk.Error
require.ErrorAs(t, err, &httpErr)
require.Equal(t, http.StatusUnauthorized, httpErr.StatusCode())
})
}
})
t.Run("TokenNotReusableAcrossClients", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
// Register first client
client1Name := fmt.Sprintf("test-client-1-%s-%d", t.Name(), time.Now().UnixNano())
regReq1 := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
ClientName: client1Name,
}
regResp1, err := client.PostOAuth2ClientRegistration(ctx, regReq1)
require.NoError(t, err)
// Register another client
client2Name := fmt.Sprintf("test-client-2-%s-%d", t.Name(), time.Now().UnixNano())
regReq2 := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example2.com/callback"},
ClientName: client2Name,
}
regResp2, err := client.PostOAuth2ClientRegistration(ctx, regReq2)
require.NoError(t, err)
// Try to use client1's token on client2
_, err = client.GetOAuth2ClientConfiguration(ctx, regResp2.ClientID, regResp1.RegistrationAccessToken)
require.Error(t, err)
var httpErr *codersdk.Error
require.ErrorAs(t, err, &httpErr)
require.Equal(t, http.StatusUnauthorized, httpErr.StatusCode())
})
t.Run("TokenNotExposedInGETResponse", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
// Register a client
clientName := fmt.Sprintf("test-client-%s-%d", t.Name(), time.Now().UnixNano())
regReq := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
ClientName: clientName,
}
regResp, err := client.PostOAuth2ClientRegistration(ctx, regReq)
require.NoError(t, err)
// Get client configuration
config, err := client.GetOAuth2ClientConfiguration(ctx, regResp.ClientID, regResp.RegistrationAccessToken)
require.NoError(t, err)
// Registration access token should not be returned in GET responses (RFC 7592)
require.Empty(t, config.RegistrationAccessToken)
})
}
// TestOAuth2PrivilegeEscalation tests that clients cannot gain access through
// registration metadata.
func TestOAuth2PrivilegeEscalation(t *testing.T) {
t.Parallel()
t.Run("UpdateNarrowsScopeToCatalog", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
_ = coderdtest.CreateFirstUser(t, client)
oauth2providertest.EnableDCR(t, client)
ctx := t.Context()
// Register a basic client
clientName := fmt.Sprintf("test-client-%d", time.Now().UnixNano())
regReq := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
ClientName: clientName,
Scope: "workspace:read",
}
regResp, err := client.PostOAuth2ClientRegistration(ctx, regReq)
require.NoError(t, err)
updateReq := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
ClientName: clientName,
Scope: "workspace:read nosuch:admin",
}
updated, err := client.PutOAuth2ClientConfiguration(ctx, regResp.ClientID, regResp.RegistrationAccessToken, updateReq)
require.NoError(t, err)
require.Equal(t, "workspace:read", updated.Scope)
// An update that keeps no catalog name is rejected rather than stored
// as an empty allowlist, which would mean unrestricted.
updateReq.Scope = "nosuch:admin"
_, err = client.PutOAuth2ClientConfiguration(ctx, regResp.ClientID, regResp.RegistrationAccessToken, updateReq)
require.ErrorContains(t, err, "invalid_client_metadata")
require.ErrorContains(t, err, "unknown or unsupported scope")
require.ErrorContains(t, err, "nosuch:admin")
config, err := client.GetOAuth2ClientConfiguration(ctx, regResp.ClientID, regResp.RegistrationAccessToken)
require.NoError(t, err)
require.Equal(t, "workspace:read", config.Scope)
})
t.Run("CustomSchemeRedirectURIs", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
_ = coderdtest.CreateFirstUser(t, client)
oauth2providertest.EnableDCR(t, client)
ctx := t.Context()
// Test valid custom schemes per RFC 7591/8252
validCustomSchemeRequests := []codersdk.OAuth2ClientRegistrationRequest{
{
RedirectURIs: []string{"com.example.myapp://callback"},
ClientName: fmt.Sprintf("native-app-1-%d", time.Now().UnixNano()),
TokenEndpointAuthMethod: "none", // Required for public clients using custom schemes
},
{
RedirectURIs: []string{"com.example.app://oauth"},
ClientName: fmt.Sprintf("native-app-2-%d", time.Now().UnixNano()),
TokenEndpointAuthMethod: "none", // Required for public clients using custom schemes
},
{
RedirectURIs: []string{"urn:ietf:wg:oauth:2.0:oob"},
ClientName: fmt.Sprintf("native-app-3-%d", time.Now().UnixNano()),
TokenEndpointAuthMethod: "none", // Required for public clients
},
{
// Bare custom schemes (no reverse-domain notation) are the
// schemes real native apps register with the OS, and PKCE,
// not the scheme's spelling, is what secures the redirect.
RedirectURIs: []string{"vscode://coder.authenticate"},
ClientName: fmt.Sprintf("native-app-vscode-%d", time.Now().UnixNano()),
TokenEndpointAuthMethod: "none",
},
{
RedirectURIs: []string{"jetbrains://coder-callback"},
ClientName: fmt.Sprintf("native-app-jetbrains-%d", time.Now().UnixNano()),
TokenEndpointAuthMethod: "none",
},
}
for i, req := range validCustomSchemeRequests {
t.Run(fmt.Sprintf("ValidCustomSchemeRequest_%d", i), func(t *testing.T) {
t.Parallel()
_, err := client.PostOAuth2ClientRegistration(ctx, req)
// Valid custom schemes should be allowed per RFC 7591/8252
require.NoError(t, err)
})
}
// Test that dangerous schemes are properly rejected for security
dangerousSchemeRequests := []struct {
req codersdk.OAuth2ClientRegistrationRequest
scheme string
}{
{
req: codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"javascript:alert('test')"},
ClientName: fmt.Sprintf("native-app-js-%d", time.Now().UnixNano()),
TokenEndpointAuthMethod: "none",
},
scheme: "javascript",
},
{
req: codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"data:text/html,<html></html>"},
ClientName: fmt.Sprintf("native-app-data-%d", time.Now().UnixNano()),
TokenEndpointAuthMethod: "none",
},
scheme: "data",
},
}
for _, test := range dangerousSchemeRequests {
t.Run(fmt.Sprintf("DangerousScheme_%s", test.scheme), func(t *testing.T) {
t.Parallel()
_, err := client.PostOAuth2ClientRegistration(ctx, test.req)
// Dangerous schemes should be rejected for security
require.Error(t, err)
require.Contains(t, err.Error(), "dangerous scheme")
})
}
// mailto, tel, and sms are not in the dangerous-scheme blocklist
// above: they hand off to a mail client, dialer, or SMS app rather
// than injecting content, so they are harmless for a confidential
// client's redirect. A public client has no secret, so the redirect
// URI's scheme is its only mechanism for regaining control, and
// none of these three return control to it the way a real redirect
// scheme does. They are rejected for public clients specifically,
// with a distinct error from the dangerous-scheme case above.
publicClientDisallowedSchemeRequests := []struct {
req codersdk.OAuth2ClientRegistrationRequest
scheme string
}{
{
req: codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"mailto:user@example.com"},
ClientName: fmt.Sprintf("native-app-mailto-%d", time.Now().UnixNano()),
TokenEndpointAuthMethod: "none",
},
scheme: "mailto",
},
{
req: codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"tel:+15555550100"},
ClientName: fmt.Sprintf("native-app-tel-%d", time.Now().UnixNano()),
TokenEndpointAuthMethod: "none",
},
scheme: "tel",
},
{
req: codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"sms:+15555550100"},
ClientName: fmt.Sprintf("native-app-sms-%d", time.Now().UnixNano()),
TokenEndpointAuthMethod: "none",
},
scheme: "sms",
},
}
for _, test := range publicClientDisallowedSchemeRequests {
t.Run(fmt.Sprintf("PublicClientDisallowedScheme_%s", test.scheme), func(t *testing.T) {
t.Parallel()
_, err := client.PostOAuth2ClientRegistration(ctx, test.req)
require.Error(t, err)
require.Contains(t, err.Error(), "public clients may not use the "+test.scheme+" scheme")
})
}
})
}
// TestOAuth2InformationDisclosure tests that error messages don't leak sensitive information
func TestOAuth2InformationDisclosure(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
_ = coderdtest.CreateFirstUser(t, client)
oauth2providertest.EnableDCR(t, client)
ctx := t.Context()
// Register a client for testing
clientName := fmt.Sprintf("test-client-%d", time.Now().UnixNano())
regReq := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
ClientName: clientName,
}
regResp, err := client.PostOAuth2ClientRegistration(ctx, regReq)
require.NoError(t, err)
t.Run("ErrorsDoNotLeakClientSecrets", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
// Try various invalid operations and ensure they don't leak the client secret
_, err := client.GetOAuth2ClientConfiguration(ctx, regResp.ClientID, "invalid-token")
require.Error(t, err)
var httpErr *codersdk.Error
require.ErrorAs(t, err, &httpErr)
// Error message should not contain any part of the client secret or registration token
errorText := strings.ToLower(httpErr.Message + httpErr.Detail)
require.NotContains(t, errorText, strings.ToLower(regResp.ClientSecret))
require.NotContains(t, errorText, strings.ToLower(regResp.RegistrationAccessToken))
})
t.Run("ErrorsDoNotLeakDatabaseDetails", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
// Try to access non-existent client
_, err := client.GetOAuth2ClientConfiguration(ctx, "non-existent-client-id", regResp.RegistrationAccessToken)
require.Error(t, err)
var httpErr *codersdk.Error
require.ErrorAs(t, err, &httpErr)
// Error message should not leak database schema information
errorText := strings.ToLower(httpErr.Message + httpErr.Detail)
require.NotContains(t, errorText, "sql")
require.NotContains(t, errorText, "database")
require.NotContains(t, errorText, "table")
require.NotContains(t, errorText, "row")
require.NotContains(t, errorText, "constraint")
})
t.Run("ErrorsAreConsistentForInvalidClients", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
// Test with various invalid client IDs to ensure consistent error responses
invalidClientIDs := []string{
"non-existent-1",
"non-existent-2",
"totally-different-format",
}
var errorMessages []string
for _, clientID := range invalidClientIDs {
_, err := client.GetOAuth2ClientConfiguration(ctx, clientID, regResp.RegistrationAccessToken)
require.Error(t, err)
var httpErr *codersdk.Error
require.ErrorAs(t, err, &httpErr)
errorMessages = append(errorMessages, httpErr.Message)
}
// All error messages should be similar (not leaking which client IDs exist vs don't exist)
for i := 1; i < len(errorMessages); i++ {
require.Equal(t, errorMessages[0], errorMessages[i])
}
})
}
// TestOAuth2ConcurrentSecurityOperations tests security under concurrent operations
func TestOAuth2ConcurrentSecurityOperations(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
_ = coderdtest.CreateFirstUser(t, client)
oauth2providertest.EnableDCR(t, client)
ctx := t.Context()
// Register a client for testing
clientName := fmt.Sprintf("test-client-%d", time.Now().UnixNano())
regReq := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
ClientName: clientName,
}
regResp, err := client.PostOAuth2ClientRegistration(ctx, regReq)
require.NoError(t, err)
t.Run("ConcurrentAccessAttempts", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
const numGoroutines = 20
var wg sync.WaitGroup
errors := make([]error, numGoroutines)
// Launch concurrent attempts to access the client configuration
for i := 0; i < numGoroutines; i++ {
wg.Go(func() {
_, err := client.GetOAuth2ClientConfiguration(ctx, regResp.ClientID, regResp.RegistrationAccessToken)
errors[i] = err
})
}
wg.Wait()
// All requests should succeed (they're all valid)
for i, err := range errors {
require.NoError(t, err, "Request %d failed", i)
}
})
t.Run("ConcurrentInvalidAccessAttempts", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
const numGoroutines = 20
var wg sync.WaitGroup
statusCodes := make([]int, numGoroutines)
// Launch concurrent attempts with invalid tokens
for i := 0; i < numGoroutines; i++ {
wg.Go(func() {
_, err := client.GetOAuth2ClientConfiguration(ctx, regResp.ClientID, fmt.Sprintf("invalid-token-%d", i))
if err == nil {
t.Errorf("Expected error for goroutine %d", i)
return
}
var httpErr *codersdk.Error
if !errors.As(err, &httpErr) {
t.Errorf("Expected codersdk.Error for goroutine %d", i)
return
}
statusCodes[i] = httpErr.StatusCode()
})
}
wg.Wait()
// All requests should fail with 401 status
for i, statusCode := range statusCodes {
require.Equal(t, http.StatusUnauthorized, statusCode, "Request %d had unexpected status", i)
}
})
t.Run("ConcurrentClientDeletion", func(t *testing.T) {
t.Parallel()
ctx := t.Context()
// Register a client specifically for deletion testing
deleteClientName := fmt.Sprintf("delete-test-client-%d", time.Now().UnixNano())
deleteRegReq := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://delete-test.example.com/callback"},
ClientName: deleteClientName,
}
deleteRegResp, err := client.PostOAuth2ClientRegistration(ctx, deleteRegReq)
require.NoError(t, err)
const numGoroutines = 5
var wg sync.WaitGroup
deleteResults := make([]error, numGoroutines)
// Launch concurrent deletion attempts
for i := 0; i < numGoroutines; i++ {
wg.Go(func() {
err := client.DeleteOAuth2ClientConfiguration(ctx, deleteRegResp.ClientID, deleteRegResp.RegistrationAccessToken)
deleteResults[i] = err
})
}
wg.Wait()
// Only one deletion should succeed, others should fail
successCount := 0
for _, err := range deleteResults {
if err == nil {
successCount++
}
}
// At least one should succeed, and multiple successes are acceptable (idempotent operation)
require.Greater(t, successCount, 0, "At least one deletion should succeed")
// Verify the client is actually deleted
_, err = client.GetOAuth2ClientConfiguration(ctx, deleteRegResp.ClientID, deleteRegResp.RegistrationAccessToken)
require.Error(t, err)
var httpErr *codersdk.Error
require.ErrorAs(t, err, &httpErr)
require.True(t, httpErr.StatusCode() == http.StatusUnauthorized || httpErr.StatusCode() == http.StatusNotFound)
})
}