Skip to content
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.

Sign up for GitHub

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

Refactor IAM id access #8969

Merged
merged 7 commits into from
Jan 22, 2025
Merged

Refactor IAM id access #8969

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d8f9223
Remove extra request in project serializer
zhiltsov-max Jan 20, 2025
c382cc4
Optimize inefficient related object id access in IAM checks
zhiltsov-max Jan 20, 2025
2864da1
Remove extra change
zhiltsov-max Jan 21, 2025
dd0375e
Remove extra assignment
zhiltsov-max Jan 21, 2025
0bebade
Remove extra code
zhiltsov-max Jan 22, 2025
c64ad72
Merge branch 'develop' into zm/optimize-iam-data-access
zhiltsov-max Jan 22, 2025
271f44a
Merge branch 'develop' into zm/optimize-iam-data-access
zhiltsov-max Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions cvat/apps/engine/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1315,8 +1315,10 @@ class AnnotationGuide(TimestampedModel):
is_public = models.BooleanField(default=False)

@property
def target(self):
return self.project or self.task
def target(self) -> Task | Project:
target = self.project or self.task
assert target # one of the fields must be set
return target

@property
def organization_id(self):
Expand Down
171 changes: 88 additions & 83 deletions cvat/apps/engine/permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
)
from cvat.apps.organizations.models import Organization

from .models import AnnotationGuide, CloudStorage, Issue, Job, Label, Project, Task
from .models import AnnotationGuide, CloudStorage, Comment, Issue, Job, Label, Project, Task, User


def _get_key(d: dict[str, Any], key_path: Union[str, Sequence[str]]) -> Optional[Any]:
Expand Down Expand Up @@ -75,6 +75,8 @@ def get_resource(self):
return None

class UserPermission(OpenPolicyAgentPermission):
obj: Optional[User]

class Scopes(StrEnum):
LIST = 'list'
VIEW = 'view'
Expand Down Expand Up @@ -134,6 +136,8 @@ def get_resource(self):
return data

class CloudStoragePermission(OpenPolicyAgentPermission):
obj: Optional[CloudStorage]

class Scopes(StrEnum):
LIST = 'list'
LIST_CONTENT = 'list:content'
Expand Down Expand Up @@ -196,15 +200,17 @@ def get_resource(self):
elif self.obj:
data = {
'id': self.obj.id,
'owner': { 'id': getattr(self.obj.owner, 'id', None) },
'owner': { 'id': self.obj.owner_id },
'organization': {
'id': self.obj.organization.id
} if self.obj.organization else None
'id': self.obj.organization_id
} if self.obj.organization_id else None
zhiltsov-max marked this conversation as resolved.
Show resolved Hide resolved
}

return data

class ProjectPermission(OpenPolicyAgentPermission):
obj: Optional[Project]

class Scopes(StrEnum):
LIST = 'list'
CREATE = 'create'
Expand Down Expand Up @@ -345,19 +351,17 @@ def get_resource(self):
if self.obj:
data = {
"id": self.obj.id,
"owner": { "id": getattr(self.obj.owner, 'id', None) },
"assignee": { "id": getattr(self.obj.assignee, 'id', None) },
'organization': {
"id": getattr(self.obj.organization, 'id', None)
}
"owner": { "id": self.obj.owner_id },
"assignee": { "id": self.obj.assignee_id },
'organization': { "id": self.obj.organization_id },
}
elif self.scope in [__class__.Scopes.CREATE, __class__.Scopes.IMPORT_BACKUP]:
data = {
"id": None,
"owner": { "id": self.user_id },
"assignee": {
"id": self.assignee_id,
} if getattr(self, 'assignee_id', None) else None,
} if self.assignee_id else None,
'organization': {
"id": self.org_id,
} if self.org_id else None,
Expand All @@ -366,6 +370,8 @@ def get_resource(self):
return data

class TaskPermission(OpenPolicyAgentPermission):
obj: Optional[Task]

class Scopes(StrEnum):
LIST = 'list'
CREATE = 'create'
Expand Down Expand Up @@ -558,17 +564,13 @@ def get_resource(self):
if self.obj:
data = {
"id": self.obj.id,
"owner": { "id": getattr(self.obj.owner, 'id', None) },
"assignee": { "id": getattr(self.obj.assignee, 'id', None) },
'organization': {
"id": getattr(self.obj.organization, 'id', None)
},
"owner": { "id": self.obj.owner_id },
"assignee": { "id": self.obj.assignee_id },
'organization': { "id": self.obj.organization_id },
"project": {
"owner": { "id": getattr(self.obj.project.owner, 'id', None) },
"assignee": { "id": getattr(self.obj.project.assignee, 'id', None) },
'organization': {
"id": getattr(self.obj.project.organization, 'id', None)
},
"owner": { "id": self.obj.project.owner_id },
"assignee": { "id": self.obj.project.assignee_id },
'organization': { "id": self.obj.project.organization_id },
} if self.obj.project else None
}
elif self.scope in [
Expand All @@ -593,18 +595,19 @@ def get_resource(self):
"id": self.org_id
},
"project": {
"owner": { "id": getattr(project.owner, 'id', None) },
"assignee": { "id": getattr(project.assignee, 'id', None) },
"owner": { "id": project.owner_id },
"assignee": { "id": project.assignee_id },
'organization': {
"id": getattr(project.organization, 'id', None),
} if project.organization is not None else None,
"id": project.organization_id,
} if project.organization_id else None,
} if project is not None else None,
}

