-
Notifications
You must be signed in to change notification settings - Fork 13
Open
Description
Problem Description
Currently, API keys stored in the apikey table are globally shared across all users and groups in the system. This poses security and privacy concerns as:
- Security Risk: Any user can see and potentially use API keys added by other users
- Privacy Concern: Users cannot have personal API keys for their own usage
- Billing Issues: API usage from shared keys cannot be attributed to specific users
- Compliance: In multi-tenant environments, sharing API keys across organizations violates data isolation principles
Current Implementation
The ApiKey model (src/backend/src/models/api_key.py) lacks user or group association:
class ApiKey(Base):
id = Column(Integer, primary_key=True, index=True)
name = Column(String, nullable=False, unique=True, index=True)
encrypted_value = Column(String, nullable=False)
description = Column(String, nullable=True)
created_at = Column(DateTime, default=datetime.utcnow)
updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow)Proposed Solution
Option 1: User-Specific API Keys (Recommended)
Add user_id to the ApiKey model to make keys personal to each user:
class ApiKey(Base):
id = Column(Integer, primary_key=True, index=True)
name = Column(String, nullable=False, index=True)
user_id = Column(Integer, ForeignKey("users.id"), nullable=False, index=True)
encrypted_value = Column(String, nullable=False)
description = Column(String, nullable=True)
created_at = Column(DateTime, default=datetime.utcnow)
updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow)
# Relationship
user = relationship("User", back_populates="api_keys")
# Unique constraint on (name, user_id) instead of just name
__table_args__ = (
UniqueConstraint('name', 'user_id', name='uq_apikey_name_user'),
)Option 2: Group-Scoped API Keys
Follow the existing pattern and add group_id to make keys accessible within groups:
class ApiKey(Base):
id = Column(Integer, primary_key=True, index=True)
name = Column(String, nullable=False, index=True)
group_id = Column(String, ForeignKey("groups.id"), nullable=False, index=True)
encrypted_value = Column(String, nullable=False)
description = Column(String, nullable=True)
created_at = Column(DateTime, default=datetime.utcnow)
updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow)
# Relationship
group = relationship("Group", back_populates="api_keys")
# Unique constraint on (name, group_id)
__table_args__ = (
UniqueConstraint('name', 'group_id', name='uq_apikey_name_group'),
)Option 3: Hybrid Approach
Support both user-specific and group-shared keys:
class ApiKey(Base):
id = Column(Integer, primary_key=True, index=True)
name = Column(String, nullable=False, index=True)
user_id = Column(Integer, ForeignKey("users.id"), nullable=True, index=True)
group_id = Column(String, ForeignKey("groups.id"), nullable=True, index=True)
scope = Column(Enum(ApiKeyScope), nullable=False, default=ApiKeyScope.USER) # USER or GROUP
encrypted_value = Column(String, nullable=False)
description = Column(String, nullable=True)
created_at = Column(DateTime, default=datetime.utcnow)
updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow)
# Check constraint: either user_id or group_id must be set
__table_args__ = (
CheckConstraint('(user_id IS NOT NULL AND group_id IS NULL AND scope = "USER") OR (user_id IS NULL AND group_id IS NOT NULL AND scope = "GROUP")', name='check_apikey_scope'),
UniqueConstraint('name', 'user_id', 'group_id', name='uq_apikey_name_scope'),
)Implementation Requirements
- Database Migration: Create Alembic migration to add user_id/group_id columns
- API Updates:
- Update API endpoints to filter keys by current user/group
- Add user context to API key router using
UserDeporGroupContextDep - Update service layer to handle user/group filtering
- Frontend Updates:
- No changes needed if API contract remains the same
- Consider adding UI indicators for personal vs shared keys (if hybrid approach)
- Data Migration:
- Existing keys could be assigned to a system user or the first admin user
- Or require users to re-enter their API keys
- Testing: Update tests to verify proper isolation
Benefits
- Security: Users can only access their own API keys
- Privacy: Personal API keys remain private
- Accountability: API usage can be tracked per user
- Flexibility: Users can have different keys for different purposes
- Consistency: Aligns with the group isolation pattern used throughout the system
Considerations
- The
memory_backendmodel is already user-specific, showing precedent for user-scoped resources - Most other entities use group_id for multi-tenant isolation
- Need to decide if API keys should be shareable within groups or strictly personal
- Consider backward compatibility for existing API keys
Recommendation
I recommend Option 1 (User-Specific) as API keys are typically personal credentials that shouldn't be shared. This aligns with security best practices and how most platforms handle API keys.
Metadata
Metadata
Assignees
Labels
No labels