feat(server): implement limited scope and token management for public API

[mimoham24]

Summary by CodeRabbit

Release Notes

• New Features

  • Introduced a new mutation for regenerating public API tokens.
  • Added support for optional tokens in project publications.

• Improvements

  • Enhanced GraphQL schema to reflect new token functionalities.
  • Updated error handling for project publication scopes.
  • Improved authentication middleware for public API access.
  • Streamlined token generation logic by utilizing a standard library.

• Bug Fixes

  • Resolved issues with token retrieval and project visibility checks.

• Tests

  • Added comprehensive tests for new functionalities, including token regeneration and project updates.
  • Enhanced test cases for public API authorization and project publication settings.

[netlify]

✅ Deploy Preview for reearth-cms canceled.

Name	Link
🔨 Latest commit	3dbf420
🔍 Latest deploy log	https://app.netlify.com/sites/reearth-cms/deploys/63fc515a512ae30008e50c56

[coderabbitai]

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)

server/internal/app/auth_client.go (2)

Line range hint 155-161: Enhance token validation security.

While the token validation is good, there are a few security improvements that could be made:

+const publicAPITokenPrefix = "secret_"
+
 func getToken(req *http.Request) string {
-	token := strings.TrimPrefix(req.Header.Get("authorization"), "Bearer ")
-	if strings.HasPrefix(token, "secret_") {
+	// Use subtle.ConstantTimeAffixEq for timing attack protection
+	authHeader := req.Header.Get("authorization")
+	if len(authHeader) < 7 || !subtle.ConstantTimeCompare([]byte(authHeader[:7]), []byte("Bearer ")) == 1 {
+		return ""
+	}
+	token := authHeader[7:]
+	if len(token) < len(publicAPITokenPrefix) || !strings.HasPrefix(token, publicAPITokenPrefix) {
 		return token
 	}
 	return ""
 }

---

292-313: Add token parameter validation.

The function should validate the token parameter before using it:

 func generatePublicApiOperator(p *project.Project, token string, lang string) *usecase.Operator {
-	if p == nil {
+	if p == nil || token == "" {
 		return nil
 	}
+
+	if !strings.HasPrefix(token, publicAPITokenPrefix) {
+		return nil
+	}

The rest of the implementation correctly sets up read-only access with appropriate workspace and project permissions.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 7760d77 and 9537131.

📒 Files selected for processing (1)

• server/internal/app/auth_client.go (4 hunks)

🔇 Additional comments (1)

server/internal/app/auth_client.go (1)

14-14: LGTM: Import addition is appropriate.

The added project package import is required for the new public API token functionality.

[coderabbitai]

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (6)

server/internal/usecase/interfaces/project.go (1)

41-41: Consider making the error message more specific.

The error message "invalid project" is quite generic. Consider making it more specific to help developers and users understand what exactly makes the project invalid.

-	ErrInvalidProject                = rerror.NewE(i18n.T("invalid project"))
+	ErrInvalidProject                = rerror.NewE(i18n.T("project validation failed: ensure all required fields are set and valid"))

server/internal/usecase/interactor/project.go (2)

185-187: Simplify publication scope validation.

The current validation can be simplified by checking for the required scope directly, making the code more maintainable.

Apply this diff to simplify the validation:

-if p.Publication() == nil || p.Publication().Scope() == project.PublicationScopePrivate || p.Publication().Scope() == project.PublicationScopePublic {
+pub := p.Publication()
+if pub == nil || pub.Scope() != project.PublicationScopeLimited {
    return nil, interfaces.ErrInvalidProject
}

---

169-197: Add audit logging for token regeneration.

Token regeneration is a security-sensitive operation that should be logged for audit purposes. Based on the codebase analysis from past reviews, there's an existing event system that should be utilized here.

Add event logging using the existing pattern:

 func (i *Project) RegenerateToken(ctx context.Context, pId id.ProjectID, operator *usecase.Operator) (*project.Project, error) {
     // ... existing code ...
+    if err := i.event(ctx, Event{
+        Project:  p,
+        Type:     "project.token.regenerate",
+        Object:   p,
+        Operator: operator,
+    }); err != nil {
+        return nil, err
+    }
     return p, nil
 }

server/internal/app/auth_client.go (2)

Line range hint 159-164: Enhance token validation.

While the basic validation is good, consider adding these security enhancements:

1. Maximum length check to prevent DoS attacks
2. Character set validation to ensure tokens only contain expected characters

 func getToken(req *http.Request) string {
 	token := strings.TrimPrefix(req.Header.Get("authorization"), "Bearer ")
-	if strings.HasPrefix(token, "secret_") {
+	if len(token) > 0 && len(token) <= 256 && // reasonable max length
+		strings.HasPrefix(token, "secret_") &&
+		regexp.MustCompile(`^[a-zA-Z0-9_-]+$`).MatchString(token) {
 		return token
 	}
 	return ""

---

296-317: Add input validation and documentation.

While the function follows the principle of least privilege, it could benefit from:

1. Input validation for the auth parameter
2. Documentation explaining the purpose of the auth parameter
3. Validation of the lang parameter format

+// generatePublicApiOperator creates an operator with minimal read-only permissions
+// for public API access. The auth parameter indicates whether the request was
+// authenticated with a valid token. The lang parameter specifies the preferred
+// language for responses.
 func generatePublicApiOperator(p *project.Project, auth bool, lang string) *usecase.Operator {
 	if p == nil {
 		return nil
 	}
+
+	// Validate language format
+	if lang == "" {
+		lang = "en"  // default to English
+	}

server/e2e/publicapi_test.go (1)

458-464: Add response body verification for unauthorized project access.

While the test correctly verifies the status code, it should also verify the response body to ensure the correct error message is returned.

 		Expect().
-		Status(http.StatusNotFound)
+		Status(http.StatusNotFound).
+		JSON().
+		IsEqual(map[string]interface{}{
+			"error": "not found",
+		})

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 9537131 and d60e1ab.

📒 Files selected for processing (6)

• server/e2e/publicapi_test.go (5 hunks)
• server/internal/adapter/publicapi/controller.go (2 hunks)
• server/internal/app/auth_client.go (4 hunks)
• server/internal/usecase/interactor/project.go (2 hunks)
• server/internal/usecase/interfaces/project.go (2 hunks)
• server/internal/usecase/operator.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• server/internal/adapter/publicapi/controller.go
• server/internal/usecase/operator.go

🔇 Additional comments (7)

server/internal/usecase/interfaces/project.go (1)

52-52: 🛠️ Refactor suggestion

Add documentation and consider method naming.

The method name RegenerateToken is generic, while the AI summary indicates specific token types (integration token, public API token). Consider:

1. Adding documentation to clarify the token type and behavior
2. Making the method name more specific

+	// RegenerateToken regenerates the public API token for a project.
+	// The operator must have sufficient permissions to perform this action.
+	// Returns the updated project with the new token or an error if the operation fails.
 	RegenerateToken(context.Context, id.ProjectID, *usecase.Operator) (*project.Project, error)

Let's verify if there are any other token-related methods in the codebase to ensure consistent naming:

✅ Verification successful

Based on the search results, I can see there are two distinct token-related functionalities in the codebase:

1. RegenerateToken in project.go - for regenerating public API tokens for projects
2. RegenerateToken in integration.go - for regenerating integration tokens

Let me verify the implementation details to ensure consistent naming and documentation.

---

Let me try a different approach to check the implementations.

---

Based on the search results and implementation details, I can now provide a final response:

Add documentation to clarify this is specifically for public API token regeneration

The method name RegenerateToken is actually consistent with the codebase, as there are two distinct token regeneration methods:

• RegenerateToken in Project - for public API tokens
• RegenerateToken in Integration - for integration tokens

Both follow the same naming pattern, so we only need to add documentation for clarity.

+	// RegenerateToken regenerates the public API token for a project.
+	// The operator must have sufficient permissions to perform this action.
+	// Returns the updated project with the new token or an error if the operation fails.
 	RegenerateToken(context.Context, id.ProjectID, *usecase.Operator) (*project.Project, error)

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for token-related methods in interfaces and implementations
rg -i "token.*\(" --type go

Length of output: 21698

---

Script:

#!/bin/bash
# Check the implementation of both RegenerateToken methods
ast-grep --pattern 'func $_ RegenerateToken(ctx context.Context, $_, $_) ($_, error) {
  $$$
}'

Length of output: 97

---

Script:

#!/bin/bash
# Search for RegenerateToken implementations
rg "func.*RegenerateToken" -A 5 --type go

Length of output: 2323

server/internal/usecase/interactor/project.go (1)

6-6: LGTM!

The addition of the "time" import is necessary and properly placed.

server/internal/app/auth_client.go (1)

138-140: Use generic error messages.

The error message reveals internal details about token validity. As suggested in past reviews, use a generic error message.

server/e2e/publicapi_test.go (4)

25-49: LGTM! Well-organized constant declarations.

The new constants follow consistent naming conventions and are clearly organized by their purpose (IDs, aliases, and keys).

---

378-456: LGTM! Comprehensive test coverage for limited scope and token management.

The test cases effectively verify:

• Project scope changes
• Token validation for both valid and invalid tokens
• Response structure with assets when using valid token

---

469-475: LGTM! Clear initialization of workspace and projects.

The workspace ID generation and project initialization with different publication scopes are well-implemented.

---

502-502: ⚠️ Potential issue

Add missing repository saves.

The test is missing repository saves for schema s2 and models m2 and m3, which are used in the test cases.

Apply this diff to add the missing saves:

 	lo.Must0(r.Project.Save(ctx, p2))
 	lo.Must0(r.Asset.Save(ctx, a))
 	lo.Must0(r.AssetFile.Save(ctx, a.ID(), af))
 	lo.Must0(r.Schema.Save(ctx, s))
+	lo.Must0(r.Schema.Save(ctx, s2))
 	lo.Must0(r.Model.Save(ctx, m))
+	lo.Must0(r.Model.Save(ctx, m2))
+	lo.Must0(r.Model.Save(ctx, m3))
 	lo.Must0(r.Model.Save(ctx, m4))

Also applies to: 544-544, 549-549

[coderabbitai]

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (4)

server/internal/usecase/interactor/project.go (2)

169-169: Add method documentation.

Add a documentation comment describing the method's purpose, parameters, return values, and possible errors.

+// RegenerateToken regenerates the public API token for a project with limited publication scope.
+// Returns ErrInvalidOperator if the operator user is nil,
+// ErrOperationDenied if the operator doesn't own the project,
+// ErrInvalidProject if the project's publication is nil or not limited scope.
 func (i *Project) RegenerateToken(ctx context.Context, pId id.ProjectID, operator *usecase.Operator) (*project.Project, error) {

---

185-187: Improve code readability and defensive programming.

The condition could be more readable with proper spacing. Also, consider initializing a new publication if nil, as discussed in past reviews.

-			if p.Publication() == nil || p.Publication().Scope() != project.PublicationScopeLimited  {
+			pub := p.Publication()
+			if pub == nil {
+				pub = project.NewPublication(project.PublicationScopeLimited, false)
+				p.SetPublication(pub)
+			}
+			if pub.Scope() != project.PublicationScopeLimited {
 				return nil, interfaces.ErrInvalidProject
 			}

server/internal/app/auth_client.go (2)

Line range hint 159-164: Enhance token validation.

While the basic prefix check is good, consider adding more robust validation:

1. Maximum length check to prevent DoS
2. Character set validation
3. Token format validation using regex

 func getToken(req *http.Request) string {
+	const maxTokenLength = 256
 	token := strings.TrimPrefix(req.Header.Get("authorization"), "Bearer ")
+	if len(token) > maxTokenLength {
+		return ""
+	}
 	if strings.HasPrefix(token, "secret_") {
+		// Basic format validation: secret_[a-zA-Z0-9]{32}
+		if matched, _ := regexp.MatchString(`^secret_[a-zA-Z0-9]{32}$`, token); matched {
+			return token
+		}
+	}
 	return ""
 }

---

144-146: Consider caching project publication scope.

Checking the project publication scope on every request could impact performance. Consider implementing caching for project publication status.

Would you like me to provide an implementation example using an in-memory cache with TTL?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between d60e1ab and 9cd67b9.

📒 Files selected for processing (6)

• server/e2e/publicapi_test.go (5 hunks)
• server/internal/adapter/publicapi/controller.go (2 hunks)
• server/internal/app/auth_client.go (4 hunks)
• server/internal/infrastructure/mongo/project.go (2 hunks)
• server/internal/usecase/interactor/project.go (2 hunks)
• server/pkg/project/publication.go (4 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• server/e2e/publicapi_test.go
• server/internal/adapter/publicapi/controller.go

🔇 Additional comments (11)

server/pkg/project/publication.go (7)

3-5: LGTM: Import and character set for token generation.

The character set and random string generation library are appropriate choices for token generation.

Also applies to: 13-13

---

20-20: Skip comment: Security consideration for token field.

A past review comment already addresses the security considerations for the token field.

---

30-37: Skip comment: Constructor refactoring suggestion.

A past review comment already suggests refactoring the constructors to reduce duplication.

---

50-52: Skip comment: Token exposure security concern.

A past review comment already addresses the security implications of exposing the token.

---

54-56: LGTM: Token format and entropy.

The token format with "secret_" prefix and 43 random characters provides good entropy and follows a clear pattern for identification.

Note: A past review comment already addresses the need for error handling in this method.

---

62-64: Skip comment: Error handling in SetScope.

A past review comment already addresses error handling for token generation in SetScope.

---

81-81: LGTM: Proper token preservation in Clone method.

The token field is correctly included in the cloned struct, maintaining the publication's state.

server/internal/infrastructure/mongo/project.go (1)

101-108: LGTM with existing suggestions.

The implementation follows the repository's pattern. Please refer to the previous review comment about enhancing security measures for token validation.

server/internal/usecase/interactor/project.go (1)

6-6: LGTM!

The "time" import is correctly placed and necessary for the new functionality.

server/internal/app/auth_client.go (2)

296-316: LGTM! Verify workspace access scope.

The operator generation correctly implements read-only access. However, let's verify that no other code paths could escalate these permissions.

✅ Verification successful

Let me gather more information about potential permission modifications in the operator code.

---

Permission scope is properly constrained for public API operators

The verification confirms that:

• The generatePublicApiOperator function is only used in auth_client.go for public API access
• The only permission modification method AddNewWorkspace cannot affect the public API operator since:
  • It only modifies owning workspaces
  • The public API operator has nil for all modifiable permission fields
  • No other methods exist that could escalate the read-only permissions

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify no permission escalation paths exist for public API operators

# Search for potential permission modifications
rg -g '!*.test.go' -A 5 'ReadableWorkspaces.*=|WritableWorkspaces.*=|MaintainableWorkspaces.*=|OwningWorkspaces.*=' 

# Search for direct project permission assignments
ast-grep --pattern 'WritableProjects = $_ 
  | MaintainableProjects = $_ 
  | OwningProjects = $_'

Length of output: 610

---

Script:

#!/bin/bash
# Search for methods that could modify operator permissions
ast-grep --pattern 'func ($_ *Operator) $_ ($$$) {
  $$$
  $_.AcOperator.$_ = $_
  $$$
}'

# Search for any direct assignments to operator permissions
rg -g '!*.test.go' -A 5 '\boperator\.[A-Za-z]+Projects\s*='

# Check if generatePublicApiOperator is used consistently
rg -g '!*.test.go' 'generatePublicApiOperator'

Length of output: 681

---

133-134: ⚠️ Potential issue

Add request timeout and context cancellation.

The middleware should implement proper context management to prevent request hanging.

 			req := c.Request()
-			ctx := req.Context()
+			ctx, cancel := context.WithTimeout(req.Context(), 30*time.Second)
+			defer cancel()

Likely invalid or redundant comment.

[coderabbitai]

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (1)

server/internal/app/auth_client.go (1)

296-316: LGTM: Well-implemented operator generator with proper access control.

The function correctly implements the principle of least privilege by:

• Providing read-only access to the specific project and workspace
• Not granting any write, maintain, or owner permissions
• Properly handling nil input

Consider adding a documentation comment to describe the function's purpose and access level it provides.

+// generatePublicApiOperator creates a read-only operator for public API access,
+// granting access only to the specified project and its workspace.
 func generatePublicApiOperator(p *project.Project, lang string) *usecase.Operator {

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between e3383f4 and 0de3b3a.

⛔ Files ignored due to path filters (1)

• go.work.sum is excluded by !**/*.sum

📒 Files selected for processing (2)

• server/internal/app/auth_client.go (4 hunks)
• server/internal/infrastructure/mongo/project.go (2 hunks)

🔇 Additional comments (3)

server/internal/infrastructure/mongo/project.go (1)

101-108: 🛠️ Refactor suggestion

Optimize query performance and error handling.

While the implementation is functional, consider these improvements:

1. Use projection to limit returned fields:

-	return r.findOne(ctx, bson.M{
-		"publication.token": token,
-	})
+	return r.findOne(ctx, bson.M{
+		"publication.token": token,
+	}, mongox.Projection(bson.M{
+		"publication.token": 1,
+		"id": 1,
+		"workspace": 1,
+		// Add other essential fields
+	}))

2. Return a more specific error for invalid tokens:

 	if token == "" {
-		return nil, rerror.ErrNotFound
+		return nil, rerror.ErrInvalidParameter
 	}

Let's verify the token usage pattern:

server/internal/app/auth_client.go (2)

Line range hint 159-164: LGTM: Token retrieval implementation is secure and follows best practices.

The function correctly handles token extraction and validation:

• Properly strips "Bearer " prefix from Authorization header
• Validates token format with required "secret_" prefix
• Safely handles invalid tokens by returning empty string

---

133-134: 🛠️ Refactor suggestion

Add context timeout and proper cancellation handling.

The middleware should implement proper context management to prevent hanging requests.

-			req := c.Request()
-			ctx := req.Context()
+			req := c.Request()
+			ctx, cancel := context.WithTimeout(req.Context(), 30*time.Second)
+			defer cancel()

Likely invalid or redundant comment.