Skip to content

Commit

Permalink
Revert "feat: use GithubAppInstallation in api (#347)" (#355)
Browse files Browse the repository at this point in the history
This reverts commit 86178ba.

This commit caused auth issues with `/graphql` endpoint in which
users were not authenticated and couldn't see source code.

Revertign for now while we figure out the exact issue and fix it.
  • Loading branch information
giovanni-guidini committed Jan 24, 2024
1 parent 86178ba commit 226f73d
Show file tree
Hide file tree
Showing 8 changed files with 202 additions and 922 deletions.
36 changes: 6 additions & 30 deletions services/repo_providers.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
import logging
from os import getenv
from typing import Callable, Dict, Optional
from typing import Callable, Dict

from django.conf import settings
from shared.encryption.token import encode_token
from shared.torngit import get

from codecov.db import sync_to_async
from codecov_auth.models import (
GITHUB_APP_INSTALLATION_DEFAULT_NAME,
GithubAppInstallation,
Owner,
Service,
)
from codecov_auth.models import Owner, Service
from core.models import Repository
from utils.config import get_config
from utils.encryption import encryptor
Expand All @@ -29,14 +24,12 @@ class TorngitInitializationFailed(Exception):


def get_token_refresh_callback(
owner: Optional[Owner], service: Service
owner: Owner, service: Service
) -> Callable[[Dict], None]:
"""
Produces a callback function that will encode and update the oauth token of an owner.
This callback is passed to the TorngitAdapter for the service.
"""
if owner is None:
return None
if service == Service.BITBUCKET or service == Service.BITBUCKET_SERVER:
return None

Expand All @@ -53,9 +46,7 @@ def callback(new_token: Dict) -> None:
return callback


def get_generic_adapter_params(
owner: Optional[Owner], service, use_ssl=False, token=None
):
def get_generic_adapter_params(owner: Owner, service, use_ssl=False, token=None):
if use_ssl:
verify_ssl = (
get_config(service, "ssl_pem")
Expand Down Expand Up @@ -96,21 +87,7 @@ def get_provider(service, adapter_params):


class RepoProviderService(object):
def _is_using_integration(self, owner: Optional[Owner], repo: Repository) -> bool:
if owner is None:
return False
ghapp_installation: Optional[
GithubAppInstallation
] = owner.github_app_installations.filter(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME
).first()
if ghapp_installation:
return ghapp_installation.is_repo_covered_by_integration(repo)
return repo.using_integration

def get_adapter(
self, owner: Optional[Owner], repo: Repository, use_ssl=False, token=None
):
def get_adapter(self, owner: Owner, repo: Repository, use_ssl=False, token=None):
"""
Return the corresponding implementation for calling the repository provider
Expand All @@ -122,11 +99,10 @@ def get_adapter(
generic_adapter_params = get_generic_adapter_params(
owner, repo.author.service, use_ssl, token
)

owner_and_repo_params = {
"repo": {
"name": repo.name,
"using_integration": self._is_using_integration(owner, repo),
"using_integration": repo.using_integration or False,
"service_id": repo.service_id,
"private": repo.private,
"repoid": repo.repoid,
Expand Down
67 changes: 1 addition & 66 deletions services/tests/test_repo_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@

from codecov.db import sync_to_async
from codecov.tests.base_test import InternalAPITest
from codecov_auth.models import (
GITHUB_APP_INSTALLATION_DEFAULT_NAME,
GithubAppInstallation,
Owner,
)
from codecov_auth.models import Owner
from codecov_auth.tests.factories import OwnerFactory
from core.tests.factories import RepositoryFactory
from services.repo_providers import RepoProviderService
Expand All @@ -37,67 +33,6 @@ def mock_get_env_ca_bundle(*args):
return "REQUESTS_CA_BUNDLE"


@pytest.mark.parametrize("using_integration", [True, False])
def test__is_using_integration_deprecated_flow(using_integration, db):
repo = RepositoryFactory.create(using_integration=using_integration)
assert (
RepoProviderService()._is_using_integration(repo.author, repo)
== using_integration
)


def test__is_using_integration_ghapp_covers_all_repos(db):
owner = OwnerFactory.create(service="github")
repo = RepositoryFactory.create(author=owner)
other_repo_same_owner = RepositoryFactory.create(author=owner)
repo_different_owner = RepositoryFactory.create()
assert repo.author != repo_different_owner.author
ghapp_installation = GithubAppInstallation(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME,
owner=owner,
repository_service_ids=None,
installation_id=12345,
)
ghapp_installation.save()
assert RepoProviderService()._is_using_integration(repo.author, repo) == True
assert (
RepoProviderService()._is_using_integration(repo.author, other_repo_same_owner)
== True
)
assert (
RepoProviderService()._is_using_integration(
repo_different_owner.author, repo_different_owner
)
== False
)


def test__is_using_integration_ghapp_covers_some_repos(db):
owner = OwnerFactory.create(service="github")
repo = RepositoryFactory.create(author=owner)
other_repo_same_owner = RepositoryFactory.create(author=owner)
repo_different_owner = RepositoryFactory.create()
assert repo.author != repo_different_owner.author
ghapp_installation = GithubAppInstallation(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME,
owner=owner,
repository_service_ids=[repo.service_id],
installation_id=12345,
)
ghapp_installation.save()
assert RepoProviderService()._is_using_integration(repo.author, repo) == True
assert (
RepoProviderService()._is_using_integration(repo.author, other_repo_same_owner)
== False
)
assert (
RepoProviderService()._is_using_integration(
repo_different_owner.author, repo_different_owner
)
== False
)


class TestRepoProviderService(InternalAPITest):
def setUp(self):
self.repo_gh = RepositoryFactory.create(
Expand Down
40 changes: 4 additions & 36 deletions upload/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import re
from datetime import timedelta
from json import dumps
from typing import Optional

import jwt
from asgiref.sync import async_to_sync
Expand All @@ -18,13 +17,7 @@
from shared.reports.enums import UploadType
from shared.torngit.exceptions import TorngitClientError, TorngitObjectNotFoundError

from codecov_auth.models import (
GITHUB_APP_INSTALLATION_DEFAULT_NAME,
SERVICE_GITHUB,
SERVICE_GITHUB_ENTERPRISE,
GithubAppInstallation,
Owner,
)
from codecov_auth.models import Owner
from core.models import Commit, CommitNotification, Pull, Repository
from plan.constants import USER_PLAN_REPRESENTATIONS
from reports.models import CommitReport, ReportSession
Expand Down Expand Up @@ -322,45 +315,20 @@ def determine_upload_pr_to_use(upload_params):
return upload_params.get("pr")


def ghapp_installation_id_to_use(repository: Repository) -> Optional[str]:
if (
repository.service != SERVICE_GITHUB
and repository.service != SERVICE_GITHUB_ENTERPRISE
):
return None

gh_app_default_installation: GithubAppInstallation = (
repository.author.github_app_installations.filter(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME
).first()
)
if (
gh_app_default_installation
and gh_app_default_installation.is_repo_covered_by_integration(repository)
):
return gh_app_default_installation.installation_id
elif repository.using_integration and repository.author.integration_id:
# THIS FLOW IS DEPRECATED
# it will (hopefully) be removed after the ghapp installation work is complete
# and the data is backfilles appropriately
return repository.author.integration_id


def try_to_get_best_possible_bot_token(repository):
ghapp_installation_id = ghapp_installation_id_to_use(repository)
if ghapp_installation_id is not None:
if repository.using_integration and repository.author.integration_id:
try:
github_token = get_github_integration_token(
repository.author.service,
installation_id=ghapp_installation_id,
integration_id=repository.author.integration_id,
)
return dict(key=github_token)
except InvalidInstallationError:
log.warning(
"Invalid installation error",
extra=dict(
service=repository.author.service,
integration_id=ghapp_installation_id,
integration_id=repository.author.integration_id,
),
)
# now we'll fallback to trying an OAuth token
Expand Down
30 changes: 2 additions & 28 deletions upload/tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,22 @@
import jwt
import pytest
from django.conf import settings
from django.test import TransactionTestCase
from rest_framework.exceptions import Throttled, ValidationError
from shared.config import get_config
from shared.github import InvalidInstallationError

from codecov_auth.models import GithubAppInstallation, Service
from core.tests.factories import CommitFactory, OwnerFactory, RepositoryFactory
from plan.constants import PlanName
from reports.tests.factories import CommitReportFactory, UploadFactory
from upload.helpers import (
check_commit_upload_constraints,
determine_repo_for_upload,
ghapp_installation_id_to_use,
try_to_get_best_possible_bot_token,
validate_activated_repo,
validate_upload,
)


class TestGithubAppInstallationUsage(TransactionTestCase):
def test_not_github_provider(self):
repo = RepositoryFactory(author__service=Service.GITLAB.value)
assert ghapp_installation_id_to_use(repo) is None

def test_github_app_installation_flow(self):
owner = OwnerFactory(service=Service.GITHUB.value, integration_id=None)
covered_repo = RepositoryFactory(author=owner)
not_covered_repo = RepositoryFactory(author=owner)
ghapp_installation = GithubAppInstallation(
owner=owner,
repository_service_ids=[covered_repo.service_id],
installation_id=200,
)
ghapp_installation.save()
assert ghapp_installation_id_to_use(covered_repo) == 200
assert ghapp_installation_id_to_use(not_covered_repo) is None


def test_try_to_get_best_possible_bot_token_no_repobot_no_ownerbot(db):
owner = OwnerFactory.create(unencrypted_oauth_token="super")
owner.save()
Expand Down Expand Up @@ -94,9 +72,7 @@ def test_try_to_get_best_possible_bot_token_using_integration(
assert try_to_get_best_possible_bot_token(repository) == {
"key": "test-token",
}
get_github_integration_token.assert_called_once_with(
"github", installation_id=12345
)
get_github_integration_token.assert_called_once_with("github", integration_id=12345)


@patch("upload.helpers.get_github_integration_token")
Expand All @@ -116,9 +92,7 @@ def test_try_to_get_best_possible_bot_token_using_invalid_integration(
"key": "bornana",
"secret": None,
}
get_github_integration_token.assert_called_once_with(
"github", installation_id=12345
)
get_github_integration_token.assert_called_once_with("github", integration_id=12345)


def test_try_to_get_best_possible_nothing_and_is_private(db):
Expand Down
4 changes: 2 additions & 2 deletions utils/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@


@cache.cache_function(ttl=480)
def get_github_integration_token(service, installation_id=None):
return _get_github_integration_token(service, integration_id=installation_id)
def get_github_integration_token(service, integration_id=None):
return _get_github_integration_token(service, integration_id=integration_id)

0 comments on commit 226f73d

Please sign in to comment.