Skip to content

Commit

Permalink
@W-14870747 - Dependency updates and disabling Socalapp in db (#2160)
Browse files Browse the repository at this point in the history
  • Loading branch information
vsbharath authored Feb 21, 2024
1 parent f60024a commit 8cf33f6
Show file tree
Hide file tree
Showing 23 changed files with 260 additions and 263 deletions.
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ Use the "Webhook secret" value as your `GITHUB_HOOK_SECRET` environment variable
in Metecho.

Use the app's "App ID" as `GITHUB_APP_ID`, "Client ID" as `GITHUB_CLIENT_ID`,
and "Client secret" as `GITHUB_CLIENT_SECRET`. These are stored in a shared lastpass note.
and "Client secret" as `GITHUB_CLIENT_SECRET`. These are stored in a shared
lastpass note.

Finally, generate a new private key for the app and set it as the
`GITHUB_APP_KEY` environment variable (the entire key, not a path to one). If
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

## Development and Deployment

See [documentation](https://metecho.readthedocs.io/en/latest/heroku-setup.html) on how to set up Metecho on Heroku.
See [documentation](https://metecho.readthedocs.io/en/latest/heroku-setup.html)
on how to set up Metecho on Heroku.

See [CONTRIBUTING.md](CONTRIBUTING.md).

Expand Down
1 change: 1 addition & 0 deletions config/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ def safe_key() -> str:
MIDDLEWARE = [
"metecho.logging_middleware.LoggingMiddleware",
"sfdo_template_helpers.admin.middleware.AdminRestrictMiddleware",
"allauth.account.middleware.AccountMiddleware",
"django.middleware.security.SecurityMiddleware",
"whitenoise.middleware.WhiteNoiseMiddleware",
"django.contrib.sessions.middleware.SessionMiddleware",
Expand Down
34 changes: 21 additions & 13 deletions docs/api/schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1871,6 +1871,11 @@ components:
- Review
- Merged
type: string
description: |-
* `Planned` - Planned
* `In progress` - In Progress
* `Review` - Review
* `Merged` - Merged
FullUser:
type: object
properties:
Expand Down Expand Up @@ -1940,8 +1945,6 @@ components:
self_guided_tour_enabled:
type: boolean
self_guided_tour_state:
type: object
additionalProperties: {}
nullable: true
organizations:
type: array
Expand Down Expand Up @@ -2078,9 +2081,7 @@ components:
properties:
enabled:
type: boolean
state:
type: object
additionalProperties: {}
state: {}
MinimalUser:
type: object
properties:
Expand Down Expand Up @@ -2118,6 +2119,10 @@ components:
- QA
- Playground
type: string
description: |-
* `Dev` - Dev
* `QA` - QA
* `Playground` - Playground
PaginatedEpicList:
type: object
properties:
Expand Down Expand Up @@ -2277,8 +2282,6 @@ components:
description:
type: string
ignored_changes_write:
type: object
additionalProperties: {}
writeOnly: true
org_config_name:
type: string
Expand Down Expand Up @@ -2481,11 +2484,17 @@ components:
- Approved
- Changes requested
type: string
description: |-
* `Approved` - Approved
* `Changes requested` - Changes Requested
RoleEnum:
enum:
- assigned_qa
- assigned_dev
type: string
description: |-
* `assigned_qa` - assigned_qa
* `assigned_dev` - assigned_dev
ScratchOrg:
type: object
properties:
Expand Down Expand Up @@ -2612,8 +2621,6 @@ components:
type: boolean
readOnly: true
installed_packages:
type: object
additionalProperties: {}
readOnly: true
is_omnistudio_installed:
type: boolean
Expand Down Expand Up @@ -2672,8 +2679,6 @@ components:
description:
type: string
ignored_changes_write:
type: object
additionalProperties: {}
writeOnly: true
org_config_name:
type: string
Expand Down Expand Up @@ -2773,8 +2778,6 @@ components:
format: uri
readOnly: true
commits:
type: object
additionalProperties: {}
readOnly: true
origin_sha:
type: string
Expand Down Expand Up @@ -2909,6 +2912,11 @@ components:
- Completed
- Canceled
type: string
description: |-
* `Planned` - Planned
* `In progress` - In Progress
* `Completed` - Completed
* `Canceled` - Canceled
securitySchemes:
cookieAuth:
type: apiKey
Expand Down
3 changes: 0 additions & 3 deletions locales_dev/en/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
"Complete a Task": "Complete a Task",
"Completed item": "Completed item",
"Confirm": "Confirm",
"Confirm Changing Developer and Deleting Dev Org": "Confirm Changing Developer and Deleting Dev Org",
"Confirm Deleting Account": "Confirm Deleting Account",
"Confirm Deleting Epic": "Confirm Deleting Epic",
"Confirm Deleting Org With Unretrieved Changes": "Confirm Deleting Org With Unretrieved Changes",
Expand Down Expand Up @@ -174,7 +173,6 @@
"GitHub Repository Name": "GitHub Repository Name",
"Go Back": "Go Back",
"Heading": "Heading",
"Health Check": "Health Check",
"Hello! What can Metecho help you do today?": "Hello! What can Metecho help you do today?",
"Help Walkthrough": "Help Walkthrough",
"Home": "Home",
Expand Down Expand Up @@ -397,7 +395,6 @@
"Tester": "Tester",
"Tester & Test Org": "Tester & Test Org",
"The current scratch org cannot be transferred to the selected GitHub user. Remove the scratch org before transferring this task or correct the following issues: {{issueDescription}}": "The current scratch org cannot be transferred to the selected GitHub user. Remove the scratch org before transferring this task or correct the following issues: {{issueDescription}}",
"The existing Dev Org for this Task has unretrieved changes. Changing the assigned Developer will also delete the Org, and any changes will be lost. Are you sure you want to do that?": "The existing Dev Org for this Task has unretrieved changes. Changing the assigned Developer will also delete the Org, and any changes will be lost. Are you sure you want to do that?",
"The existing Dev Org for this Task has unretrieved changes. Removing the assigned Developer will also delete the Org, and any changes will be lost. Are you sure you want to do that?": "The existing Dev Org for this Task has unretrieved changes. Removing the assigned Developer will also delete the Org, and any changes will be lost. Are you sure you want to do that?",
"The last line of the log is “{{message}}” If you need support, your scratch org id is {{orgId}}.": "The last line of the log is “{{message}}” If you need support, your scratch org id is {{orgId}}.",
"There are no available Epic Collaborators.": "There are no available Epic Collaborators.",
Expand Down
2 changes: 1 addition & 1 deletion metecho/api/gh.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
import os
import pathlib
import shutil
from typing import Generator
import zipfile
from glob import glob
from typing import Generator

from cumulusci.utils import temporary_dir
from django.conf import settings
Expand Down
5 changes: 3 additions & 2 deletions metecho/api/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,9 @@ def create_repository(

else:
repo = org.create_repository(
project.repo_name, description=project.description,
private=settings.ENABLE_CREATE_PRIVATE_REPO
project.repo_name,
description=project.description,
private=settings.ENABLE_CREATE_PRIVATE_REPO,
)
team.add_repository(repo.full_name, permission="push")
project.repo_id = repo.id
Expand Down
6 changes: 3 additions & 3 deletions metecho/api/migrations/0118_project_deleted_at.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
class Migration(migrations.Migration):

dependencies = [
('api', '0117_scratchorg_installed_packages'),
("api", "0117_scratchorg_installed_packages"),
]

operations = [
migrations.AddField(
model_name='project',
name='deleted_at',
model_name="project",
name="deleted_at",
field=models.DateTimeField(blank=True, null=True),
),
]
9 changes: 7 additions & 2 deletions metecho/api/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from django.contrib.auth.models import AbstractUser
from django.contrib.auth.models import UserManager as BaseUserManager
from django.contrib.sites.models import Site
from django.core.exceptions import ValidationError
from django.core.exceptions import MultipleObjectsReturned, ValidationError
from django.core.mail import send_mail
from django.core.serializers.json import DjangoJSONEncoder
from django.db import models, transaction
Expand Down Expand Up @@ -240,6 +240,9 @@ def avatar_url(self) -> Optional[str]:
return self.github_account.get_avatar_url()
except (AttributeError, KeyError, TypeError):
return None
# if social app exists in both db and settings retrun sample url
except MultipleObjectsReturned:
return "https://example.com/avatar/"

@property
def org_id(self) -> Optional[str]:
Expand Down Expand Up @@ -716,8 +719,10 @@ def __str__(self):
return self.name

def save(self, *args, **kwargs):
if not self.id:
super().save(*args, **kwargs)
self.update_status()
return super().save(*args, **kwargs)
return super().save()

def subscribable_by(self, user): # pragma: nocover
return True
Expand Down
1 change: 1 addition & 0 deletions metecho/api/push.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
scratchorg.list
SCRATCH_ORG_RECREATE
"""

from copy import deepcopy
from typing import TYPE_CHECKING, Optional

Expand Down
6 changes: 3 additions & 3 deletions metecho/api/tests/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -880,9 +880,9 @@ def test_assign__scratch_org_transfer(
_ = user_factory(
socialaccount_set__provider="github",
socialaccount_set__uid=GH_WITH_METECHO_ID,
devhub_username=FIRST_DEVHUB_USER
if target_has_same_dev_hub
else SECOND_DEVHUB_USER,
devhub_username=(
FIRST_DEVHUB_USER if target_has_same_dev_hub else SECOND_DEVHUB_USER
),
)
target_gh_with_user = git_hub_user_factory(id=GH_WITH_METECHO_ID)

Expand Down
2 changes: 1 addition & 1 deletion metecho/api/tests/templatetags_api_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def test_serialize(user_factory):
"id": str(user.id),
"username": "[email protected]",
"email": "[email protected]",
"avatar_url": None,
"avatar_url": "https://example.com/avatar/",
"github_id": user.github_id,
"is_staff": False,
"valid_token_for": None,
Expand Down
4 changes: 2 additions & 2 deletions metecho/oauth2/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@


class CustomSocialAccountAdapter(DefaultSocialAccountAdapter):
def authentication_error(self, *args, **kwargs):
def on_authentication_error(self, *args, **kwargs):
"""Make sure that auth errors get logged"""
logger.error(f"Social Account authentication error: {args}, {kwargs}")
return super().authentication_error(*args, **kwargs)
return super().on_authentication_error(*args, **kwargs)
3 changes: 1 addition & 2 deletions metecho/oauth2/github/tests/views.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from unittest import mock

import pytest
from allauth.socialaccount.models import SocialApp

from ..views import CustomGitHubOAuth2Adapter

Expand All @@ -10,7 +9,7 @@ class TestGitHubOAuth2Adapter:
@pytest.mark.django_db
def test_complete_login(self, mocker, rf):
mocker.patch("metecho.oauth2.github.views.GitHubOAuth2Adapter.complete_login")
token = mock.MagicMock(app=SocialApp(provider="github"))
token = mock.MagicMock()
request = rf.get("/")
adapter = CustomGitHubOAuth2Adapter(request)
adapter.complete_login(request, None, token)
Expand Down
8 changes: 1 addition & 7 deletions metecho/oauth2/github/views.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
from allauth.socialaccount.providers.github.views import GitHubOAuth2Adapter

from ..views import (
LoggingOAuth2CallbackView,
LoggingOAuth2LoginView,
ensure_socialapp_in_db,
)
from ..views import LoggingOAuth2CallbackView, LoggingOAuth2LoginView


class CustomGitHubOAuth2Adapter(GitHubOAuth2Adapter):
"""GitHub adapter that can handle the app being configured in settings"""

def complete_login(self, request, app, token, **kwargs):
# make sure token is attached to a SocialApp in the db
ensure_socialapp_in_db(token)
return super().complete_login(request, app, token, **kwargs)


Expand Down
19 changes: 15 additions & 4 deletions metecho/oauth2/salesforce/tests/provider.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,25 @@
import pytest

from ..provider import CustomSalesforceProvider


def test_get_auth_params(rf):
@pytest.mark.django_db
def test_get_auth_params(rf, social_app_factory):
request = rf.get("/")
result = CustomSalesforceProvider(request).get_auth_params(request, None)
app = social_app_factory(
provider="salesforce",
)
provider = CustomSalesforceProvider(request, app)
result = provider.get_auth_params(request, None)
assert "prompt" in result and result["prompt"] == "login"


def test_extract_uid(rf):
@pytest.mark.django_db
def test_extract_uid(rf, social_app_factory):
request = rf.get("/")
provider = CustomSalesforceProvider(request)
app = social_app_factory(
provider="salesforce",
)
provider = CustomSalesforceProvider(request, app)
result = provider.extract_uid({"organization_id": "ORG", "user_id": "USER"})
assert result == "ORG/USER"
8 changes: 1 addition & 7 deletions metecho/oauth2/salesforce/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@

from metecho.api.constants import ORGANIZATION_DETAILS

from ..views import (
LoggingOAuth2CallbackView,
LoggingOAuth2LoginView,
ensure_socialapp_in_db,
)
from ..views import LoggingOAuth2CallbackView, LoggingOAuth2LoginView

logger = logging.getLogger(__name__)
ORGID_RE = re.compile(r"^00D[a-zA-Z0-9]{15}$")
Expand Down Expand Up @@ -66,8 +62,6 @@ def get_org_details(self, extra_data, token):
return resp.json()

def complete_login(self, request, app, token, **kwargs):
# make sure token is attached to a SocialApp in the db
ensure_socialapp_in_db(token)

token = fernet_decrypt(token.token)
headers = {"Authorization": f"Bearer {token}"}
Expand Down
4 changes: 2 additions & 2 deletions metecho/oauth2/tests/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

def test_authentication_error_logs(mocker):
mocker.patch(
"allauth.socialaccount.adapter.DefaultSocialAccountAdapter.authentication_error"
"allauth.socialaccount.adapter.DefaultSocialAccountAdapter.on_authentication_error"
) # noqa
error = mocker.patch("metecho.oauth2.adapter.logger.error")
adapter = CustomSocialAccountAdapter()
adapter.authentication_error()
adapter.on_authentication_error()
assert error.called
Loading

0 comments on commit 8cf33f6

Please sign in to comment.