From cd9c52fe33882911969c67bf0d55131ca2c33865 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 23 Oct 2025 10:48:53 +0800 Subject: [PATCH] Normalize request paths in ACL checks and add path normalization utility - Implemented path normalization in the ACL enforcement logic to ensure consistent matching by removing trailing slashes from request paths. - Introduced a new utility function, normalizePath, to handle path normalization across various components. - Updated endpoint matching and public endpoint checks to utilize the normalized paths, improving access decision accuracy. - Enhanced team and member creation logic to include a new field, __yao_team_id, for better data management. --- openapi/oauth/acl/scope.go | 41 ++- openapi/oauth/acl/scope_test.go | 150 ++++++++ openapi/oauth/providers/user/member.go | 3 + openapi/oauth/providers/user/team.go | 1 + openapi/oauth/types/authorized.go | 177 ++++++++++ openapi/oauth/types/authorized_test.go | 464 +++++++++++++++++++++++++ openapi/user/team.go | 25 +- 7 files changed, 844 insertions(+), 17 deletions(-) create mode 100644 openapi/oauth/acl/scope_test.go create mode 100644 openapi/oauth/types/authorized.go create mode 100644 openapi/oauth/types/authorized_test.go diff --git a/openapi/oauth/acl/scope.go b/openapi/oauth/acl/scope.go index 91190f20..f410761b 100644 --- a/openapi/oauth/acl/scope.go +++ b/openapi/oauth/acl/scope.go @@ -341,6 +341,9 @@ func (m *ScopeManager) buildIndexes() error { // addEndpointRule adds an endpoint rule to the index func (m *ScopeManager) addEndpointRule(method, path, action string, scopes []string) error { + // Normalize path: remove trailing slash (except for root path) + path = normalizePath(path) + // Get or create PathMatcher for this method matcher := m.endpointIndex[method] if matcher == nil { @@ -478,16 +481,19 @@ func (m *ScopeManager) Check(req *AccessRequest) *AccessDecision { UserScopes: req.Scopes, } + // Normalize the request path + normalizedPath := normalizePath(req.Path) + // 1. Check if it's a public endpoint - publicKey := req.Method + " " + req.Path + publicKey := req.Method + " " + normalizedPath if _, ok := m.publicPaths[publicKey]; ok { decision.Allowed = true decision.Reason = "public endpoint" return decision } - // 2. Find matching endpoint - endpoint, pattern := m.matchEndpoint(req.Method, req.Path) + // 2. Find matching endpoint (matchEndpoint will normalize the path again, but it's idempotent) + endpoint, pattern := m.matchEndpoint(req.Method, normalizedPath) if endpoint == nil { // No match found, use default policy decision.Allowed = m.defaultAction == "allow" @@ -560,16 +566,19 @@ func (m *ScopeManager) CheckRestricted(req *AccessRequest) *AccessDecision { UserScopes: req.Scopes, } + // Normalize the request path + normalizedPath := normalizePath(req.Path) + // 1. Check if it's a public endpoint - public endpoints cannot be restricted - publicKey := req.Method + " " + req.Path + publicKey := req.Method + " " + normalizedPath if _, ok := m.publicPaths[publicKey]; ok { decision.Allowed = true decision.Reason = "public endpoint" return decision } - // 2. Find matching endpoint - endpoint, pattern := m.matchEndpoint(req.Method, req.Path) + // 2. Find matching endpoint (matchEndpoint will normalize the path again, but it's idempotent) + endpoint, pattern := m.matchEndpoint(req.Method, normalizedPath) if endpoint == nil { // No match found - not restricted decision.Allowed = true @@ -622,6 +631,9 @@ func (m *ScopeManager) CheckRestricted(req *AccessRequest) *AccessDecision { // matchEndpoint finds the matching endpoint for a request func (m *ScopeManager) matchEndpoint(method, path string) (*EndpointInfo, string) { + // Normalize path: remove trailing slash (except for root path) + path = normalizePath(path) + matcher := m.endpointIndex[method] if matcher == nil { return nil, "" @@ -823,3 +835,20 @@ func (m *ScopeManager) Reload() error { return nil } + +// normalizePath normalizes a path by removing trailing slashes (except for root path "/") +// This ensures consistent path matching regardless of whether the request or definition has a trailing slash +// Examples: +// - "/user/teams/" -> "/user/teams" +// - "/user/teams" -> "/user/teams" +// - "/" -> "/" +// - "" -> "" +func normalizePath(path string) string { + // Empty path or root path - return as is + if path == "" || path == "/" { + return path + } + + // Remove trailing slash + return strings.TrimSuffix(path, "/") +} diff --git a/openapi/oauth/acl/scope_test.go b/openapi/oauth/acl/scope_test.go new file mode 100644 index 00000000..97e8e156 --- /dev/null +++ b/openapi/oauth/acl/scope_test.go @@ -0,0 +1,150 @@ +package acl + +import ( + "testing" +) + +func TestNormalizePath(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "path with trailing slash", + input: "/user/teams/", + expected: "/user/teams", + }, + { + name: "path without trailing slash", + input: "/user/teams", + expected: "/user/teams", + }, + { + name: "root path", + input: "/", + expected: "/", + }, + { + name: "empty path", + input: "", + expected: "", + }, + { + name: "nested path with trailing slash", + input: "/user/teams/members/", + expected: "/user/teams/members", + }, + { + name: "nested path without trailing slash", + input: "/user/teams/members", + expected: "/user/teams/members", + }, + { + name: "path with multiple trailing slashes", + input: "/user/teams//", + expected: "/user/teams/", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := normalizePath(tt.input) + if result != tt.expected { + t.Errorf("normalizePath(%q) = %q, want %q", tt.input, result, tt.expected) + } + }) + } +} + +// TestPathMatchingWithTrailingSlash tests that paths match correctly regardless of trailing slashes +func TestPathMatchingWithTrailingSlash(t *testing.T) { + manager := &ScopeManager{ + endpointIndex: make(map[string]*PathMatcher), + publicPaths: make(map[string]struct{}), + } + + // Add a test endpoint without trailing slash + err := manager.addEndpointRule("POST", "/user/teams", "require-scopes", []string{"teams:write:own"}) + if err != nil { + t.Fatalf("Failed to add endpoint rule: %v", err) + } + + // Test that matching works with trailing slash + endpoint, pattern := manager.matchEndpoint("POST", "/user/teams/") + if endpoint == nil { + t.Errorf("Expected to match endpoint POST /user/teams/, but got nil") + } + if pattern != "/user/teams" { + t.Errorf("Expected pattern /user/teams, got %s", pattern) + } + + // Test that matching works without trailing slash + endpoint, pattern = manager.matchEndpoint("POST", "/user/teams") + if endpoint == nil { + t.Errorf("Expected to match endpoint POST /user/teams, but got nil") + } + if pattern != "/user/teams" { + t.Errorf("Expected pattern /user/teams, got %s", pattern) + } +} + +// TestPathMatchingExactPaths tests exact path matching with normalization +func TestPathMatchingExactPaths(t *testing.T) { + manager := &ScopeManager{ + endpointIndex: make(map[string]*PathMatcher), + publicPaths: make(map[string]struct{}), + } + + // Add endpoints with different trailing slash patterns + testCases := []struct { + method string + definedPath string + requestPaths []string + shouldMatch bool + }{ + { + method: "POST", + definedPath: "/user/teams", + requestPaths: []string{"/user/teams", "/user/teams/"}, + shouldMatch: true, + }, + { + method: "GET", + definedPath: "/user/teams/", + requestPaths: []string{"/user/teams", "/user/teams/"}, + shouldMatch: true, + }, + { + method: "DELETE", + definedPath: "/user/profile", + requestPaths: []string{"/user/profile", "/user/profile/"}, + shouldMatch: true, + }, + } + + for _, tc := range testCases { + // Add the endpoint + err := manager.addEndpointRule(tc.method, tc.definedPath, "allow", nil) + if err != nil { + t.Fatalf("Failed to add endpoint rule %s %s: %v", tc.method, tc.definedPath, err) + } + + // Test all request paths + for _, requestPath := range tc.requestPaths { + endpoint, pattern := manager.matchEndpoint(tc.method, requestPath) + if tc.shouldMatch && endpoint == nil { + t.Errorf("Expected %s %s to match defined path %s, but got nil", + tc.method, requestPath, tc.definedPath) + } else if tc.shouldMatch && endpoint != nil { + // Both paths should normalize to the same pattern + expectedPattern := normalizePath(tc.definedPath) + if pattern != expectedPattern { + t.Errorf("Expected pattern %s, got %s (defined: %s, request: %s)", + expectedPattern, pattern, tc.definedPath, requestPath) + } + } + } + } +} + diff --git a/openapi/oauth/providers/user/member.go b/openapi/oauth/providers/user/member.go index 097981f4..acf711df 100644 --- a/openapi/oauth/providers/user/member.go +++ b/openapi/oauth/providers/user/member.go @@ -130,6 +130,9 @@ func (u *DefaultUser) CreateMember(ctx context.Context, memberData maps.MapStrAn return 0, fmt.Errorf("role_id is required in memberData") } + // Add __yao_team_id to the member data + memberData["__yao_team_id"] = memberData["team_id"] + // Set default values if not provided if _, exists := memberData["member_type"]; !exists { memberData["member_type"] = "user" diff --git a/openapi/oauth/providers/user/team.go b/openapi/oauth/providers/user/team.go index 400b6e5e..1905bccd 100644 --- a/openapi/oauth/providers/user/team.go +++ b/openapi/oauth/providers/user/team.go @@ -82,6 +82,7 @@ func (u *DefaultUser) CreateTeam(ctx context.Context, teamData maps.MapStrAny) ( return "", fmt.Errorf("failed to generate team_id: %w", err) } teamData["team_id"] = teamID + teamData["__yao_team_id"] = teamID // Add __yao_team_id to the team data } // Validate required fields diff --git a/openapi/oauth/types/authorized.go b/openapi/oauth/types/authorized.go new file mode 100644 index 00000000..5d9df5bc --- /dev/null +++ b/openapi/oauth/types/authorized.go @@ -0,0 +1,177 @@ +package types + +import ( + "github.com/yaoapp/gou/model" + "github.com/yaoapp/kun/maps" +) + +// CreateAccessScope extracts access scope from the authorized info for creating records +func (info *AuthorizedInfo) CreateAccessScope() *model.AccessScope { + if info == nil { + return nil + } + + scope := &model.AccessScope{} + + if info.UserID != "" { + scope.CreatedBy = info.UserID + } + + if info.TeamID != "" { + scope.TeamID = info.TeamID + } + + if info.TenantID != "" { + scope.TenantID = info.TenantID + } + + return scope +} + +// UpdateAccessScope extracts access scope from the authorized info for updating records +func (info *AuthorizedInfo) UpdateAccessScope() *model.AccessScope { + if info == nil { + return nil + } + + scope := &model.AccessScope{} + + if info.UserID != "" { + scope.UpdatedBy = info.UserID + } + + if info.TeamID != "" { + scope.TeamID = info.TeamID + } + + if info.TenantID != "" { + scope.TenantID = info.TenantID + } + + return scope +} + +// AccessScope extracts access scope from the authorized info +// Returns an AccessScope with all available fields populated +// Use specific Wheres methods (WheresTeamOnly, WheresCreatorOnly, etc.) to control query logic +func (info *AuthorizedInfo) AccessScope() *model.AccessScope { + if info == nil { + return nil + } + + scope := &model.AccessScope{} + + if info.UserID != "" { + scope.CreatedBy = info.UserID + scope.UpdatedBy = info.UserID + } + + if info.TeamID != "" { + scope.TeamID = info.TeamID + } + + if info.TenantID != "" { + scope.TenantID = info.TenantID + } + + return scope +} + +// WithCreateScope appends CreateAccessScope fields to data for insertion +// Returns map[string]interface{} with access scope fields added +func (info *AuthorizedInfo) WithCreateScope(data interface{}) map[string]interface{} { + scope := info.CreateAccessScope() + if scope == nil { + // If no scope, just convert data to map[string]interface{} + result := map[string]interface{}{} + switch v := data.(type) { + case map[string]interface{}: + return v + default: + // Try to convert using type assertion for common map types + if m, ok := data.(map[string]interface{}); ok { + return m + } + return result + } + } + return scope.Append(data) +} + +// WithUpdateScope appends UpdateAccessScope fields to data for update +// Returns map[string]interface{} with access scope fields added +func (info *AuthorizedInfo) WithUpdateScope(data interface{}) map[string]interface{} { + scope := info.UpdateAccessScope() + if scope == nil { + // If no scope, just convert data to map[string]interface{} + result := map[string]interface{}{} + switch v := data.(type) { + case map[string]interface{}: + return v + default: + // Try to convert using type assertion for common map types + if m, ok := data.(map[string]interface{}); ok { + return m + } + return result + } + } + return scope.Append(data) +} + +// CopyCreateScope copies CreateAccessScope fields from source to dest +// Extracts __yao_created_by, __yao_team_id, __yao_tenant_id from source +// Returns dest map[string]interface{} with access scope fields added +func CopyCreateScope(source, dest interface{}) map[string]interface{} { + sourceMap := convertToMap(source) + scope := &model.AccessScope{} + + // Extract fields from source + if createdBy, ok := sourceMap["__yao_created_by"].(string); ok && createdBy != "" { + scope.CreatedBy = createdBy + } + if teamID, ok := sourceMap["__yao_team_id"].(string); ok && teamID != "" { + scope.TeamID = teamID + } + if tenantID, ok := sourceMap["__yao_tenant_id"].(string); ok && tenantID != "" { + scope.TenantID = tenantID + } + + return scope.Append(dest) +} + +// CopyUpdateScope copies UpdateAccessScope fields from source to dest +// Extracts __yao_updated_by, __yao_team_id, __yao_tenant_id from source +// Returns dest map[string]interface{} with access scope fields added +func CopyUpdateScope(source, dest interface{}) map[string]interface{} { + sourceMap := convertToMap(source) + scope := &model.AccessScope{} + + if updatedBy, ok := sourceMap["__yao_updated_by"].(string); ok && updatedBy != "" { + scope.UpdatedBy = updatedBy + } + if teamID, ok := sourceMap["__yao_team_id"].(string); ok && teamID != "" { + scope.TeamID = teamID + } + if tenantID, ok := sourceMap["__yao_tenant_id"].(string); ok && tenantID != "" { + scope.TenantID = tenantID + } + + return scope.Append(dest) +} + +// convertToMap converts interface{} to map[string]interface{} +func convertToMap(data interface{}) map[string]interface{} { + if data == nil { + return map[string]interface{}{} + } + + switch v := data.(type) { + case map[string]interface{}: + return v + case maps.MapStrAny: + return map[string]interface{}(v) + default: + return map[string]interface{}{} + } +} diff --git a/openapi/oauth/types/authorized_test.go b/openapi/oauth/types/authorized_test.go new file mode 100644 index 00000000..cd164827 --- /dev/null +++ b/openapi/oauth/types/authorized_test.go @@ -0,0 +1,464 @@ +package types + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCreateAccessScope(t *testing.T) { + info := &AuthorizedInfo{ + UserID: "user123", + TeamID: "team456", + TenantID: "tenant789", + } + + scope := info.CreateAccessScope() + assert.NotNil(t, scope) + assert.Equal(t, "user123", scope.CreatedBy) + assert.Empty(t, scope.UpdatedBy) + assert.Equal(t, "team456", scope.TeamID) + assert.Equal(t, "tenant789", scope.TenantID) +} + +func TestCreateAccessScopeNil(t *testing.T) { + var info *AuthorizedInfo + scope := info.CreateAccessScope() + assert.Nil(t, scope) +} + +func TestCreateAccessScopePartial(t *testing.T) { + info := &AuthorizedInfo{ + UserID: "user123", + } + + scope := info.CreateAccessScope() + assert.NotNil(t, scope) + assert.Equal(t, "user123", scope.CreatedBy) + assert.Empty(t, scope.UpdatedBy) + assert.Empty(t, scope.TeamID) + assert.Empty(t, scope.TenantID) +} + +func TestUpdateAccessScope(t *testing.T) { + info := &AuthorizedInfo{ + UserID: "user456", + TeamID: "team789", + TenantID: "tenant000", + } + + scope := info.UpdateAccessScope() + assert.NotNil(t, scope) + assert.Empty(t, scope.CreatedBy) + assert.Equal(t, "user456", scope.UpdatedBy) + assert.Equal(t, "team789", scope.TeamID) + assert.Equal(t, "tenant000", scope.TenantID) +} + +func TestUpdateAccessScopeNil(t *testing.T) { + var info *AuthorizedInfo + scope := info.UpdateAccessScope() + assert.Nil(t, scope) +} + +func TestAccessScope(t *testing.T) { + info := &AuthorizedInfo{ + UserID: "user123", + TeamID: "team456", + TenantID: "tenant789", + } + + scope := info.AccessScope() + assert.NotNil(t, scope) + assert.Equal(t, "user123", scope.CreatedBy) + assert.Equal(t, "user123", scope.UpdatedBy) + assert.Equal(t, "team456", scope.TeamID) + assert.Equal(t, "tenant789", scope.TenantID) +} + +func TestAccessScopeNil(t *testing.T) { + var info *AuthorizedInfo + scope := info.AccessScope() + assert.Nil(t, scope) +} + +func TestAccessScopeEmptyFields(t *testing.T) { + info := &AuthorizedInfo{} + scope := info.AccessScope() + assert.NotNil(t, scope) + assert.Empty(t, scope.CreatedBy) + assert.Empty(t, scope.UpdatedBy) + assert.Empty(t, scope.TeamID) + assert.Empty(t, scope.TenantID) +} + +func TestAccessScopeOnlyUser(t *testing.T) { + info := &AuthorizedInfo{ + UserID: "user999", + } + + scope := info.AccessScope() + assert.NotNil(t, scope) + assert.Equal(t, "user999", scope.CreatedBy) + assert.Equal(t, "user999", scope.UpdatedBy) + assert.Empty(t, scope.TeamID) + assert.Empty(t, scope.TenantID) +} + +func TestAccessScopeOnlyTeam(t *testing.T) { + info := &AuthorizedInfo{ + TeamID: "team111", + TenantID: "tenant222", + } + + scope := info.AccessScope() + assert.NotNil(t, scope) + assert.Empty(t, scope.CreatedBy) + assert.Empty(t, scope.UpdatedBy) + assert.Equal(t, "team111", scope.TeamID) + assert.Equal(t, "tenant222", scope.TenantID) +} + +func TestAccessScopeIntegration(t *testing.T) { + info := &AuthorizedInfo{ + Subject: "subject123", + ClientID: "client456", + UserID: "user789", + TeamID: "team000", + TenantID: "tenant111", + } + + // Test CreateAccessScope + createScope := info.CreateAccessScope() + assert.Equal(t, "user789", createScope.CreatedBy) + assert.Empty(t, createScope.UpdatedBy) + assert.Equal(t, "team000", createScope.TeamID) + assert.Equal(t, "tenant111", createScope.TenantID) + + // Test UpdateAccessScope + updateScope := info.UpdateAccessScope() + assert.Empty(t, updateScope.CreatedBy) + assert.Equal(t, "user789", updateScope.UpdatedBy) + assert.Equal(t, "team000", updateScope.TeamID) + assert.Equal(t, "tenant111", updateScope.TenantID) + + // Test AccessScope + queryScope := info.AccessScope() + assert.Equal(t, "user789", queryScope.CreatedBy) + assert.Equal(t, "user789", queryScope.UpdatedBy) + assert.Equal(t, "team000", queryScope.TeamID) + assert.Equal(t, "tenant111", queryScope.TenantID) +} + +func TestWithCreateScope(t *testing.T) { + info := &AuthorizedInfo{ + UserID: "user123", + TeamID: "team456", + TenantID: "tenant789", + } + + data := map[string]interface{}{ + "name": "Test Team", + "description": "A test team", + } + + result := info.WithCreateScope(data) + assert.Equal(t, "Test Team", result["name"]) + assert.Equal(t, "A test team", result["description"]) + assert.Equal(t, "user123", result["__yao_created_by"]) + assert.Equal(t, "team456", result["__yao_team_id"]) + assert.Equal(t, "tenant789", result["__yao_tenant_id"]) + assert.Nil(t, result["__yao_updated_by"]) +} + +func TestWithCreateScopeNil(t *testing.T) { + var info *AuthorizedInfo + data := map[string]interface{}{ + "name": "Test", + } + + result := info.WithCreateScope(data) + assert.Equal(t, "Test", result["name"]) + assert.Nil(t, result["__yao_created_by"]) + assert.Nil(t, result["__yao_team_id"]) +} + +func TestWithCreateScopePartial(t *testing.T) { + info := &AuthorizedInfo{ + UserID: "user123", + } + + data := map[string]interface{}{ + "name": "Test", + } + + result := info.WithCreateScope(data) + assert.Equal(t, "Test", result["name"]) + assert.Equal(t, "user123", result["__yao_created_by"]) + assert.Nil(t, result["__yao_team_id"]) + assert.Nil(t, result["__yao_tenant_id"]) +} + +func TestWithUpdateScope(t *testing.T) { + info := &AuthorizedInfo{ + UserID: "user456", + TeamID: "team789", + TenantID: "tenant000", + } + + data := map[string]interface{}{ + "name": "Updated Team", + "description": "An updated team", + } + + result := info.WithUpdateScope(data) + assert.Equal(t, "Updated Team", result["name"]) + assert.Equal(t, "An updated team", result["description"]) + assert.Equal(t, "user456", result["__yao_updated_by"]) + assert.Equal(t, "team789", result["__yao_team_id"]) + assert.Equal(t, "tenant000", result["__yao_tenant_id"]) + assert.Nil(t, result["__yao_created_by"]) +} + +func TestWithUpdateScopeNil(t *testing.T) { + var info *AuthorizedInfo + data := map[string]interface{}{ + "name": "Test", + } + + result := info.WithUpdateScope(data) + assert.Equal(t, "Test", result["name"]) + assert.Nil(t, result["__yao_updated_by"]) + assert.Nil(t, result["__yao_team_id"]) +} + +func TestWithScopesIntegration(t *testing.T) { + info := &AuthorizedInfo{ + UserID: "user999", + TeamID: "team888", + TenantID: "tenant777", + } + + // Create scenario + createData := map[string]interface{}{ + "name": "New Record", + } + createResult := info.WithCreateScope(createData) + assert.Equal(t, "New Record", createResult["name"]) + assert.Equal(t, "user999", createResult["__yao_created_by"]) + assert.Equal(t, "team888", createResult["__yao_team_id"]) + assert.Equal(t, "tenant777", createResult["__yao_tenant_id"]) + assert.Nil(t, createResult["__yao_updated_by"]) + + // Update scenario + updateData := map[string]interface{}{ + "name": "Updated Record", + "status": "active", + } + updateResult := info.WithUpdateScope(updateData) + assert.Equal(t, "Updated Record", updateResult["name"]) + assert.Equal(t, "active", updateResult["status"]) + assert.Equal(t, "user999", updateResult["__yao_updated_by"]) + assert.Equal(t, "team888", updateResult["__yao_team_id"]) + assert.Equal(t, "tenant777", updateResult["__yao_tenant_id"]) + assert.Nil(t, updateResult["__yao_created_by"]) +} + +func TestCopyCreateScope(t *testing.T) { + source := map[string]interface{}{ + "id": 1, + "name": "Original Record", + "__yao_created_by": "user123", + "__yao_team_id": "team456", + "__yao_tenant_id": "tenant789", + "__yao_updated_by": "user999", // Should not be copied + } + + dest := map[string]interface{}{ + "name": "New Record", + "description": "A new record", + } + + result := CopyCreateScope(source, dest) + assert.Equal(t, "New Record", result["name"]) + assert.Equal(t, "A new record", result["description"]) + assert.Equal(t, "user123", result["__yao_created_by"]) + assert.Equal(t, "team456", result["__yao_team_id"]) + assert.Equal(t, "tenant789", result["__yao_tenant_id"]) + assert.Nil(t, result["__yao_updated_by"]) // Should not be copied +} + +func TestCopyCreateScopePartial(t *testing.T) { + source := map[string]interface{}{ + "name": "Original", + "__yao_created_by": "user123", + } + + dest := map[string]interface{}{ + "name": "New", + } + + result := CopyCreateScope(source, dest) + assert.Equal(t, "New", result["name"]) + assert.Equal(t, "user123", result["__yao_created_by"]) + assert.Nil(t, result["__yao_team_id"]) + assert.Nil(t, result["__yao_tenant_id"]) +} + +func TestCopyCreateScopeEmpty(t *testing.T) { + source := map[string]interface{}{ + "name": "Original", + } + + dest := map[string]interface{}{ + "name": "New", + } + + result := CopyCreateScope(source, dest) + assert.Equal(t, "New", result["name"]) + assert.Nil(t, result["__yao_created_by"]) + assert.Nil(t, result["__yao_team_id"]) + assert.Nil(t, result["__yao_tenant_id"]) +} + +func TestCopyUpdateScope(t *testing.T) { + source := map[string]interface{}{ + "id": 1, + "name": "Original Record", + "__yao_updated_by": "user456", + "__yao_team_id": "team789", + "__yao_tenant_id": "tenant000", + "__yao_created_by": "user123", // Should not be copied + } + + dest := map[string]interface{}{ + "name": "Updated Record", + "status": "active", + } + + result := CopyUpdateScope(source, dest) + assert.Equal(t, "Updated Record", result["name"]) + assert.Equal(t, "active", result["status"]) + assert.Equal(t, "user456", result["__yao_updated_by"]) + assert.Equal(t, "team789", result["__yao_team_id"]) + assert.Equal(t, "tenant000", result["__yao_tenant_id"]) + assert.Nil(t, result["__yao_created_by"]) // Should not be copied +} + +func TestCopyUpdateScopePartial(t *testing.T) { + source := map[string]interface{}{ + "name": "Original", + "__yao_updated_by": "user456", + "__yao_team_id": "team789", + } + + dest := map[string]interface{}{ + "name": "Updated", + } + + result := CopyUpdateScope(source, dest) + assert.Equal(t, "Updated", result["name"]) + assert.Equal(t, "user456", result["__yao_updated_by"]) + assert.Equal(t, "team789", result["__yao_team_id"]) + assert.Nil(t, result["__yao_tenant_id"]) +} + +func TestCopyUpdateScopeEmpty(t *testing.T) { + source := map[string]interface{}{ + "name": "Original", + } + + dest := map[string]interface{}{ + "name": "Updated", + } + + result := CopyUpdateScope(source, dest) + assert.Equal(t, "Updated", result["name"]) + assert.Nil(t, result["__yao_updated_by"]) + assert.Nil(t, result["__yao_team_id"]) + assert.Nil(t, result["__yao_tenant_id"]) +} + +func TestCopyCreateScopeRealWorld(t *testing.T) { + // Simulate real-world scenario from team.go + authInfo := &AuthorizedInfo{ + UserID: "063254760529", + TeamID: "242182710786", + TenantID: "tenant789", + } + + // This mimics: teamData := authInfo.WithCreateScope(...) + teamData := authInfo.WithCreateScope(map[string]interface{}{ + "name": "A", + "description": "AAAA", + }) + + // Verify teamData has the scope fields + assert.Equal(t, "063254760529", teamData["__yao_created_by"]) + assert.Equal(t, "242182710786", teamData["__yao_team_id"]) + assert.Equal(t, "tenant789", teamData["__yao_tenant_id"]) + + // Now copy from teamData to ownerMemberData + ownerMemberData := CopyCreateScope(teamData, map[string]interface{}{ + "team_id": "242182710786", + "user_id": "063254760529", + "member_type": "user", + "role_id": "owner:free", + "status": "active", + }) + + // Verify the scope fields were copied + assert.Equal(t, "063254760529", ownerMemberData["__yao_created_by"], "created_by should be copied from source") + assert.Equal(t, "242182710786", ownerMemberData["__yao_team_id"], "team_id should be copied from source") + assert.Equal(t, "tenant789", ownerMemberData["__yao_tenant_id"], "tenant_id should be copied from source") + + // Verify dest data is preserved + assert.Equal(t, "242182710786", ownerMemberData["team_id"]) + assert.Equal(t, "063254760529", ownerMemberData["user_id"]) + assert.Equal(t, "user", ownerMemberData["member_type"]) + assert.Equal(t, "owner:free", ownerMemberData["role_id"]) + assert.Equal(t, "active", ownerMemberData["status"]) +} + +func TestCopyScopesIntegration(t *testing.T) { + // Simulate a create operation + originalRecord := map[string]interface{}{ + "id": 1, + "name": "Original Team", + "__yao_created_by": "user123", + "__yao_team_id": "team456", + "__yao_tenant_id": "tenant789", + } + + // Copy to create a child record + childData := map[string]interface{}{ + "name": "Child Record", + "parent_id": 1, + } + childResult := CopyCreateScope(originalRecord, childData) + assert.Equal(t, "Child Record", childResult["name"]) + assert.Equal(t, 1, childResult["parent_id"]) + assert.Equal(t, "user123", childResult["__yao_created_by"]) + assert.Equal(t, "team456", childResult["__yao_team_id"]) + assert.Equal(t, "tenant789", childResult["__yao_tenant_id"]) + + // Simulate an update operation + updateRecord := map[string]interface{}{ + "id": 1, + "name": "Updated Team", + "__yao_created_by": "user123", + "__yao_updated_by": "user999", + "__yao_team_id": "team456", + "__yao_tenant_id": "tenant789", + } + + updateData := map[string]interface{}{ + "description": "Updated description", + } + updateResult := CopyUpdateScope(updateRecord, updateData) + assert.Equal(t, "Updated description", updateResult["description"]) + assert.Equal(t, "user999", updateResult["__yao_updated_by"]) + assert.Equal(t, "team456", updateResult["__yao_team_id"]) + assert.Equal(t, "tenant789", updateResult["__yao_tenant_id"]) + assert.Nil(t, updateResult["__yao_created_by"]) // Should not be copied +} diff --git a/openapi/user/team.go b/openapi/user/team.go index 39f8b85f..55785425 100644 --- a/openapi/user/team.go +++ b/openapi/user/team.go @@ -16,6 +16,7 @@ import ( "github.com/yaoapp/yao/openapi/oauth" "github.com/yaoapp/yao/openapi/oauth/authorized" "github.com/yaoapp/yao/openapi/oauth/providers/user" + "github.com/yaoapp/yao/openapi/oauth/types" "github.com/yaoapp/yao/openapi/response" ) @@ -149,14 +150,16 @@ func GinTeamGet(c *gin.Context) { // GinTeamCreate handles POST /teams - Create user team func GinTeamCreate(c *gin.Context) { // Get authorized user info - authInfo := oauth.GetAuthorizedInfo(c) - if authInfo == nil || authInfo.UserID == "" { - errorResp := &response.ErrorResponse{ - Code: response.ErrInvalidClient.Code, - ErrorDescription: "User not authenticated", + authInfo := authorized.GetInfo(c) + if authInfo.Constraints.OwnerOnly { + if authInfo == nil || authInfo.UserID == "" { + errorResp := &response.ErrorResponse{ + Code: response.ErrInvalidClient.Code, + ErrorDescription: "User not authenticated", + } + response.RespondWithError(c, response.StatusUnauthorized, errorResp) + return } - response.RespondWithError(c, response.StatusUnauthorized, errorResp) - return } // Parse request body @@ -171,10 +174,10 @@ func GinTeamCreate(c *gin.Context) { } // Prepare team data - teamData := maps.MapStrAny{ + teamData := authInfo.WithCreateScope(maps.MapStrAny{ "name": req.Name, "description": req.Description, - } + }) // Add settings if provided if req.Settings != nil { @@ -745,7 +748,7 @@ func teamCreate(ctx context.Context, userID string, teamData maps.MapStrAny) (st } // Add the creator as an owner member of the team - ownerMemberData := maps.MapStrAny{ + ownerMemberData := types.CopyCreateScope(teamData, maps.MapStrAny{ "team_id": teamID, "user_id": userID, "member_type": "user", @@ -754,7 +757,7 @@ func teamCreate(ctx context.Context, userID string, teamData maps.MapStrAny) (st "joined_at": time.Now(), "created_at": time.Now(), "updated_at": time.Now(), - } + }) _, err = provider.CreateMember(ctx, ownerMemberData) if err != nil {