return data

class JobPermission(OpenPolicyAgentPermission):
task_id: Optional[int]
obj: Optional[Job]

class Scopes(StrEnum):
CREATE = 'create'
Expand Down Expand Up @@ -751,52 +754,51 @@ def get_resource(self):
data = None
if self.obj:
if self.obj.segment.task.project:
organization = self.obj.segment.task.project.organization
organization_id = self.obj.segment.task.project.organization_id
else:
organization = self.obj.segment.task.organization
organization_id = self.obj.segment.task.organization_id

data = {
"id": self.obj.id,
"assignee": { "id": getattr(self.obj.assignee, 'id', None) },
'organization': {
"id": getattr(organization, 'id', None)
},
"assignee": { "id": self.obj.assignee_id },
'organization': { "id": organization_id },
"task": {
"owner": { "id": getattr(self.obj.segment.task.owner, 'id', None) },
"assignee": { "id": getattr(self.obj.segment.task.assignee, 'id', None) }
"owner": { "id": self.obj.segment.task.owner_id },
"assignee": { "id": self.obj.segment.task.assignee_id }
},
"project": {
"owner": { "id": getattr(self.obj.segment.task.project.owner, 'id', None) },
"assignee": { "id": getattr(self.obj.segment.task.project.assignee, 'id', None) }
"owner": { "id": self.obj.segment.task.project.owner_id },
"assignee": { "id": self.obj.segment.task.project.assignee_id }
} if self.obj.segment.task.project else None
}
elif self.scope == __class__.Scopes.CREATE:
if self.task_id is None:
raise ValidationError("task_id is not specified")
task = Task.objects.get(id=self.task_id)

organization_id = None
if task.project:
organization = task.project.organization
organization_id = task.project.organization_id
else:
organization = task.organization
organization_id = task.organization_id

data = {
'organization': {
"id": getattr(organization, 'id', None)
},
'organization': { "id": organization_id },
"task": {
"owner": { "id": getattr(task.owner, 'id', None) },
"assignee": { "id": getattr(task.assignee, 'id', None) }
"owner": { "id": task.owner_id },
"assignee": { "id": task.assignee_id }
},
"project": {
"owner": { "id": getattr(task.project.owner, 'id', None) },
"assignee": { "id": getattr(task.project.assignee, 'id', None) }
"owner": { "id": task.project.owner_id },
"assignee": { "id": task.project.assignee_id }
} if task.project else None
}

return data

class CommentPermission(OpenPolicyAgentPermission):
obj: Optional[Comment]

class Scopes(StrEnum):
LIST = 'list'
CREATE = 'create'
Expand Down Expand Up @@ -834,30 +836,29 @@ def get_scopes(request, view, obj):
def get_resource(self):
data = None
def get_common_data(db_issue):
organization_id = None
if db_issue.job.segment.task.project:
organization = db_issue.job.segment.task.project.organization
organization_id = db_issue.job.segment.task.project.organization_id
else:
organization = db_issue.job.segment.task.organization
organization_id = db_issue.job.segment.task.organization_id

data = {
"project": {
"owner": { "id": getattr(db_issue.job.segment.task.project.owner, 'id', None) },
"assignee": { "id": getattr(db_issue.job.segment.task.project.assignee, 'id', None) }
"owner": { "id": db_issue.job.segment.task.project.owner_id },
"assignee": { "id": db_issue.job.segment.task.project.assignee_id }
} if db_issue.job.segment.task.project else None,
"task": {
"owner": { "id": getattr(db_issue.job.segment.task.owner, 'id', None) },
"assignee": { "id": getattr(db_issue.job.segment.task.assignee, 'id', None) }
"owner": { "id": db_issue.job.segment.task.owner_id},
"assignee": { "id": db_issue.job.segment.task.assignee_id }
},
"job": {
"assignee": { "id": getattr(db_issue.job.assignee, 'id', None) }
"assignee": { "id": db_issue.job.assignee_id }
},
"issue": {
"owner": { "id": getattr(db_issue.owner, 'id', None) },
"assignee": { "id": getattr(db_issue.assignee, 'id', None) }
"owner": { "id": db_issue.owner_id},
"assignee": { "id": db_issue.assignee_id }
},
'organization': {
"id": getattr(organization, 'id', None)
}
'organization': { "id": organization_id }
}

return data
Expand All @@ -882,6 +883,8 @@ def get_common_data(db_issue):
return data

