Skip to content

Commit

Permalink
Login with GitHub (#7)
Browse files Browse the repository at this point in the history
* Start api models for contribution agreement (#82)

* api models for contribution

* change import sorting and format

* fixture data for testing

* change model fields

* test string parsing

* Migrate models

* fixture data for testing

* api models and tests files

* fix import statements

* fix code format

* Fix static types

* fix linting test file format

* import fix

* Check static code

* import sort

* update DateTime Field

* Tests for models

* Repository testing

* fix class name

* tests for models

* correct formatting and comments

* Change meta classes

* Fix code format and imports

* Delete all fruit files

* Delete last fruit migration

* Fix migration issue

* change data fields

* Apply changes after review

* apply initial migrations

* Fix contributor type error

* Add verbose names

* Fix import error

* Create api serializers

* Test the serializer's functionality

* Check static code

* Add module doc strings

* Contributor tests

* Apply changes from second review

* Migrate models

* Fix doc strings

* check if a contributor has signed latest agreement

* fix name arguments

* Add urls for views

* allow any permission change

* Endpoint to check agreement signature

* fix too many ancestors

* change test data

* Set up test files

* tests for contributor api

* test endpoint action

* test no contributor in data table

* Add validation on time of signature

* sign agreement endpoint

* Update fixture data for new tests

* Complete and test second endpoint

* Improve test coverage

* Apply review changes

* Apply changed from second review

* ignore pylint warning

* Set API version

* Log into github and create new contributor

* test login with github

* add new dependency

* update user data during login

* fix line too long issue

* Refactor code

* Apply review changes

* Override contributor create method

* Apply second review changes

* apply review changes

* Add method doc strings
  • Loading branch information
SalmanAsh authored Aug 5, 2024
1 parent 80b01b9 commit 56516b5
Show file tree
Hide file tree
Showing 8 changed files with 277 additions and 21 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,4 @@ cython_debug/
#.idea/

# Django
static/
static/
4 changes: 2 additions & 2 deletions api/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 3.2.25 on 2024-07-22 14:17
# Generated by Django 3.2.25 on 2024-07-23 13:03

import django.core.validators
from django.db import migrations, models
Expand All @@ -19,7 +19,7 @@ class Migration(migrations.Migration):
('id', models.IntegerField(help_text="The contributor's GitHub user-ID.", primary_key=True, serialize=False)),
('email', models.EmailField(max_length=254, verbose_name='email')),
('name', models.TextField(verbose_name='name')),
('location', models.TextField(verbose_name='location')),
('location', models.TextField(null=True, verbose_name='location')),
('html_url', models.TextField(verbose_name='html url')),
('avatar_url', models.TextField(verbose_name='avatar url')),
],
Expand Down
2 changes: 1 addition & 1 deletion api/models/contributor.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class Contributor(models.Model):
)
email = models.EmailField(_("email"))
name = models.TextField(_("name"))
location = models.TextField(_("location"))
location = models.TextField(_("location"), null=True)
html_url = models.TextField(_("html url"))
avatar_url = models.TextField(_("avatar url"))

Expand Down
3 changes: 1 addition & 2 deletions api/serializers/agreement_signature.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@

from ..models import AgreementSignature, Contributor


# pylint: disable=missing-class-docstring
# pylint: disable=missing-function-docstring
# pylint: disable=too-many-ancestors


class AgreementSignatureSerializer(ModelSerializer[User, AgreementSignature]):
class Meta:
model = AgreementSignature
Expand Down
34 changes: 32 additions & 2 deletions api/serializers/contributor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,45 @@
© Ocado Group
Created on 12/07/2024 at 14:07:59(+01:00).
"""

import typing as t

from codeforlife.serializers import ModelSerializer
from codeforlife.user.models import User

from ..models import Contributor


# pylint: disable-next=missing-class-docstring,too-many-ancestors
class ContributorSerializer(ModelSerializer[User, Contributor]):
"""Contributor serializer class"""

class Meta:
model = Contributor
fields = ["id", "email", "name", "location", "html_url", "avatar_url"]
extra_kwargs: t.Dict[str, t.Dict[str, t.Any]] = {
"id": {"validators": []}
}

def create(self, validated_data):
try:
# Update an existing contributor
contributor = Contributor.objects.get(id=validated_data["id"])
contributor.email = validated_data["email"]
contributor.name = validated_data["name"]
contributor.location = validated_data["location"]
contributor.html_url = validated_data["html_url"]
contributor.avatar_url = validated_data["avatar_url"]

contributor.save(
update_fields=[
"email",
"name",
"location",
"html_url",
"avatar_url",
]
)
except Contributor.DoesNotExist:
# Create a new contributor
contributor = Contributor.objects.create(**validated_data)

return contributor
68 changes: 66 additions & 2 deletions api/views/contributor.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,81 @@
Created on 16/07/2024 at 11:03:09(+01:00).
"""

