-
Notifications
You must be signed in to change notification settings - Fork 621
UN-2407 [FEAT] Add global API deployment keys for grouped API deployments #1881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,3 +9,4 @@ class DeploymentExecutionDTO: | |
|
|
||
| api: APIDeployment | ||
| api_key: str | ||
| is_global_key: bool = False | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| from django.apps import AppConfig | ||
|
|
||
|
|
||
| class GlobalApiDeploymentKeyConfig(AppConfig): | ||
| default_auto_field = "django.db.models.BigAutoField" | ||
| name = "global_api_deployment_key" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| # Generated by Django 4.2.1 on 2026-03-25 18:22 | ||
|
|
||
| import uuid | ||
|
|
||
| import django.db.models.deletion | ||
| from django.conf import settings | ||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| initial = True | ||
|
|
||
| dependencies = [ | ||
| migrations.swappable_dependency(settings.AUTH_USER_MODEL), | ||
| ("account_v2", "0004_user_is_service_account"), | ||
| ("api_v2", "0003_add_organization_rate_limit"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.CreateModel( | ||
| name="GlobalApiDeploymentKey", | ||
| fields=[ | ||
| ("created_at", models.DateTimeField(auto_now_add=True)), | ||
| ("modified_at", models.DateTimeField(auto_now=True)), | ||
| ( | ||
| "id", | ||
| models.UUIDField( | ||
| default=uuid.uuid4, | ||
| editable=False, | ||
| primary_key=True, | ||
| serialize=False, | ||
| ), | ||
| ), | ||
| ("name", models.CharField(max_length=128)), | ||
| ("description", models.TextField(max_length=512)), | ||
| ("key", models.UUIDField(default=uuid.uuid4, unique=True)), | ||
| ("is_active", models.BooleanField(default=True)), | ||
| ( | ||
| "allow_all_deployments", | ||
| models.BooleanField( | ||
| db_comment="If True, this key can authenticate any API deployment in the org", | ||
| default=False, | ||
| ), | ||
| ), | ||
| ( | ||
| "api_deployments", | ||
| models.ManyToManyField( | ||
| blank=True, | ||
| related_name="global_api_deployment_keys", | ||
| to="api_v2.apideployment", | ||
| ), | ||
| ), | ||
| ( | ||
| "created_by", | ||
| models.ForeignKey( | ||
| null=True, | ||
| on_delete=django.db.models.deletion.SET_NULL, | ||
| related_name="global_api_deployment_keys_created", | ||
| to=settings.AUTH_USER_MODEL, | ||
| ), | ||
| ), | ||
| ( | ||
| "modified_by", | ||
| models.ForeignKey( | ||
| null=True, | ||
| on_delete=django.db.models.deletion.SET_NULL, | ||
| related_name="+", | ||
| to=settings.AUTH_USER_MODEL, | ||
| ), | ||
| ), | ||
| ( | ||
| "organization", | ||
| models.ForeignKey( | ||
| blank=True, | ||
| db_comment="Foreign key reference to the Organization model.", | ||
| default=None, | ||
| null=True, | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| to="account_v2.organization", | ||
| ), | ||
| ), | ||
|
Comment on lines
+71
to
+81
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check how organization FKs are typically defined in this codebase
rg -n "ForeignKey.*organization" --type py -A3 -B1 | head -80Repository: Zipstack/unstract Length of output: 43 🏁 Script executed: # First, let's find and read the migration file mentioned
find . -path "*global_api_deployment_key/migrations/0001_initial.py" -type fRepository: Zipstack/unstract Length of output: 123 🏁 Script executed: # Search for the GlobalApiDeploymentKey model definition
rg -n "class GlobalApiDeploymentKey" --type py -A 30Repository: Zipstack/unstract Length of output: 13214 🏁 Script executed: # Check for viewsets related to global_api_deployment_key
fd -t f "viewset" | xargs rg -l "GlobalApiDeploymentKey" --type pyRepository: Zipstack/unstract Length of output: 370 🏁 Script executed: # Look for other models with nullable organization FKs to understand the pattern
rg -n "organization.*ForeignKey" --type py -B 2 -A 5Repository: Zipstack/unstract Length of output: 1884 🏁 Script executed: cat -n backend/global_api_deployment_key/migrations/0001_initial.py | head -100Repository: Zipstack/unstract Length of output: 4113 🏁 Script executed: # Check the full Meta constraints in the model
cat -n backend/global_api_deployment_key/models.py | tail -20Repository: Zipstack/unstract Length of output: 945 🏁 Script executed: # Check if there are any data migrations or model updates after the initial migration
ls -la backend/global_api_deployment_key/migrations/Repository: Zipstack/unstract Length of output: 297 Organization FK nullability follows codebase pattern but allows theoretical duplicate names at DB level. The If organization should always be present at the application level, consider whether the mixin pattern should be reconsidered for this model, or document why nullability is needed. 🤖 Prompt for AI Agents |
||
| ], | ||
| options={ | ||
| "db_table": "global_api_deployment_key", | ||
| }, | ||
| ), | ||
| migrations.AddConstraint( | ||
| model_name="globalapideploymentkey", | ||
| constraint=models.UniqueConstraint( | ||
| fields=("name", "organization"), | ||
| name="unique_global_api_deployment_key_name_per_org", | ||
| ), | ||
| ), | ||
| ] | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,58 @@ | ||||||||||||||
| import uuid | ||||||||||||||
|
|
||||||||||||||
| from account_v2.models import User | ||||||||||||||
| from api_v2.models import APIDeployment | ||||||||||||||
| from django.db import models | ||||||||||||||
| from utils.models.base_model import BaseModel | ||||||||||||||
| from utils.models.organization_mixin import DefaultOrganizationMixin | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| class GlobalApiDeploymentKey(DefaultOrganizationMixin, BaseModel): | ||||||||||||||
| id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) | ||||||||||||||
| name = models.CharField(max_length=128) | ||||||||||||||
| description = models.TextField(max_length=512) | ||||||||||||||
| key = models.UUIDField(default=uuid.uuid4, unique=True) | ||||||||||||||
| is_active = models.BooleanField(default=True) | ||||||||||||||
| allow_all_deployments = models.BooleanField( | ||||||||||||||
| default=False, | ||||||||||||||
| db_comment="If True, this key can authenticate any API deployment in the org", | ||||||||||||||
|
Comment on lines
+16
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Default scope is overly permissive. Line 16/17 sets 🔐 Proposed fix allow_all_deployments = models.BooleanField(
- default=True,
+ default=False,
db_comment="If True, this key can authenticate any API deployment in the org",
)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| ) | ||||||||||||||
| api_deployments = models.ManyToManyField( | ||||||||||||||
| APIDeployment, | ||||||||||||||
| blank=True, | ||||||||||||||
| related_name="global_api_deployment_keys", | ||||||||||||||
| ) | ||||||||||||||
| created_by = models.ForeignKey( | ||||||||||||||
| User, | ||||||||||||||
| on_delete=models.SET_NULL, | ||||||||||||||
| null=True, | ||||||||||||||
| related_name="global_api_deployment_keys_created", | ||||||||||||||
| ) | ||||||||||||||
| modified_by = models.ForeignKey( | ||||||||||||||
| User, | ||||||||||||||
| on_delete=models.SET_NULL, | ||||||||||||||
| null=True, | ||||||||||||||
| related_name="+", | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| class Meta: | ||||||||||||||
| db_table = "global_api_deployment_key" | ||||||||||||||
| constraints = [ | ||||||||||||||
| models.UniqueConstraint( | ||||||||||||||
| fields=["name", "organization"], | ||||||||||||||
| name="unique_global_api_deployment_key_name_per_org", | ||||||||||||||
| ), | ||||||||||||||
| ] | ||||||||||||||
|
|
||||||||||||||
| def __str__(self): | ||||||||||||||
| return f"{self.name} ({self.organization})" | ||||||||||||||
|
|
||||||||||||||
| def has_access_to_deployment(self, api_deployment): | ||||||||||||||
| """Check if this key can authenticate the given API deployment.""" | ||||||||||||||
| if not self.is_active: | ||||||||||||||
| return False | ||||||||||||||
| if self.organization_id != api_deployment.organization_id: | ||||||||||||||
| return False | ||||||||||||||
| if self.allow_all_deployments: | ||||||||||||||
| return True | ||||||||||||||
| return self.api_deployments.filter(id=api_deployment.id).exists() | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| from platform_api.permissions import IsOrganizationAdmin | ||
|
|
||
| __all__ = ["IsOrganizationAdmin"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enforce tenant-scoped profile authorization for global keys.
On Lines 395-397, global-key requests skip ownership checks and only require profile existence. That weakens isolation: a caller can potentially use any known
llm_profile_id, including from another org. Add an explicit org-scoped authorization check before returning (and keep a generic error on mismatch).🤖 Prompt for AI Agents