class IssuePermission(OpenPolicyAgentPermission):
obj: Optional[Issue]

class Scopes(StrEnum):
LIST = 'list'
CREATE = 'create'
Expand Down Expand Up @@ -926,25 +929,26 @@ def get_scopes(request, view, obj):
def get_resource(self):
data = None
def get_common_data(db_job):
organization_id = None
if db_job.segment.task.project:
organization = db_job.segment.task.project.organization
organization_id = db_job.segment.task.project.organization_id
else:
organization = db_job.segment.task.organization
organization_id = db_job.segment.task.organization_id

data = {
"project": {
"owner": { "id": getattr(db_job.segment.task.project.owner, 'id', None) },
"assignee": { "id": getattr(db_job.segment.task.project.assignee, 'id', None) }
"owner": { "id": db_job.segment.task.project.owner_id },
"assignee": { "id": db_job.segment.task.project.assignee_id }
} if db_job.segment.task.project else None,
"task": {
"owner": { "id": getattr(db_job.segment.task.owner, 'id', None) },
"assignee": { "id": getattr(db_job.segment.task.assignee, 'id', None) }
"owner": { "id": db_job.segment.task.owner_id },
"assignee": { "id": db_job.segment.task.assignee_id }
},
"job": {
"assignee": { "id": getattr(db_job.assignee, 'id', None) }
"assignee": { "id": db_job.assignee_id }
},
'organization': {
"id": getattr(organization, 'id', None)
"id": organization_id
}
}

Expand All @@ -955,8 +959,8 @@ def get_common_data(db_job):
data = get_common_data(db_job)
data.update({
"id": self.obj.id,
"owner": { "id": getattr(self.obj.owner, 'id', None) },
"assignee": { "id": getattr(self.obj.assignee, 'id', None) }
"owner": { "id": self.obj.owner_id },
"assignee": { "id": self.obj.assignee_id }
})
elif self.scope.startswith(__class__.Scopes.CREATE):
job_id = self.job_id
Expand Down Expand Up @@ -1051,29 +1055,30 @@ def get_resource(self):
data = None

if self.obj:
organization_id = None
if self.obj.project:
organization = self.obj.project.organization
organization_id = self.obj.project.organization_id
else:
organization = self.obj.task.organization
organization_id = self.obj.task.organization_id

data = {
"id": self.obj.id,
'organization': {
"id": getattr(organization, 'id', None)
},
'organization': { "id": organization_id },
"task": {
"owner": { "id": getattr(self.obj.task.owner, 'id', None) },
"assignee": { "id": getattr(self.obj.task.assignee, 'id', None) }
"owner": { "id": self.obj.task.owner_id },
"assignee": { "id": self.obj.task.assignee_id }
} if self.obj.task else None,
"project": {
"owner": { "id": getattr(self.obj.project.owner, 'id', None) },
"assignee": { "id": getattr(self.obj.project.assignee, 'id', None) }
"owner": { "id": self.obj.project.owner_id },
"assignee": { "id": self.obj.project.assignee_id }
} if self.obj.project else None,
}

return data

class AnnotationGuidePermission(OpenPolicyAgentPermission):
obj: Optional[AnnotationGuide]

class Scopes(StrEnum):
VIEW = 'view'
UPDATE = 'update'
Expand Down Expand Up @@ -1112,16 +1117,15 @@ def get_scopes(request, view, obj):
def get_resource(self):
data = {}
if self.obj:
db_target = getattr(self.obj, 'target', {})
db_organization = getattr(db_target, 'organization', {})
db_target = self.obj.target
data.update({
'id': self.obj.id,
'target': {
'owner': { 'id': getattr(getattr(db_target, 'owner', {}), 'id', None) },
'assignee': { 'id': getattr(getattr(db_target, 'assignee', {}), 'id', None) },
'owner': { 'id': db_target.owner_id },
'assignee': { 'id': db_target.assignee_id },
'is_job_staff': db_target.is_job_staff(self.user_id),
},
'organization': { 'id': getattr(db_organization, 'id', None) }
'organization': { 'id': self.obj.organization_id }
})
elif self.scope == __class__.Scopes.CREATE:
db_target = None
Expand All @@ -1135,13 +1139,14 @@ def get_resource(self):
db_target = Task.objects.get(id=self.task_id)
except Task.DoesNotExist as ex:
raise ValidationError(str(ex))
db_organization = getattr(db_target, 'organization', {})

organization_id = getattr(db_target, 'organization_id', None)
data.update({
'target': {
'owner': { 'id': db_target.owner.id },
'assignee': { 'id': getattr(db_target.assignee, 'id', None) }
'owner': { 'id': getattr(db_target, "owner_id", None) },
'assignee': { 'id': getattr(db_target, "assignee_id", None) },
},
'organization': { 'id': getattr(db_organization, 'id', None) }
'organization': { 'id': organization_id }
})
return data

Expand Down
Loading
Loading