import requests
from codeforlife.permissions import AllowAny
from codeforlife.request import Request
from codeforlife.response import Response
from codeforlife.user.models import User
from codeforlife.views import ModelViewSet
from codeforlife.views import ModelViewSet, action
from django.conf import settings
from rest_framework import status

from ..models import Contributor
from ..serializers import ContributorSerializer


# pylint: disable-next=missing-class-docstring,too-many-ancestors
class ContributorViewSet(ModelViewSet[User, Contributor]):
http_method_names = ["get", "post"]
http_method_names = ["get"] # "post"
permission_classes = [AllowAny]
serializer_class = ContributorSerializer
queryset = Contributor.objects.all()

# TODO: delete custom action and override default create action.
@action(detail=False, methods=["get"])
def log_into_github(self, request: Request):
"""
Creates a new contributor or updates an existing contributor.
This requires users to authorize us to read their GitHub account.
https://docs.github.com/en/apps/creating-github-apps/
writing-code-for-a-github-app/building-a-login
-with-github-button-with-a-github-app
"""
# Get code from login request
code = request.GET.get("code")
if not code:
return Response(status=status.HTTP_500_INTERNAL_SERVER_ERROR)

# Get user access Token
response = requests.post(
url="https://github.com/login/oauth/access_token",
headers={"Accept": "application/json"},
params={
"client_id": settings.GH_CLIENT_ID,
"client_secret": settings.GH_CLIENT_SECRET,
"code": code,
},
timeout=5,
)

if not response.ok:
return Response(status=response.status_code)
auth_data = response.json()

# Code expired
if "error" in auth_data:
return Response(
status=status.HTTP_451_UNAVAILABLE_FOR_LEGAL_REASONS,
)

# Get user's information
response = requests.get(
url="https://api.github.com/user",
headers={
"Accept": "application/json",
"X-GitHub-Api-Version": "2022-11-28",
# pylint: disable-next=line-too-long
"Authorization": f"{auth_data['token_type']} {auth_data['access_token']}",
},
timeout=5,
)

