-
Notifications
You must be signed in to change notification settings - Fork 28
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
[feat] Add endpoint that checks whether an owner has Gen AI consent #1102
Open
rohitvinnakota-codecov
wants to merge
18
commits into
main
Choose a base branch
from
rvinnakota/add-ai-auth-endpoint
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
a82089d
Add gen ai endpoint
rohitvinnakota-codecov 7e5a1a9
undo
rohitvinnakota-codecov e47e5a3
lint
rohitvinnakota-codecov d4ba9a6
Typing
rohitvinnakota-codecov 9f365bd
Update
rohitvinnakota-codecov 5bdbff6
Update
rohitvinnakota-codecov 28457dd
Lint
rohitvinnakota-codecov a0a5ad8
remove print
rohitvinnakota-codecov d52a405
lint
rohitvinnakota-codecov 4c5ef9d
Merge branch 'main' into rvinnakota/add-ai-auth-endpoint
rohitvinnakota-codecov d6192d5
Update
rohitvinnakota-codecov 4e4f279
Merge branch 'rvinnakota/add-ai-auth-endpoint' of https://github.com/…
rohitvinnakota-codecov ed71817
Add serializer
rohitvinnakota-codecov 386372f
sort imports
rohitvinnakota-codecov c0b07ff
Update hosts
rohitvinnakota-codecov da13f82
Fix tests
rohitvinnakota-codecov 624ee10
Update
rohitvinnakota-codecov ac5908b
Update api/gen_ai/views.py
rohitvinnakota-codecov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
from rest_framework import serializers | ||
|
||
|
||
class GenAIAuthSerializer(serializers.Serializer): | ||
is_valid = serializers.BooleanField() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
import hmac | ||
import json | ||
from hashlib import sha256 | ||
from unittest.mock import patch | ||
|
||
from django.urls import reverse | ||
from rest_framework import status | ||
from rest_framework.test import APITestCase | ||
from shared.django_apps.core.tests.factories import OwnerFactory | ||
|
||
from codecov_auth.models import GithubAppInstallation | ||
|
||
PAYLOAD_SECRET = b"testixik8qdauiab1yiffydimvi72ekq" | ||
VIEW_URL = reverse("auth") | ||
|
||
|
||
def sign_payload(payload, secret=PAYLOAD_SECRET): | ||
data = json.dumps(payload, separators=(",", ":")).encode("utf-8") | ||
signature = "sha256=" + hmac.new(secret, data, digestmod=sha256).hexdigest() | ||
return signature, data | ||
|
||
|
||
class GenAIAuthViewTests(APITestCase): | ||
@patch("utils.config.get_config", return_value=PAYLOAD_SECRET) | ||
def test_missing_parameters(self, mock_config): | ||
payload = {} | ||
sig, data = sign_payload(payload) | ||
response = self.client.post( | ||
VIEW_URL, | ||
data=payload, | ||
content_type="application/json", | ||
HTTP_HTTP_X_GEN_AI_AUTH_SIGNATURE=sig, | ||
) | ||
self.assertEqual(response.status_code, 400) | ||
self.assertIn("Missing required parameters", response.data) | ||
|
||
@patch("utils.config.get_config", return_value=PAYLOAD_SECRET) | ||
def test_invalid_signature(self, mock_config): | ||
payload = {"external_owner_id": "owner1", "repo_service_id": "101"} | ||
# Create a wrong signature by altering the payload before signing | ||
wrong_sig = ( | ||
"sha256=" + hmac.new(PAYLOAD_SECRET, b"{}", digestmod=sha256).hexdigest() | ||
) | ||
response = self.client.post( | ||
VIEW_URL, | ||
data=payload, | ||
content_type="application/json", | ||
HTTP_HTTP_X_GEN_AI_AUTH_SIGNATURE=wrong_sig, | ||
) | ||
self.assertEqual(response.status_code, 403) | ||
|
||
@patch("utils.config.get_config", return_value=PAYLOAD_SECRET) | ||
def test_owner_not_found(self, mock_config): | ||
payload = {"external_owner_id": "nonexistent_owner", "repo_service_id": "101"} | ||
sig, serialized_data = sign_payload(payload) | ||
response = self.client.post( | ||
VIEW_URL, | ||
HTTP_HTTP_X_GEN_AI_AUTH_SIGNATURE=sig, | ||
data=serialized_data, | ||
content_type="application/json", | ||
) | ||
self.assertEqual(response.status_code, 404) | ||
|
||
@patch("utils.config.get_config", return_value=PAYLOAD_SECRET) | ||
def test_no_installation(self, mock_config): | ||
_ = OwnerFactory(service="github", service_id="owner1", username="test1") | ||
payload = {"external_owner_id": "owner1", "repo_service_id": "101"} | ||
sig, data = sign_payload(payload) | ||
response = self.client.post( | ||
VIEW_URL, | ||
data=data, | ||
content_type="application/json", | ||
HTTP_HTTP_X_GEN_AI_AUTH_SIGNATURE=sig, | ||
) | ||
|
||
self.assertEqual(response.status_code, 200) | ||
self.assertEqual(response.data, {"is_valid": False}) | ||
|
||
@patch("utils.config.get_config", return_value=PAYLOAD_SECRET) | ||
def test_authorized(self, mock_config): | ||
owner = OwnerFactory(service="github", service_id="owner2", username="test2") | ||
app_install = GithubAppInstallation( | ||
installation_id=12345, | ||
owner=owner, | ||
name="ai-features", | ||
repository_service_ids=["101", "202"], | ||
) | ||
app_install.save() | ||
payload = {"external_owner_id": "owner2", "repo_service_id": "101"} | ||
sig, data = sign_payload(payload) | ||
response = self.client.post( | ||
VIEW_URL, | ||
data=data, | ||
content_type="application/json", | ||
HTTP_HTTP_X_GEN_AI_AUTH_SIGNATURE=sig, | ||
) | ||
self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
self.assertEqual(response.data, {"is_valid": True}) | ||
|
||
@patch("utils.config.get_config", return_value=PAYLOAD_SECRET) | ||
def test_unauthorized(self, mock_config): | ||
owner = OwnerFactory(service="github", service_id="owner3", username="test3") | ||
# Create a GithubAppInstallation where the list does not include the requested repo_service_id. | ||
app_install = GithubAppInstallation.objects.create( | ||
installation_id=2, | ||
owner=owner, | ||
name="ai-features", | ||
repository_service_ids=["303", "404"], | ||
) | ||
app_install.save() | ||
payload = {"external_owner_id": "owner3", "repo_service_id": "101"} | ||
sig, data = sign_payload(payload) | ||
response = self.client.post( | ||
VIEW_URL, | ||
data=data, | ||
content_type="application/json", | ||
HTTP_HTTP_X_GEN_AI_AUTH_SIGNATURE=sig, | ||
) | ||
self.assertEqual(response.status_code, 200) | ||
self.assertEqual(response.data, {"is_valid": False}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
from django.urls import path | ||
|
||
from .views import GenAIAuthView | ||
|
||
urlpatterns = [ | ||
path("auth/", GenAIAuthView.as_view(), name="auth"), | ||
] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
import hmac | ||
import logging | ||
from hashlib import sha256 | ||
|
||
from django.utils.crypto import constant_time_compare | ||
from rest_framework.exceptions import NotFound, PermissionDenied | ||
from rest_framework.permissions import AllowAny | ||
from rest_framework.response import Response | ||
from rest_framework.views import APIView | ||
|
||
from api.gen_ai.serializers import GenAIAuthSerializer | ||
from codecov_auth.models import GithubAppInstallation, Owner | ||
from graphql_api.types.owner.owner import AI_FEATURES_GH_APP_ID | ||
from utils.config import get_config | ||
|
||
log = logging.getLogger(__name__) | ||
|
||
|
||
class GenAIAuthView(APIView): | ||
permission_classes = [AllowAny] | ||
serializer_class = GenAIAuthSerializer | ||
|
||
def validate_signature(self, request): | ||
key = get_config( | ||
"gen_ai", "auth_secret", default=b"testixik8qdauiab1yiffydimvi72ekq" | ||
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. Is it possible to avoid setting a default here and instead error out if it is not set? |
||
) | ||
if isinstance(key, str): | ||
key = key.encode("utf-8") | ||
expected_sig = request.headers.get("HTTP-X-GEN-AI-AUTH-SIGNATURE") | ||
print(request.headers) | ||
rohitvinnakota-codecov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
computed_sig = ( | ||
"sha256=" + hmac.new(key, request.body, digestmod=sha256).hexdigest() | ||
) | ||
if not (expected_sig and constant_time_compare(computed_sig, expected_sig)): | ||
rohitvinnakota-codecov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
raise PermissionDenied("Invalid signature") | ||
|
||
def post(self, request, *args, **kwargs): | ||
self.validate_signature(request) | ||
external_owner_id = request.data.get("external_owner_id") | ||
repo_service_id = request.data.get("repo_service_id") | ||
if not external_owner_id or not repo_service_id: | ||
return Response("Missing required parameters", status=400) | ||
try: | ||
owner = Owner.objects.get(service_id=external_owner_id) | ||
except Owner.DoesNotExist: | ||
raise NotFound("Owner not found") | ||
|
||
is_authorized = True | ||
|
||
app_install = GithubAppInstallation.objects.filter( | ||
owner_id=owner.ownerid, app_id=AI_FEATURES_GH_APP_ID | ||
).first() | ||
|
||
if not app_install: | ||
is_authorized = False | ||
|
||
else: | ||
repo_ids = app_install.repository_service_ids | ||
if repo_ids and repo_service_id not in repo_ids: | ||
is_authorized = False | ||
|
||
return Response({"is_valid": is_authorized}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I would avoid going from bytes -> string -> bytes with
json.dumps()
. Ordering of the keys isn't guaranteed (afaik), this can result in different signatures. The recommendation when working with payload signatures like this is to always calculate them based on the raw bytes.