Add OAuth authorization tests and improve error handling

openapi/oauth.go

@@ -1,6 +1,8 @@
 package openapi
 
 import (
+	"net/http"
+
 	"github.com/gin-gonic/gin"
 	"github.com/yaoapp/yao/openapi/oauth/types"
 )
@@ -66,15 +68,50 @@ func (openapi *OpenAPI) attachOAuth(base *gin.RouterGroup) {
 // oauthAuthorize handles authorization requests - RFC 6749 Section 3.1
 func (openapi *OpenAPI) oauthAuthorize(c *gin.Context) {
 	// Parse and validate authorization request
-	authReq, err := openapi.parseAuthorizationRequest(c)
+	authReq, parseErr := openapi.parseAuthorizationRequest(c)
-	if err != nil {
+	if parseErr != nil {
-		openapi.respondWithAuthorizationError(c, authReq.RedirectURI, err, authReq.State)
+		openapi.respondWithAuthorizationError(c, authReq.RedirectURI, parseErr, authReq.State)
 		return
 	}
 
-	// TODO: Implement full authorization logic
+	// Call OAuth service to process authorization request
-	// For now, return server error to indicate not implemented
+	authResp, err := openapi.OAuth.Authorize(c, authReq)
-	openapi.respondWithAuthorizationError(c, authReq.RedirectURI, ErrServerError, authReq.State)
+	if err != nil {
+		// OAuth service returned an error
+		openapi.respondWithAuthorizationError(c, authReq.RedirectURI, ErrServerError, authReq.State)
+		return
+	}
+
+	// Check if authorization response contains an error
+	if authResp.Error != "" {
+		// Convert OAuth service error to ErrorResponse
+		oauthError := &ErrorResponse{
+			Code:             authResp.Error,
+			ErrorDescription: authResp.ErrorDescription,
+		}
+		openapi.respondWithAuthorizationError(c, authReq.RedirectURI, oauthError, authReq.State)
+		return
+	}
+
+	// Success: redirect to client with authorization code
+	redirectURL := authReq.RedirectURI
+	if redirectURL != "" {
+		separator := "?"
+		if len(redirectURL) > 0 && redirectURL[len(redirectURL)-1:] == "?" {
+			separator = "&"
+		}
+
+		redirectURL += separator + "code=" + authResp.Code
+		if authResp.State != "" {
+			redirectURL += "&state=" + authResp.State
+		}
+
+		c.Redirect(http.StatusFound, redirectURL)
+		return
+	}
+
+	// Fallback: return JSON response if no redirect URI (should not happen with valid requests)
+	openapi.respondWithSuccess(c, StatusOK, authResp)
 }
 
 // oauthToken handles token requests - RFC 6749 Section 3.2
@@ -379,9 +416,9 @@ func (openapi *OpenAPI) handleDeviceCodeGrant(c *gin.Context) {
 }
 
 // parseAuthorizationRequest parses and validates authorization request parameters
-func (openapi *OpenAPI) parseAuthorizationRequest(c *gin.Context) (*AuthorizationRequest, *ErrorResponse) {
+func (openapi *OpenAPI) parseAuthorizationRequest(c *gin.Context) (*types.AuthorizationRequest, *ErrorResponse) {
 	// Parse authorization request parameters from both GET (query) and POST (form) methods
-	authReq := &AuthorizationRequest{
+	authReq := &types.AuthorizationRequest{
 		ClientID:            openapi.getParam(c, "client_id"),
 		ResponseType:        openapi.getParam(c, "response_type"),
 		RedirectURI:         openapi.getParam(c, "redirect_uri"),

openapi/oauth_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"io"
 	"net/http"
+	"net/url"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -114,3 +115,210 @@ func TestOAuthRegister(t *testing.T) {
 		}
 	})
 }
+
+func TestOAuthAuthorize(t *testing.T) {
+	serverURL := Prepare(t)
+	defer Clean()
+
+	// Register a test client for realistic testing
+	testClient := RegisterTestClient(t, "OAuth Test Client", []string{"http://localhost/callback"})
+	defer CleanupTestClient(t, testClient.ClientID)
+
+	// Prepare test data
+	endpoint := serverURL + Server.Config.BaseURL + "/oauth/authorize"
+	t.Logf("Testing authorize endpoint: %s", endpoint)
+
+	t.Run("Valid Authorization Request", func(t *testing.T) {
+		// Test valid authorization request with real client
+		params := url.Values{}
+		params.Set("client_id", testClient.ClientID) // Use real registered client ID
+		params.Set("response_type", "code")
+		params.Set("redirect_uri", testClient.RedirectURIs[0]) // Use registered redirect URI
+		params.Set("scope", "openid profile")
+		params.Set("state", "test-state-123")
+
+		requestURL := endpoint + "?" + params.Encode()
+		t.Logf("Making GET request to: %s", requestURL)
+
+		// Configure HTTP client to not follow redirects automatically
+		client := &http.Client{
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				return http.ErrUseLastResponse
+			},
+		}
+
+		resp, err := client.Get(requestURL)
+		assert.NoError(t, err)
+		assert.NotNil(t, resp)
+		defer resp.Body.Close()
+
+		t.Logf("Response status code: %d", resp.StatusCode)
+
+		// Should redirect with either success (302) or error (302)
+		assert.Equal(t, http.StatusFound, resp.StatusCode)
+
+		// Check redirect location
+		location := resp.Header.Get("Location")
+		assert.NotEmpty(t, location, "Location header should be present")
+		t.Logf("Redirect location: %s", location)
+
+		// Parse redirect URL to check parameters
+		redirectURL, err := url.Parse(location)
+		assert.NoError(t, err)
+
+		// Should contain either 'code' (success) or 'error' (failure) parameter
+		query := redirectURL.Query()
+		hasCode := query.Get("code") != ""
+		hasError := query.Get("error") != ""
+		assert.True(t, hasCode || hasError, "Redirect should contain either 'code' or 'error' parameter")
+
+		// State parameter should be preserved
+		assert.Equal(t, "test-state-123", query.Get("state"), "State parameter should be preserved")
+
+		t.Logf("Authorization result - Code: %s, Error: %s", query.Get("code"), query.Get("error"))
+	})
+
+	t.Run("Invalid Client ID", func(t *testing.T) {
+		// Test with invalid client ID
+		params := url.Values{}
+		params.Set("client_id", "invalid-client-id")
+		params.Set("response_type", "code")
+		params.Set("redirect_uri", "http://localhost/callback")
+		params.Set("scope", "openid profile")
+		params.Set("state", "test-state-456")
+
+		requestURL := endpoint + "?" + params.Encode()
+
+		client := &http.Client{
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				return http.ErrUseLastResponse
+			},
+		}
+
+		resp, err := client.Get(requestURL)
+		assert.NoError(t, err)
+		assert.NotNil(t, resp)
+		defer resp.Body.Close()
+
+		assert.Equal(t, http.StatusFound, resp.StatusCode)
+
+		location := resp.Header.Get("Location")
+		redirectURL, err := url.Parse(location)
+		assert.NoError(t, err)
+
+		query := redirectURL.Query()
+		assert.Equal(t, "invalid_client", query.Get("error"), "Should return invalid_client error")
+		assert.Equal(t, "test-state-456", query.Get("state"), "State should be preserved")
+	})
+
+	t.Run("Valid Authorization Request via POST", func(t *testing.T) {
+		// Test valid authorization request with POST method
+		form := url.Values{}
+		form.Set("client_id", testClient.ClientID)
+		form.Set("response_type", "code")
+		form.Set("redirect_uri", testClient.RedirectURIs[0])
+		form.Set("scope", "openid profile")
+		form.Set("state", "test-post-state-789")
+
+		t.Logf("Making POST request to: %s", endpoint)
+
+		// Configure HTTP client to not follow redirects automatically
+		client := &http.Client{
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				return http.ErrUseLastResponse
+			},
+		}
+
+		resp, err := client.PostForm(endpoint, form)
+		assert.NoError(t, err)
+		assert.NotNil(t, resp)
+		defer resp.Body.Close()
+
+		t.Logf("Response status code: %d", resp.StatusCode)
+
+		// Should redirect with either success (302) or error (302)
+		assert.Equal(t, http.StatusFound, resp.StatusCode)
+
+		// Check redirect location
+		location := resp.Header.Get("Location")
+		assert.NotEmpty(t, location, "Location header should be present")
+		t.Logf("Redirect location: %s", location)
+
+		// Parse redirect URL to check parameters
+		redirectURL, err := url.Parse(location)
+		assert.NoError(t, err)
+
+		// Should contain either 'code' (success) or 'error' (failure) parameter
+		query := redirectURL.Query()
+		hasCode := query.Get("code") != ""
+		hasError := query.Get("error") != ""
+		assert.True(t, hasCode || hasError, "Redirect should contain either 'code' or 'error' parameter")
+
+		// State parameter should be preserved
+		assert.Equal(t, "test-post-state-789", query.Get("state"), "State parameter should be preserved")
+
+		t.Logf("Authorization result (POST) - Code: %s, Error: %s", query.Get("code"), query.Get("error"))
+	})
+
+	t.Run("Invalid Response Type via POST", func(t *testing.T) {
+		// Test with invalid response type using POST
+		form := url.Values{}
+		form.Set("client_id", testClient.ClientID)
+		form.Set("response_type", "token") // Implicit flow - deprecated in OAuth 2.1
+		form.Set("redirect_uri", testClient.RedirectURIs[0])
+		form.Set("scope", "openid profile")
+		form.Set("state", "test-invalid-response-type")
+
+		client := &http.Client{
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				return http.ErrUseLastResponse
+			},
+		}
+
+		resp, err := client.PostForm(endpoint, form)
+		assert.NoError(t, err)
+		assert.NotNil(t, resp)
+		defer resp.Body.Close()
+
+		assert.Equal(t, http.StatusFound, resp.StatusCode)
+
+		location := resp.Header.Get("Location")
+		redirectURL, err := url.Parse(location)
+		assert.NoError(t, err)
+
+		query := redirectURL.Query()
+		assert.Equal(t, "unsupported_response_type", query.Get("error"), "Should return unsupported_response_type error")
+		assert.Equal(t, "test-invalid-response-type", query.Get("state"), "State should be preserved")
+	})
+
+	t.Run("Missing Required Parameters via POST", func(t *testing.T) {
+		// Test with missing client_id using POST
+		form := url.Values{}
+		// Missing client_id
+		form.Set("response_type", "code")
+		form.Set("redirect_uri", testClient.RedirectURIs[0])
+		form.Set("scope", "openid profile")
+		form.Set("state", "test-missing-client-id")
+
+		client := &http.Client{
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				return http.ErrUseLastResponse
+			},
+		}
+
+		resp, err := client.PostForm(endpoint, form)
+		assert.NoError(t, err)
+		assert.NotNil(t, resp)
+		defer resp.Body.Close()
+
+		assert.Equal(t, http.StatusFound, resp.StatusCode)
+
+		location := resp.Header.Get("Location")
+		redirectURL, err := url.Parse(location)
+		assert.NoError(t, err)
+
+		query := redirectURL.Query()
+		assert.Equal(t, "invalid_request", query.Get("error"), "Should return invalid_request error")
+		assert.Equal(t, "test-missing-client-id", query.Get("state"), "State should be preserved")
+	})
+}

openapi/openapi_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/gin-gonic/gin"
 	"github.com/stretchr/testify/assert"
 	"github.com/yaoapp/yao/config"
+	"github.com/yaoapp/yao/openapi/oauth/types"
 	"github.com/yaoapp/yao/test"
 )
 
@@ -169,6 +170,154 @@ func Clean() {
 	test.Clean()
 }
 
+// RegisterTestClient registers a test OAuth client and returns the client information.
+//
+// AI ASSISTANT INSTRUCTIONS:
+// Use this function to create test OAuth clients for testing OAuth endpoints.
+// This function provides realistic test clients that can be used for authentication flows.
+// ALWAYS clean up test clients using CleanupTestClient() to prevent test interference.
+//
+// Usage pattern:
+//
+//	func TestOAuthEndpoint(t *testing.T) {
+//	    serverURL := Prepare(t)
+//	    defer Clean()
+//
+//	    // Register a test client
+//	    client := RegisterTestClient(t, "Test Client", []string{"http://localhost/callback"})
+//	    defer CleanupTestClient(t, client.ClientID)
+//
+//	    // Use client.ClientID and client.ClientSecret in your tests
+//	    // Example: test OAuth authorize with real client_id
+//	}
+//
+// PARAMETERS:
+// - t: The test instance for error reporting
+// - clientName: Human-readable name for the client (e.g., "Test Web App")
+// - redirectURIs: List of valid redirect URIs for the client
+//
+// RETURN VALUE:
+// Returns a pointer to types.ClientInfo containing:
+// - ClientID: Generated unique client identifier
+// - ClientSecret: Generated client secret (for confidential clients)
+// - RedirectURIs: The provided redirect URIs
+// - Other OAuth client metadata
+//
+// ERROR HANDLING:
+// If client registration fails, the test will fail immediately with a descriptive error message.
+func RegisterTestClient(t *testing.T, clientName string, redirectURIs []string) *types.ClientInfo {
+	if Server == nil || Server.OAuth == nil {
+		t.Fatal("OpenAPI server not initialized. Call Prepare(t) first.")
+	}
+
+	// Create dynamic client registration request
+	req := &types.DynamicClientRegistrationRequest{
+		ClientName:   clientName,
+		RedirectURIs: redirectURIs,
+		GrantTypes: []string{
+			"authorization_code",
+			"refresh_token",
+			"client_credentials",
+		},
+		ResponseTypes: []string{
+			"code",
+		},
+		ApplicationType:         "web",
+		TokenEndpointAuthMethod: "client_secret_basic",
+		Scope:                   "openid profile email",
+	}
+
+	// Register the client using the OAuth service
+	ctx := context.Background()
+	response, err := Server.OAuth.DynamicClientRegistration(ctx, req)
+	if err != nil {
+		t.Fatalf("Failed to register test client: %v", err)
+	}
+
+	// Convert response to ClientInfo for easier usage
+	clientInfo := &types.ClientInfo{
+		ClientID:                response.ClientID,
+		ClientSecret:            response.ClientSecret,
+		ClientName:              response.ClientName,
+		RedirectURIs:            response.RedirectURIs,
+		GrantTypes:              response.GrantTypes,
+		ResponseTypes:           response.ResponseTypes,
+		ApplicationType:         response.ApplicationType,
+		TokenEndpointAuthMethod: response.TokenEndpointAuthMethod,
+		Scope:                   response.Scope,
+		ClientURI:               response.ClientURI,
+		LogoURI:                 response.LogoURI,
+		TosURI:                  response.TosURI,
+		PolicyURI:               response.PolicyURI,
+		Contacts:                response.Contacts,
+	}
+
+	t.Logf("Registered test client: %s (ID: %s)", clientName, clientInfo.ClientID)
+	return clientInfo
+}
+
+// CleanupTestClient removes a test OAuth client from the system.
+//
+// AI ASSISTANT INSTRUCTIONS:
+// ALWAYS call this function to clean up test clients created with RegisterTestClient().
+// Use defer to ensure cleanup happens even if tests fail or panic.
+// Proper cleanup prevents test interference and maintains a clean test environment.
+//
+// Usage pattern:
+//
+//	client := RegisterTestClient(t, "Test Client", []string{"http://localhost/callback"})
+//	defer CleanupTestClient(t, client.ClientID)
+//
+// PARAMETERS:
+// - t: The test instance for error reporting
+// - clientID: The client ID to remove (obtained from RegisterTestClient return value)
+//
+// ERROR HANDLING:
+// If client deletion fails, logs an error but does not fail the test.
+// This prevents cleanup failures from affecting test results.
+func CleanupTestClient(t *testing.T, clientID string) {
+	if Server == nil || Server.OAuth == nil {
+		// Server might already be cleaned up, which is OK
+		return
+	}
+
+	if clientID == "" {
+		return
+	}
+
+	// Delete the client using the OAuth service
+	ctx := context.Background()
+	err := Server.OAuth.DeleteClient(ctx, clientID)
+	if err != nil {
+		// Log error but don't fail the test - cleanup should be resilient
+		t.Logf("Warning: Failed to cleanup test client %s: %v", clientID, err)
+	} else {
+		t.Logf("Cleaned up test client: %s", clientID)
+	}
+}
+
+// CreateTestClientCredentials creates a simple test client with just ID and secret for basic testing.
+//
+// AI ASSISTANT INSTRUCTIONS:
+// Use this function when you need a quick test client without full OAuth registration.
+// This is useful for testing non-OAuth endpoints or when you need predictable client credentials.
+// This creates an in-memory client that doesn't persist and doesn't need cleanup.
+//
+// Usage pattern:
+//
+//	clientID, clientSecret := CreateTestClientCredentials()
+//	// Use in Basic Auth or client_credentials grant tests
+//
+// RETURN VALUES:
+// - clientID: A predictable test client ID
+// - clientSecret: A predictable test client secret
+//
+// NOTE: This function creates temporary credentials and doesn't register them with the OAuth service.
+// For full OAuth flow testing, use RegisterTestClient() instead.
+func CreateTestClientCredentials() (clientID, clientSecret string) {
+	return "test-client-id", "test-client-secret"
+}
+
 func TestLoad(t *testing.T) {
 	serverURL := Prepare(t)
 	defer Clean()