Merge pull request #1025 from trheyi/main

Enhance OAuth functionality with registration tests and response impr…

openapi/oauth.go

@@ -178,8 +178,8 @@ func (openapi *OpenAPI) oauthRegister(c *gin.Context) {
 		return
 	}
 
-	// Return the registration response
+	// Return the registration response directly (RFC 7591 compliant)
-	openapi.respondWithSuccess(c, StatusCreated, res)
+	openapi.respondWithOAuthDirect(c, StatusCreated, res)
 }
 
 // oauthGetClient retrieves client configuration - RFC 7592
@@ -323,7 +323,7 @@ func (openapi *OpenAPI) handleAuthorizationCodeGrant(c *gin.Context) {
 	}
 
 	// Use OAuth 2.1 compliant response
-	openapi.respondWithOAuth21TokenSuccess(c, token)
+	openapi.respondWithTokenSuccess(c, token)
 }
 
 func (openapi *OpenAPI) handleRefreshTokenGrant(c *gin.Context) {
@@ -345,7 +345,7 @@ func (openapi *OpenAPI) handleRefreshTokenGrant(c *gin.Context) {
 		Scope:        "openid profile email",
 	}
 
-	openapi.respondWithOAuth21TokenSuccess(c, response)
+	openapi.respondWithTokenSuccess(c, response)
 }
 
 func (openapi *OpenAPI) handleClientCredentialsGrant(c *gin.Context) {
@@ -362,7 +362,7 @@ func (openapi *OpenAPI) handleClientCredentialsGrant(c *gin.Context) {
 		Scope:       scope,
 	}
 
-	openapi.respondWithOAuth21TokenSuccess(c, token)
+	openapi.respondWithTokenSuccess(c, token)
 }
 
 func (openapi *OpenAPI) handleDeviceCodeGrant(c *gin.Context) {

openapi/oauth_test.go

@@ -1 +1,116 @@
 package openapi
+
+import (
+	"bytes"
+	"encoding/json"
+	"io"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/yaoapp/yao/openapi/oauth/types"
+)
+
+func TestOAuthRegister(t *testing.T) {
+	serverURL := Prepare(t)
+	defer Clean()
+
+	// Debug: Check if Server is properly initialized
+	if Server == nil {
+		t.Fatal("OpenAPI Server is nil")
+	}
+
+	if Server.Config == nil {
+		t.Fatal("OpenAPI Server.Config is nil")
+	}
+
+	if Server.OAuth == nil {
+		t.Fatal("OpenAPI Server.OAuth is nil")
+	}
+
+	t.Logf("Server initialized with BaseURL: %s", Server.Config.BaseURL)
+
+	// Get base URL from server config
+	baseURL := ""
+	if Server != nil && Server.Config != nil {
+		baseURL = Server.Config.BaseURL
+	}
+
+	endpoint := serverURL + baseURL + "/oauth/register"
+	t.Logf("Testing endpoint: %s", endpoint)
+
+	t.Run("Valid Client Registration", func(t *testing.T) {
+		// Minimal valid registration request to isolate the issue
+		req := types.DynamicClientRegistrationRequest{
+			RedirectURIs: []string{
+				"http://localhost/callback",
+			},
+			ClientName: "Test Client",
+		}
+
+		// Convert to JSON
+		jsonData, err := json.Marshal(req)
+		assert.NoError(t, err)
+		t.Logf("Request JSON: %s", string(jsonData))
+
+		// Make POST request
+		t.Logf("Making POST request to: %s", endpoint)
+		resp, err := http.Post(endpoint, "application/json", bytes.NewBuffer(jsonData))
+		assert.NoError(t, err)
+		assert.NotNil(t, resp)
+		defer resp.Body.Close()
+
+		t.Logf("Response status code: %d", resp.StatusCode)
+
+		// Verify OAuth 2.1 security headers are present
+		assert.Equal(t, "no-store", resp.Header.Get("Cache-Control"), "Cache-Control header should be set")
+		assert.Equal(t, "no-cache", resp.Header.Get("Pragma"), "Pragma header should be set")
+		assert.Equal(t, "nosniff", resp.Header.Get("X-Content-Type-Options"), "X-Content-Type-Options header should be set")
+		assert.Equal(t, "DENY", resp.Header.Get("X-Frame-Options"), "X-Frame-Options header should be set")
+		assert.Equal(t, "no-referrer", resp.Header.Get("Referrer-Policy"), "Referrer-Policy header should be set")
+		assert.Equal(t, "application/json;charset=UTF-8", resp.Header.Get("Content-Type"), "Content-Type header should be set")
+
+		// Read the complete response body for debugging
+		bodyBytes, _ := io.ReadAll(resp.Body)
+		t.Logf("Complete response body: %s", string(bodyBytes))
+
+		// Reset the response body for JSON decoding
+		resp.Body = io.NopCloser(bytes.NewReader(bodyBytes))
+
+		// Check status code
+		if resp.StatusCode != http.StatusCreated {
+			// Read error response for debugging
+			body, _ := io.ReadAll(resp.Body)
+			t.Logf("Expected 201, got %d. Response: %s", resp.StatusCode, string(body))
+		}
+		assert.Equal(t, http.StatusCreated, resp.StatusCode)
+
+		// Parse response
+		t.Logf("Parsing response body...")
+		var response types.DynamicClientRegistrationResponse
+		err = json.NewDecoder(resp.Body).Decode(&response)
+		if err != nil {
+			t.Logf("Failed to decode response: %v", err)
+		}
+		assert.NoError(t, err)
+
+		t.Logf("Response ClientID: %s", response.ClientID)
+		t.Logf("Response ClientSecret: %s", response.ClientSecret)
+
+		// Verify response contains generated client credentials
+		assert.NotEmpty(t, response.ClientID)
+		assert.NotEmpty(t, response.ClientSecret)
+
+		// Verify request data is preserved in response
+		if response.DynamicClientRegistrationRequest != nil {
+			assert.Equal(t, req.ClientName, response.DynamicClientRegistrationRequest.ClientName)
+			assert.Equal(t, req.RedirectURIs, response.DynamicClientRegistrationRequest.RedirectURIs)
+
+			// Verify that default values were applied when not specified in request
+			assert.NotEmpty(t, response.DynamicClientRegistrationRequest.GrantTypes, "Server should apply default grant types")
+			assert.NotEmpty(t, response.DynamicClientRegistrationRequest.ResponseTypes, "Server should apply default response types")
+			assert.Equal(t, "web", response.DynamicClientRegistrationRequest.ApplicationType, "Server should apply default application type")
+			assert.Equal(t, "client_secret_basic", response.DynamicClientRegistrationRequest.TokenEndpointAuthMethod, "Server should apply default auth method")
+		}
+	})
+}

openapi/openapi_test.go

@@ -23,6 +23,15 @@ var testServer *http.Server
 // All tests in the openapi package MUST use these utility functions for proper test environment setup.
 // This is a preparation utility function, NOT an actual test case.
 //
+// TESTING GUIDELINES FOR AI ASSISTANTS:
+// 1. DO NOT modify configuration files (openapi.yao, app.yao, etc.) to make tests pass
+// 2. DO NOT bypass validation or security checks to make tests pass
+// 3. If tests fail, investigate the root cause - it may be a real program bug that needs fixing
+// 4. Tests should verify actual functionality, not just pass assertions
+// 5. Use realistic test data that represents real-world usage scenarios
+// 6. When tests fail, check: environment setup, missing dependencies, configuration issues, actual code bugs
+// 7. Fix the underlying issue in the code, not the test or configuration
+//
 // Usage pattern for ALL openapi tests:
 //
 //	func TestYourFunction(t *testing.T) {

openapi/response.go

@@ -11,34 +11,70 @@ import (
 // Type aliases for OAuth types to simplify usage
 type (
 	// Core response types
-	ErrorResponse        = types.ErrorResponse
+
-	Token                = types.Token
+	// ErrorResponse represents an OAuth 2.0 error response as defined in RFC 6749
+	ErrorResponse = types.ErrorResponse
+
+	// Token represents an OAuth 2.0 access token response as defined in RFC 6749
+	Token = types.Token
+
+	// RefreshTokenResponse represents an OAuth 2.0 refresh token response
 	RefreshTokenResponse = types.RefreshTokenResponse
 
 	// Authorization flow types
-	AuthorizationRequest  = types.AuthorizationRequest
+
+	// AuthorizationRequest represents an OAuth 2.0 authorization request parameters
+	AuthorizationRequest = types.AuthorizationRequest
+
+	// AuthorizationResponse represents an OAuth 2.0 authorization response
 	AuthorizationResponse = types.AuthorizationResponse
 
 	// Client management types
-	ClientInfo                        = types.ClientInfo
+
-	DynamicClientRegistrationRequest  = types.DynamicClientRegistrationRequest
+	// ClientInfo represents OAuth 2.0 client registration information
+	ClientInfo = types.ClientInfo
+
+	// DynamicClientRegistrationRequest represents a dynamic client registration request as defined in RFC 7591
+	DynamicClientRegistrationRequest = types.DynamicClientRegistrationRequest
+
+	// DynamicClientRegistrationResponse represents a dynamic client registration response as defined in RFC 7591
 	DynamicClientRegistrationResponse = types.DynamicClientRegistrationResponse
 
 	// Extended OAuth types
+
+	// DeviceAuthorizationResponse represents a device authorization response as defined in RFC 8628
 	DeviceAuthorizationResponse = types.DeviceAuthorizationResponse
-	PushedAuthorizationRequest  = types.PushedAuthorizationRequest
+
+	// PushedAuthorizationRequest represents a pushed authorization request as defined in RFC 9126
+	PushedAuthorizationRequest = types.PushedAuthorizationRequest
+
+	// PushedAuthorizationResponse represents a pushed authorization response as defined in RFC 9126
 	PushedAuthorizationResponse = types.PushedAuthorizationResponse
-	TokenExchangeResponse       = types.TokenExchangeResponse
+
-	TokenIntrospectionResponse  = types.TokenIntrospectionResponse
+	// TokenExchangeResponse represents a token exchange response as defined in RFC 8693
+	TokenExchangeResponse = types.TokenExchangeResponse
+
+	// TokenIntrospectionResponse represents a token introspection response as defined in RFC 7662
+	TokenIntrospectionResponse = types.TokenIntrospectionResponse
 
 	// Discovery types
+
+	// AuthorizationServerMetadata represents OAuth 2.0 authorization server metadata as defined in RFC 8414
 	AuthorizationServerMetadata = types.AuthorizationServerMetadata
-	ProtectedResourceMetadata   = types.ProtectedResourceMetadata
+
+	// ProtectedResourceMetadata represents OAuth 2.0 protected resource metadata as defined in RFC 9728
+	ProtectedResourceMetadata = types.ProtectedResourceMetadata
 
 	// Security types
+
+	// WWWAuthenticateChallenge represents a WWW-Authenticate challenge header structure
 	WWWAuthenticateChallenge = types.WWWAuthenticateChallenge
-	JWKSResponse             = types.JWKSResponse
+
-	JWK                      = types.JWK
+	// JWKSResponse represents a JSON Web Key Set response as defined in RFC 7517
+	JWKSResponse = types.JWKSResponse
+
+	// JWK represents a JSON Web Key as defined in RFC 7517
+	JWK = types.JWK
 )
 
 // Standard OAuth 2.0/2.1 Error Codes - RFC 6749 Section 5.2
@@ -119,10 +155,27 @@ type StandardResponse struct {
 	RequestID string         `json:"request_id,omitempty"`
 }
 
+// setOAuthSecurityHeaders sets standard OAuth 2.0/2.1 security headers
+// These headers are required by OAuth 2.1 specification for enhanced security
+func (openapi *OpenAPI) setOAuthSecurityHeaders(c *gin.Context) {
+	c.Header("Cache-Control", "no-store")
+	c.Header("Pragma", "no-cache")
+	c.Header("X-Content-Type-Options", "nosniff")
+	c.Header("X-Frame-Options", "DENY")
+	c.Header("Referrer-Policy", "no-referrer")
+}
+
+// setJSONContentType sets JSON content type header for OAuth responses
+func (openapi *OpenAPI) setJSONContentType(c *gin.Context) {
+	c.Header("Content-Type", "application/json;charset=UTF-8")
+}
+
 // Response helper functions for consistent OAuth responses
 
 // respondWithSuccess sends a successful OAuth response
 func (openapi *OpenAPI) respondWithSuccess(c *gin.Context, statusCode int, data interface{}) {
+	openapi.setOAuthSecurityHeaders(c)
+
 	response := StandardResponse{
 		Success:   true,
 		Data:      data,
@@ -130,13 +183,13 @@ func (openapi *OpenAPI) respondWithSuccess(c *gin.Context, statusCode int, data
 		RequestID: c.GetString("request_id"),
 	}
 
-	c.Header("Cache-Control", "no-store")
-	c.Header("Pragma", "no-cache")
 	c.JSON(statusCode, response)
 }
 
 // respondWithError sends an OAuth error response
 func (openapi *OpenAPI) respondWithError(c *gin.Context, statusCode int, err *ErrorResponse) {
+	openapi.setOAuthSecurityHeaders(c)
+
 	response := StandardResponse{
 		Success:   false,
 		Error:     err,
@@ -144,9 +197,6 @@ func (openapi *OpenAPI) respondWithError(c *gin.Context, statusCode int, err *Er
 		RequestID: c.GetString("request_id"),
 	}
 
-	c.Header("Cache-Control", "no-store")
-	c.Header("Pragma", "no-cache")
-
 	// Add WWW-Authenticate header for 401 responses
 	if statusCode == StatusUnauthorized {
 		openapi.addWWWAuthenticateHeader(c, err)
@@ -156,21 +206,29 @@ func (openapi *OpenAPI) respondWithError(c *gin.Context, statusCode int, err *Er
 }
 
 // respondWithTokenSuccess sends a successful token response (without wrapper)
+// This method is used for OAuth token endpoint responses that must follow RFC 6749 format
 func (openapi *OpenAPI) respondWithTokenSuccess(c *gin.Context, token interface{}) {
-	c.Header("Cache-Control", "no-store")
+	openapi.setOAuthSecurityHeaders(c)
-	c.Header("Pragma", "no-cache")
+	openapi.setJSONContentType(c)
-	c.Header("Content-Type", "application/json;charset=UTF-8")
 	c.JSON(StatusOK, token)
 }
 
 // respondWithTokenError sends a token endpoint error response (without wrapper)
+// This method is used for OAuth token endpoint errors that must follow RFC 6749 format
 func (openapi *OpenAPI) respondWithTokenError(c *gin.Context, err *ErrorResponse) {
-	c.Header("Cache-Control", "no-store")
+	openapi.setOAuthSecurityHeaders(c)
-	c.Header("Pragma", "no-cache")
+	openapi.setJSONContentType(c)
-	c.Header("Content-Type", "application/json;charset=UTF-8")
 	c.JSON(StatusBadRequest, err)
 }
 
+// respondWithOAuthDirect sends a direct OAuth response without StandardResponse wrapper
+// This method is used for endpoints that require RFC-compliant response format (e.g., client registration)
+func (openapi *OpenAPI) respondWithOAuthDirect(c *gin.Context, statusCode int, data interface{}) {
+	openapi.setOAuthSecurityHeaders(c)
+	openapi.setJSONContentType(c)
+	c.JSON(statusCode, data)
+}
+
 // respondWithAuthorizationError sends an authorization endpoint error via redirect
 func (openapi *OpenAPI) respondWithAuthorizationError(c *gin.Context, redirectURI string, err *ErrorResponse, state string) {
 	// Build error redirect URL
@@ -276,29 +334,17 @@ func createErrorWithState(baseError *ErrorResponse, state string) *ErrorResponse
 	return errorWithState
 }
 
-// OAuth 2.1 specific response helpers
+// Legacy OAuth 2.1 specific response helpers (deprecated)
+// These methods are kept for backward compatibility but should use the unified approach above
 
 // respondWithOAuth21Error ensures OAuth 2.1 compliance for error responses
+// Deprecated: Use respondWithError instead, which now includes all OAuth 2.1 security headers
 func (openapi *OpenAPI) respondWithOAuth21Error(c *gin.Context, statusCode int, err *ErrorResponse) {
-	// OAuth 2.1 requires additional security headers
-	c.Header("Cache-Control", "no-store")
-	c.Header("Pragma", "no-cache")
-	c.Header("X-Content-Type-Options", "nosniff")
-	c.Header("X-Frame-Options", "DENY")
-	c.Header("Referrer-Policy", "no-referrer")
-
 	openapi.respondWithError(c, statusCode, err)
 }
 
 // respondWithOAuth21TokenSuccess ensures OAuth 2.1 compliance for token responses
+// Deprecated: Use respondWithTokenSuccess instead, which now includes all OAuth 2.1 security headers
 func (openapi *OpenAPI) respondWithOAuth21TokenSuccess(c *gin.Context, token interface{}) {
-	// OAuth 2.1 requires additional security headers
+	openapi.respondWithTokenSuccess(c, token)
-	c.Header("Cache-Control", "no-store")
-	c.Header("Pragma", "no-cache")
-	c.Header("X-Content-Type-Options", "nosniff")
-	c.Header("X-Frame-Options", "DENY")
-	c.Header("Referrer-Policy", "no-referrer")
-	c.Header("Content-Type", "application/json;charset=UTF-8")
-
-	c.JSON(StatusOK, token)
 }