serializer = self.get_serializer(data=response.json())
serializer.is_valid(raise_exception=True)
self.perform_create(serializer)
return Response(
serializer.data,
status=status.HTTP_201_CREATED,
headers=self.get_success_headers(serializer.data),
)
180 changes: 170 additions & 10 deletions api/views/contributor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@
Created on 16/07/2024 at 14:54:09(+01:00).
"""

import json
from unittest.mock import Mock, patch

import requests
from codeforlife.tests import ModelViewSetTestCase
from codeforlife.user.models import User
from django.conf import settings
from rest_framework import status

from ..models import Contributor
from .contributor import ContributorViewSet
Expand All @@ -29,15 +35,169 @@ def test_retrieve(self):
"""Can retrieve a single contributor."""
self.client.retrieve(model=self.contributor1)

def test_create(self):
"""Can create a contributor."""
self.client.create(
data={
"id": 100,
def test_log_into_github__no_code(self):
"""Login API call does not return a code."""
self.client.get(
self.reverse_action("log_into_github"),
data={"code": ""},
status_code_assertion=status.HTTP_500_INTERNAL_SERVER_ERROR,
)

def _assert_request_github_access_token(self, request: Mock, code: str):
"""Retrieve the use access token in exchange for the code."""
request.assert_called_once_with(
url="https://github.com/login/oauth/access_token",
headers={"Accept": "application/json"},
params={
"client_id": settings.GH_CLIENT_ID,
"client_secret": settings.GH_CLIENT_SECRET,
"code": code,
},
timeout=5,
)

def _assert_request_github_user(self, request: Mock, auth: str):
"""Retrieve user data using the access token."""
request.assert_called_once_with(
url="https://api.github.com/user",
headers={
"Accept": "application/json",
"Authorization": auth,
"X-GitHub-Api-Version": "2022-11-28",
},
timeout=5,
)

def test_log_into_github__no_access_token(self):
"""0Auth API call did not return an access token"""
code = "3e074f3e12656707cf7f"

response = requests.Response()
response.status_code = status.HTTP_500_INTERNAL_SERVER_ERROR

with patch.object(
requests, "post", return_value=response
) as requests_post:
self.client.get(
self.reverse_action("log_into_github"),
data={"code": code},
status_code_assertion=status.HTTP_500_INTERNAL_SERVER_ERROR,
)

self._assert_request_github_access_token(requests_post, code)

def test_log_into_github__code_expired(self):
"""Access token was not generated due to expired code."""
code = "3e074f3e12656707cf7f"

response = requests.Response()
response.status_code = status.HTTP_200_OK
response.encoding = "utf-8"
# pylint: disable-next=protected-access
response._content = json.dumps({"error": "bad request."}).encode(
"utf-8"
)

with patch.object(
requests, "post", return_value=response
) as requests_post:
self.client.get(
self.reverse_action(
"log_into_github",
),
data={"code": code},
# pylint: disable-next=line-too-long
status_code_assertion=status.HTTP_451_UNAVAILABLE_FOR_LEGAL_REASONS,
)

self._assert_request_github_access_token(requests_post, code)

def test_log_into_github__existing_contributor(self):
"""User already logged-in in the past and exists as a contributor"""
code = "3e074f3e12656707cf7f"

response_post = requests.Response()
response_post.status_code = status.HTTP_200_OK
response_post.encoding = "utf-8"
# pylint: disable-next=protected-access
response_post._content = json.dumps(
{"access_token": "123254", "token_type": "Bearer"}
).encode("utf-8")

response_get = requests.Response()
response_get.status_code = status.HTTP_200_OK
response_get.encoding = "utf-8"
# pylint: disable-next=protected-access
response_get._content = json.dumps(
{
"id": 1,
"email": "[email protected]",
"name": "Test Contributor",
"location": "Hatfield",
"html_url": "https://github.com/contributortest",
"avatar_url": "https://contributortest.github.io/",
"name": "contributor one",
"location": "London",
"html_url": "https://github.com/contributor1",
"avatar_url": "https://contributor1.github.io/",
}
)
).encode("utf-8")

with patch.object(
requests, "post", return_value=response_post
) as requests_post:
with patch.object(
requests, "get", return_value=response_get
) as requests_get:
self.client.get(
self.reverse_action(
"log_into_github",
),
data={"code": code},
status_code_assertion=status.HTTP_201_CREATED,
)

self._assert_request_github_access_token(requests_post, code)
self._assert_request_github_user(requests_get, "Bearer 123254")

def test_log_into_github__new_contributor(self):
"""
User is logging-in for the first time and will be added
to the contributor data table
"""
code = "3e074f3e12656707cf7f"

response_post = requests.Response()
response_post.status_code = status.HTTP_200_OK
response_post.encoding = "utf-8"
# pylint: disable-next=protected-access
response_post._content = json.dumps(
{"access_token": "123254", "token_type": "Bearer"}
).encode("utf-8")

response_get = requests.Response()
response_get.status_code = status.HTTP_200_OK
response_get.encoding = "utf-8"
# pylint: disable-next=protected-access
response_get._content = json.dumps(
{
"id": 999999999999999,
"email": "[email protected]",
"name": "contributor new",
"location": "London",
"html_url": "https://github.com/contributornew",
"avatar_url": "https://contributornew.github.io/",
}
).encode("utf-8")

with patch.object(
requests, "post", return_value=response_post
) as requests_post:
with patch.object(
requests, "get", return_value=response_get
) as requests_get:
self.client.get(
self.reverse_action(
"log_into_github",
),
data={"code": code},
)

self._assert_request_github_access_token(requests_post, code)
self._assert_request_github_user(requests_get, "Bearer 123254")
5 changes: 4 additions & 1 deletion settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,18 @@
https://docs.djangoproject.com/en/3.2/ref/settings/
"""

import os
from pathlib import Path

# pylint: disable-next=wildcard-import,unused-wildcard-import
from codeforlife.settings import *

# Github
GH_ORG = "ocadotechnology"
GH_REPO = "codeforlife-workspace"
GH_REPO = "codeforlife-workspace"
GH_FILE = "CONTRIBUTING.md"
GH_CLIENT_ID = os.getenv("GITHUB_CLIENT_ID", "replace-me")
GH_CLIENT_SECRET = os.getenv("GITHUB_CLIENT_SECRET", "replace-me")

# Build paths inside the project like this: BASE_DIR / 'subdir'.
BASE_DIR = Path(__file__).resolve().parent
Expand Down

0 comments on commit 56516b5

Please sign in to comment.