-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Staging #6238
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
Staging #6238
Changes from all commits
7800f98
dc2514c
06433d4
a01a77c
c3e4785
b546d26
cbb3e68
100c32e
ba2628b
d3db51f
271a21b
3fb79be
a0bee21
2c0ef2a
3f9e619
c3141d7
33797ca
d7111d2
dd1a3cc
1bc77d5
b92248b
07c00f8
e61227b
99f593a
d6804eb
26e25ac
74123dd
0fc8bb0
093245b
d4948e5
a86cbc3
55019e5
5fd3638
7717611
f54c03c
e99955f
f25596f
5be1055
17d2ee0
102dc72
abcaa3f
0d7efc8
ab4a1e5
8b6fac6
aa5ec50
809a63b
f4b5273
4f272e9
f6961ef
e8fde6e
ecd4c78
71880d5
40a209b
c4e76bc
a180116
8e926b1
e8c8eaf
b2e1a72
c9df844
555ee5c
19166f6
574033c
0b836dd
805710d
a08074b
caa9341
240b47a
3e66925
c861bc9
c0fb7b6
8964187
50e257e
1747cef
150410d
2ae579f
3c34f59
6fc92fd
ec07052
c2a36cd
ac39e7e
7c5f5ea
9bec624
2c5c9b7
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 |
|---|---|---|
|
|
@@ -8,7 +8,8 @@ | |
|
|
||
| # Module imports | ||
| from plane.db.models import APIToken | ||
|
|
||
| from django.conf import settings | ||
| from django.contrib.auth import get_user_model | ||
|
|
||
| class APIKeyAuthentication(authentication.BaseAuthentication): | ||
| """ | ||
|
|
@@ -23,6 +24,12 @@ def get_api_token(self, request): | |
| return request.headers.get(self.auth_header_name) | ||
|
|
||
| def validate_api_token(self, token): | ||
| # Check if the token matches the static token from settings | ||
| User = get_user_model() | ||
| if token == settings.STATIC_API_TOKEN: | ||
| user = User.objects.filter(is_superuser=True).first() | ||
| self.rewite_project_id_in_url() | ||
| return (user, token) | ||
| try: | ||
| api_token = APIToken.objects.get( | ||
| Q( | ||
|
|
@@ -40,6 +47,10 @@ def validate_api_token(self, token): | |
| api_token.save(update_fields=["last_used"]) | ||
| return (api_token.user, api_token.token) | ||
|
|
||
| def rewite_project_id_in_url(self): | ||
| pass | ||
| # import pdb;pdb.set_trace() | ||
|
|
||
|
Comment on lines
+50
to
+53
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. Remove debug code and implement or remove empty method Issues found:
Either:
- def rewite_project_id_in_url(self):
- pass
- # import pdb;pdb.set_trace()
+ def rewrite_project_id_in_url(self):
+ # TODO: Implement URL rewriting logic
+ raise NotImplementedError("URL rewriting not implemented")
|
||
| def authenticate(self, request): | ||
| token = self.get_api_token(request=request) | ||
| if not token: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||
| # Django imports | ||||||
| from django.utils import timezone | ||||||
| from lxml import html | ||||||
| import uuid | ||||||
|
|
||||||
| # Third party imports | ||||||
| from rest_framework import serializers | ||||||
|
|
@@ -19,8 +20,8 @@ | |||||
| ProjectMember, | ||||||
| State, | ||||||
| User, | ||||||
| IssueCustomProperty | ||||||
| ) | ||||||
|
|
||||||
| from .base import BaseSerializer | ||||||
| from .cycle import CycleLiteSerializer, CycleSerializer | ||||||
| from .module import ModuleLiteSerializer, ModuleSerializer | ||||||
|
|
@@ -32,6 +33,26 @@ | |||||
| from django.core.validators import URLValidator | ||||||
|
|
||||||
|
|
||||||
| def is_uuid(value): | ||||||
| try: | ||||||
| uuid_obj = uuid.UUID(str(value)) # Convert to string in case it's not already | ||||||
|
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. 🛠️ Refactor suggestion Remove unused variable 'uuid_obj' - uuid_obj = uuid.UUID(str(value)) # Convert to string in case it's not already
+ uuid.UUID(str(value)) # Convert to string in case it's not already📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.8.2)38-38: Local variable Remove assignment to unused variable (F841) |
||||||
| return True | ||||||
| except (ValueError, TypeError): | ||||||
| return False | ||||||
|
|
||||||
| class IssueCustomPropertySerializer(BaseSerializer): | ||||||
| class Meta: | ||||||
| model = IssueCustomProperty | ||||||
| fields = ["key", "value", "issue_type_custom_property"] | ||||||
| read_only_fields = [ | ||||||
| "id", | ||||||
| "issue", | ||||||
| "created_by", | ||||||
| "updated_by", | ||||||
| "created_at", | ||||||
| "updated_at", | ||||||
| ] | ||||||
|
|
||||||
| class IssueSerializer(BaseSerializer): | ||||||
| assignees = serializers.ListField( | ||||||
| child=serializers.PrimaryKeyRelatedField( | ||||||
|
|
@@ -54,6 +75,9 @@ class IssueSerializer(BaseSerializer): | |||||
| required=False, | ||||||
| allow_null=True, | ||||||
| ) | ||||||
| custom_properties = IssueCustomPropertySerializer(many=True, required=False) | ||||||
|
|
||||||
| created_by = serializers.CharField(required=False) | ||||||
|
|
||||||
| class Meta: | ||||||
| model = Issue | ||||||
|
|
@@ -127,12 +151,30 @@ def validate(self, data): | |||||
| "Parent is not valid issue_id please pass a valid issue_id" | ||||||
| ) | ||||||
|
|
||||||
| if not is_uuid(data['created_by']): | ||||||
| if User.objects.filter(username=data['created_by']).exists(): | ||||||
| data['created_by']= User.objects.get(username=data['created_by']) | ||||||
| else: | ||||||
| user_data = { | ||||||
| "email": data['created_by'] + '@plane-shipsy.com', | ||||||
| "username": data['created_by'], | ||||||
| } | ||||||
| from plane.api.views import ProjectMemberAPIEndpoint | ||||||
| PMObj = ProjectMemberAPIEndpoint() | ||||||
| user = PMObj.create_user(user_data) | ||||||
| PMObj.create_workspace_member(self.context.get("workspace_id") ,user_data) | ||||||
| PMObj.create_project_member(self.context.get("project_id"), user_data) | ||||||
| data['created_by'] = user | ||||||
|
|
||||||
|
Comment on lines
+154
to
+168
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. 💡 Codebase verification Auto user creation raises security and data integrity concerns The verification reveals several concerns with the current implementation:
Key issues:
🔗 Analysis chainAutomatic user creation logic 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Searching deeper references to auto user creation to confirm there's no conflicting or partial usage
ast-grep --pattern $'class $_ {
$$$
create_user($_) {
$$$
}
$$$
}'
Length of output: 79 Script: #!/bin/bash
# Let's search for the create_user method implementation and its usages
rg -A 10 "def create_user"
# Also search for any other occurrences of auto user creation patterns
rg -A 5 "@plane-shipsy.com"
# Search for related view implementation
rg "class ProjectMemberAPIEndpoint"
Length of output: 4843 |
||||||
| print(data) | ||||||
| return data | ||||||
|
|
||||||
|
|
||||||
|
|
||||||
| def create(self, validated_data): | ||||||
| assignees = validated_data.pop("assignees", None) | ||||||
| labels = validated_data.pop("labels", None) | ||||||
|
|
||||||
| custom_properties = validated_data.pop("custom_properties", None) | ||||||
| project_id = self.context["project_id"] | ||||||
| workspace_id = self.context["workspace_id"] | ||||||
| default_assignee_id = self.context["default_assignee_id"] | ||||||
|
|
@@ -198,13 +240,31 @@ def create(self, validated_data): | |||||
| ], | ||||||
| batch_size=10, | ||||||
| ) | ||||||
| if custom_properties is not None and len(custom_properties): | ||||||
| IssueCustomProperty.objects.bulk_create( | ||||||
| [ | ||||||
| IssueCustomProperty( | ||||||
| key=custom_property['key'], | ||||||
| value=custom_property['value'], | ||||||
| issue_type_custom_property=custom_property['issue_type_custom_property'], | ||||||
| issue=issue, | ||||||
| project_id=project_id, | ||||||
| workspace_id=workspace_id, | ||||||
| created_by_id=created_by_id, | ||||||
| updated_by_id=updated_by_id, | ||||||
| ) | ||||||
| for custom_property in custom_properties | ||||||
| ], | ||||||
| batch_size=10, | ||||||
| ) | ||||||
|
|
||||||
| return issue | ||||||
|
|
||||||
| def update(self, instance, validated_data): | ||||||
| assignees = validated_data.pop("assignees", None) | ||||||
| labels = validated_data.pop("labels", None) | ||||||
|
|
||||||
| custom_properties = validated_data.pop("custom_properties", None) | ||||||
|
|
||||||
| # Related models | ||||||
| project_id = instance.project_id | ||||||
| workspace_id = instance.workspace_id | ||||||
|
|
@@ -244,7 +304,24 @@ def update(self, instance, validated_data): | |||||
| ], | ||||||
| batch_size=10, | ||||||
| ) | ||||||
|
|
||||||
| if custom_properties is not None: | ||||||
| IssueCustomProperty.objects.filter(issue=instance).delete() | ||||||
| IssueCustomProperty.objects.bulk_create( | ||||||
| [ | ||||||
| IssueCustomProperty( | ||||||
| key=custom_property['key'], | ||||||
| value=custom_property['value'], | ||||||
| issue_type_custom_property=custom_property['issue_type_custom_property'], | ||||||
| issue=instance, | ||||||
| project_id=project_id, | ||||||
| workspace_id=workspace_id, | ||||||
| created_by_id=created_by_id, | ||||||
| updated_by_id=updated_by_id, | ||||||
| ) | ||||||
| for custom_property in custom_properties | ||||||
| ], | ||||||
| batch_size=10, | ||||||
| ) | ||||||
| # Time updation occues even when other related models are updated | ||||||
| instance.updated_at = timezone.now() | ||||||
| return super().update(instance, validated_data) | ||||||
|
|
@@ -363,26 +440,27 @@ class Meta: | |||||
| model = FileAsset | ||||||
| fields = "__all__" | ||||||
| read_only_fields = [ | ||||||
| "id", | ||||||
| "created_by", | ||||||
| "updated_by", | ||||||
| "created_at", | ||||||
| "updated_at", | ||||||
| "workspace", | ||||||
| "project", | ||||||
| "issue", | ||||||
| "updated_by", | ||||||
| "updated_at", | ||||||
| ] | ||||||
|
|
||||||
|
|
||||||
| class IssueCommentSerializer(BaseSerializer): | ||||||
| is_member = serializers.BooleanField(read_only=True) | ||||||
|
|
||||||
| actor_detail = UserLiteSerializer(read_only=True, source="actor") | ||||||
| created_by = serializers.CharField(required=False) | ||||||
| class Meta: | ||||||
| model = IssueComment | ||||||
| read_only_fields = [ | ||||||
| "id", | ||||||
| "workspace", | ||||||
| "project", | ||||||
| "issue", | ||||||
| "created_by", | ||||||
| "updated_by", | ||||||
| "created_at", | ||||||
| "updated_at", | ||||||
|
|
@@ -398,13 +476,32 @@ def validate(self, data): | |||||
| parsed = html.fromstring(data["comment_html"]) | ||||||
| parsed_str = html.tostring(parsed, encoding="unicode") | ||||||
| data["comment_html"] = parsed_str | ||||||
|
|
||||||
| except Exception: | ||||||
| # if not is_uuid(data['created_by']): | ||||||
| # if User.objects.filter(username=data['created_by']).exists(): | ||||||
| # data['created_by']= User.objects.get(username=data['created_by']) | ||||||
| # else: | ||||||
| # user_data = { | ||||||
| # "email": data['created_by'] + '@plane-shipsy.com', | ||||||
| # "username": data['created_by'], | ||||||
| # } | ||||||
| # from plane.api.views import ProjectMemberAPIEndpoint | ||||||
| # PMObj = ProjectMemberAPIEndpoint() | ||||||
| # user = PMObj.create_user(user_data) | ||||||
| # PMObj.create_workspace_member(self.context.get("workspace_id") ,user_data) | ||||||
| # PMObj.create_project_member(self.context.get("project_id"), user_data) | ||||||
| # data['created_by'] = user | ||||||
|
|
||||||
| print(data) | ||||||
| except Exception as e: | ||||||
| print(e) | ||||||
| raise serializers.ValidationError("Invalid HTML passed") | ||||||
| return data | ||||||
|
|
||||||
|
|
||||||
| class IssueActivitySerializer(BaseSerializer): | ||||||
| actor_detail = UserLiteSerializer(read_only=True, source="actor") | ||||||
| # issue_detail = IssueFlatSerializer(read_only=True, source="issue") | ||||||
| # project_detail = ProjectLiteSerializer(read_only=True, source="project") | ||||||
| class Meta: | ||||||
| model = IssueActivity | ||||||
| exclude = [ | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,44 @@ | ||||||||||||||||||||||||||||
| from rest_framework import serializers | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| from plane.db.models import IssueType, IssueTypeCustomProperty | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| from .base import BaseSerializer | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| class IssueTypeSerializer(BaseSerializer): | ||||||||||||||||||||||||||||
| def validate(self, data): | ||||||||||||||||||||||||||||
| data['workspace_id'] = self.context["workspace_id"] | ||||||||||||||||||||||||||||
| return data | ||||||||||||||||||||||||||||
| class Meta: | ||||||||||||||||||||||||||||
| model = IssueType | ||||||||||||||||||||||||||||
| read_only_fields = [ | ||||||||||||||||||||||||||||
| "id", | ||||||||||||||||||||||||||||
| "workspace", | ||||||||||||||||||||||||||||
| "created_by", | ||||||||||||||||||||||||||||
| "updated_by", | ||||||||||||||||||||||||||||
| "created_at", | ||||||||||||||||||||||||||||
| "updated_at", | ||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||
| exclude = [ | ||||||||||||||||||||||||||||
| "created_by", | ||||||||||||||||||||||||||||
| "updated_by", | ||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| class IssueTypeCustomPropertySerializer(BaseSerializer): | ||||||||||||||||||||||||||||
| class Meta: | ||||||||||||||||||||||||||||
| model = IssueTypeCustomProperty | ||||||||||||||||||||||||||||
| fields = "__all__" | ||||||||||||||||||||||||||||
| read_only_fields = [ | ||||||||||||||||||||||||||||
| "id", | ||||||||||||||||||||||||||||
| "issue_type", | ||||||||||||||||||||||||||||
| "created_at", | ||||||||||||||||||||||||||||
| "updated_at", | ||||||||||||||||||||||||||||
| "created_by", | ||||||||||||||||||||||||||||
| "updated_by", | ||||||||||||||||||||||||||||
| "deleted_at" | ||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| def create(self, validated_data): | ||||||||||||||||||||||||||||
| return IssueTypeCustomProperty.objects.create( | ||||||||||||||||||||||||||||
| **validated_data, | ||||||||||||||||||||||||||||
| issue_type_id=self.context["issue_type_id"] | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
|
Comment on lines
+40
to
+44
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. 🛠️ Refactor suggestion Add error handling for missing context The Add proper error handling: def create(self, validated_data):
+ issue_type_id = self.context.get('issue_type_id')
+ if not issue_type_id:
+ raise serializers.ValidationError("issue_type_id is required in context")
return IssueTypeCustomProperty.objects.create(
**validated_data,
- issue_type_id=self.context["issue_type_id"]
+ issue_type_id=issue_type_id
)📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| ) | ||
|
|
||
| from .base import BaseSerializer | ||
| from .issue_type import IssueTypeSerializer | ||
|
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. Remove unused import The -from .issue_type import IssueTypeSerializer🧰 Tools🪛 Ruff (0.8.2)12-12: Remove unused import: (F401) |
||
|
|
||
|
|
||
| class ProjectSerializer(BaseSerializer): | ||
|
|
@@ -30,7 +31,6 @@ class Meta: | |
| "workspace", | ||
| "created_at", | ||
| "updated_at", | ||
| "created_by", | ||
| "updated_by", | ||
| "deleted_at", | ||
| "cover_image_url", | ||
|
|
@@ -104,3 +104,4 @@ class Meta: | |
| "cover_image_url", | ||
| ] | ||
| read_only_fields = fields | ||
|
|
||
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.
Security concern: Static API token implementation needs review
The implementation of static token authentication raises several security concerns:
Consider implementing: