server: set role admin for no auth mode

[shreeharsha-factly]

Summary by CodeRabbit

Bug Fixes

• Fixed API key authentication to correctly establish user identity and organization context for authenticated requests
• Enhanced role-based access control with validation that rejects requests from users lacking required organization-level permissions

[claude]

Claude finished @shreeharsha-factly's task —— View job

---

Code Review Complete ✅

I've analyzed the changes in this PR that adds admin role assignment for "no auth mode" scenarios. Here's my comprehensive review:

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze modified middleware files
• Review for code quality and best practices
• Check for potential bugs and security issues
• Assess performance considerations
• Evaluate test coverage needs
• Provide comprehensive feedback

---

Review Findings

✅ Code Quality & Best Practices

The changes are clean and follow Go conventions:

• Proper import organization with the new models import
• Consistent use of context keys (RoleCtxKey)
• Maintains existing code style and patterns

⚠️ Security Concerns & Issues

1. Overly Permissive Default Role Assignment

• Files: apikey.go:50, auth.go:73, auth.go:100
• Issue: All "no auth mode" scenarios automatically assign models.Admin role without any validation
• Impact: This grants maximum privileges by default, which violates the principle of least privilege
• Recommendation: Consider using models.Member as default, or add configuration to control the default role

2. Inconsistent Role Assignment Logic

• File: role.go:38-42
• Issue: The added error handling block returns 403 when orgsRole == "" but this logic appears to duplicate existing behavior
• Concern: The original logic didn't have explicit error handling for empty roles - this change might alter existing behavior unexpectedly

🐛 Potential Bugs

1. Duplicate Error Handling in role.go

• Lines: 38-42
• Issue: The added check for empty orgsRole may be redundant with existing logic
• Risk: Could change error response patterns for existing users

2. Missing Role Validation

• All files: No validation that models.Admin is a valid role value
• Risk: Runtime issues if the Role constants change

⚡ Performance Considerations

• Impact: Minimal - only adds simple variable assignments
• Concern: No performance issues identified

🧪 Test Coverage

Major Gap: No tests found for middleware components

• Missing: Unit tests for all three modified middleware functions
• Recommendation: Add tests covering:
  • API key auth with role assignment
  • Header auth with admin role
  • API auth with admin role
  • Role authorization error scenarios

📋 Specific Recommendations

1. Security: Consider making the default role configurable:

   defaultRole := viper.GetString("default_no_auth_role") // fallback to Member
   if defaultRole == "" {
       defaultRole = string(models.Member)
   }
   ctx.Locals(RoleCtxKey, models.Role(defaultRole))

2. Documentation: Add comments explaining why admin role is assigned in no-auth scenarios

3. Testing: Implement comprehensive middleware tests

4. Role Validation: Add validation for role assignments:

   role := models.Admin
   if role != models.Admin && role != models.Member {
       // handle invalid role
   }

🎯 Summary

This PR successfully implements the feature but raises security concerns by defaulting to admin privileges. The changes are functionally sound but would benefit from a more security-conscious approach to role assignment and comprehensive test coverage.

Severity: Medium - functional but has security implications that should be addressed.

---

[coderabbitai]

📝 Walkthrough

Walkthrough

These changes modify the API authentication and authorization middleware to assign a default admin role to authenticated users and enforce that users must have a role within their organization, returning a 403 error if none exists.

Changes

Cohort / File(s)	Summary
Role Context Assignment server/interfaces/http/middleware/apikey.go, server/interfaces/http/middleware/auth.go	Sets RoleCtxKey to models.Admin in the request context after successful API key and OAuth authentication, establishing a default admin role for downstream handlers.
Role Validation Guard server/interfaces/http/middleware/role.go	Adds early exit logic in RoleAuthorization to return 403 Forbidden with JSON error response when a user lacks a role assignment in the current organization, preventing further processing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 With keys and tokens, admins rise,
Roles are set before their eyes,
But none? A 403 denies!
Middleware guards with rabbit's sighs... 🔐

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title refers to a real aspect of the changes (setting admin role in auth middleware), but does not accurately represent the main changes which include role authorization guard logic and context assignments across multiple middleware files.	Consider revising the title to be more specific about the actual changes, such as 'server: set admin role in auth middleware and add role authorization guard' to better reflect the full scope of the modifications.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/roles

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

server/interfaces/http/middleware/role.go (1)

38-43: Consider adding consistent error response format.

The error response here uses "error" and "message" fields, but apikey.go includes a "code" field in its responses. For API consistency, consider including the status code in the response body.

♻️ Optional: Add code field for consistency

 		if orgsRole == "" {
 			return c.Status(fiber.StatusForbidden).JSON(fiber.Map{
 				"error":   "forbidden",
 				"message": "user does not have a role in this organization",
+				"code":    fiber.StatusForbidden,
 			})
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/interfaces/http/middleware/role.go` around lines 38 - 43, The response
returned when orgsRole == "" uses "error" and "message" but lacks the numeric
"code" field used elsewhere (e.g., apikey.go); update the JSON payload in the
role-check branch to include a "code" key set to the HTTP status
(fiber.StatusForbidden) so the response shape matches apikey.go, and mirror this
change for any other error returns in the same middleware (look for the orgsRole
check and the surrounding middleware function) to keep API error format
consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@server/interfaces/http/middleware/role.go`:
- Around line 38-43: The response returned when orgsRole == "" uses "error" and
"message" but lacks the numeric "code" field used elsewhere (e.g., apikey.go);
update the JSON payload in the role-check branch to include a "code" key set to
the HTTP status (fiber.StatusForbidden) so the response shape matches apikey.go,
and mirror this change for any other error returns in the same middleware (look
for the orgsRole check and the surrounding middleware function) to keep API
error format consistent.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1e7e888 and aa01491.

📒 Files selected for processing (3)

• server/interfaces/http/middleware/apikey.go
• server/interfaces/http/middleware/auth.go
• server/interfaces/http/middleware/role.go