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.

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

@W-14870747 - Dependecy updates and disabling Socalapp in db #2160

Merged
merged 5 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
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
2 changes: 1 addition & 1 deletion metecho/oauth2/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ class CustomSocialAccountAdapter(DefaultSocialAccountAdapter):
def 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
2 changes: 1 addition & 1 deletion metecho/oauth2/tests/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

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()
Expand Down
19 changes: 0 additions & 19 deletions metecho/oauth2/views.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import logging

from allauth.socialaccount import providers
from allauth.socialaccount.models import SocialApp
from allauth.socialaccount.providers.oauth2.views import (
OAuth2CallbackView,
OAuth2LoginView,
Expand All @@ -11,23 +9,6 @@
logger = logging.getLogger(__name__)


def ensure_socialapp_in_db(token):
"""Make sure that token is attached to a SocialApp in the db.

Since we are using SocialApps constructed from settings,
there are none in the db for tokens to be related to
unless we create them here.
"""
if token.app.pk is None:
provider = providers.registry.by_id(token.app.provider)
app, created = SocialApp.objects.get_or_create(
provider=provider.id,
name=provider.name,
client_id="-",
)
token.app = app


class LoggingOAuth2LoginView(OAuth2LoginView):
def dispatch(self, request, *args, **kwargs):
ret = super().dispatch(request, *args, **kwargs)
Expand Down
Loading
Loading