Repository: slarse/labelbot

Issue:
General refactor
Make sure functions are in appropriate modules, double check docstrings, and stuff like that. Make it look nice.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/slarse/labelbot/pull/46?src=pr&el=h1) Report
> Merging [#46](https://codecov.io/gh/slarse/labelbot/pull/46?src=pr&el=desc) into [master](https://codecov.io/gh/slarse/labelbot/commit/c269a68c3820ebc0d60788a32c34916607323a7e?src=pr&el=desc) will **increase** coverage by `0.36%`.
> The diff coverage is `73.68%`.

[![Impacted file tree graph](https://codecov.io/gh/slarse/labelbot/pull/46/graphs/tree.svg?width=650&token=vwkCZPYZ7N&height=150&src=pr)](https://codecov.io/gh/slarse/labelbot/pull/46?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
+ Coverage   84.95%   85.32%   +0.36%     
==========================================
  Files           4        4              
  Lines         113      109       -4     
  Branches        5        5              
==========================================
- Hits           96       93       -3     
+ Misses         16       15       -1     
  Partials        1        1
```


| [Impacted Files](https://codecov.io/gh/slarse/labelbot/pull/46?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [labelbot/github\_api.py](https://codecov.io/gh/slarse/labelbot/pull/46/diff?src=pr&el=tree#diff-bGFiZWxib3QvZ2l0aHViX2FwaS5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [labelbot/bot.py](https://codecov.io/gh/slarse/labelbot/pull/46/diff?src=pr&el=tree#diff-bGFiZWxib3QvYm90LnB5) | `66.66% <50%> (-3.11%)` | :arrow_down: |
| [labelbot/auth.py](https://codecov.io/gh/slarse/labelbot/pull/46/diff?src=pr&el=tree#diff-bGFiZWxib3QvYXV0aC5weQ==) | `75% <75%> (-1.48%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/slarse/labelbot/pull/46?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/slarse/labelbot/pull/46?src=pr&el=footer). Last update [c269a68...80bc6be](https://codecov.io/gh/slarse/labelbot/pull/46?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/labelbot/auth.py b/labelbot/auth.py
index c23a0a1..a0bfa67 100644
--- a/labelbot/auth.py
+++ b/labelbot/auth.py
@@ -1,9 +1,8 @@
-"""Handles authentication tokens with the GitHub API.
+"""Functions for handling authentication procedures.
 
 .. module:: auth
-    :synopsis: Functions for generating tokens used for authentication with GitHub.
+    :synopsis: Functions for handling authentication procedures.
 .. moduleauthor:: Lars Hummelgren <larshum@kth.se> & Joakim Croona <jcroona@kth.se>
-
 """
 
 import datetime
@@ -12,6 +11,10 @@ import json
 import jwcrypto
 import python_jwt
 import requests
+import boto3
+import botocore
+import hmac
+import hashlib
 
 USER_AGENT = "label-bot"
 
@@ -31,7 +34,7 @@ def generate_jwt_token(private_pem: bytes, app_id: int) -> str:
     return python_jwt.generate_jwt(payload, private_key, "RS256", duration)
 
 
-def generate_installation_access_token(jwt_token: str, installation_id):
+def generate_installation_access_token(jwt_token: str, installation_id) -> str:
     """Generates an installation access token using a JWT token and an installation id.
 
     An installation access token is valid for 1 hour.
@@ -50,3 +53,40 @@ def generate_installation_access_token(jwt_token: str, installation_id):
     url = f"https://api.github.com/app/installations/{installation_id}/access_tokens"
     r = requests.post(url, headers=headers)
     return r.json()["token"]
+
+
+def get_pem(bucket_name: str, bucket_key: str) -> bytes:
+    """Reads a private PEM file from an S3 bucket.
+
+    Args:
+        bucket_name: Name of the S3 bucket.
+        bucket_key: Bucket key for the PEM file.
+    Returns:
+        Contents of the PEM file.
+    """
+    s3 = boto3.resource("s3")
+    s3.Bucket(bucket_name).download_file(bucket_key, "/tmp/key.pem")
+    with open("/tmp/key.pem", "rb") as f:
+        return f.read()
+
+
+def authenticate_request(shared_secret: str, body: str, signature: str) -> bool:
+    """Checks if the MAC (message authentication code) sent in the request is really
+    from GitHub.
+
+    Args:
+        shared_secret: A secret shared between GitHub and the bot.
+        body: Body of the HTTP request.
+        signature: The header containing the MAC.
+    Returns:
+        True iff the signature is a MAC computed with the body of the request and the
+        shared secret.
+    """
+    if signature is None:
+        return False
+
+    sha_body = hmac.new(
+        shared_secret.encode("utf8"), body.encode("utf8"), hashlib.sha1
+    ).hexdigest()
+    _, sha_github = signature.split("=")
+    return hmac.compare_digest(sha_body, sha_github)
diff --git a/labelbot/bot.py b/labelbot/bot.py
index 163071f..7d6090f 100644
--- a/labelbot/bot.py
+++ b/labelbot/bot.py
@@ -1,14 +1,17 @@
+"""Event handler for AWS lambda.
+
+This is the main module of labelbot, and contains the event handler for AWS lambda.  If
+for any reason one would like to use a different service than AWS lambda, this is the
+functionality that needs to be changed.
+
+.. module:: bot
+    :synopsis: Event handler for AWS lambda.
+.. moduleauthor:: Simon Larsén <slarse@kth.se> & Joakim Croona <jcroona@kth.se>
+"""
 import json
-from jwcrypto import jwk
-import python_jwt
 import os
-import boto3
-import botocore
-import hmac
-import hashlib
 from labelbot import auth
 from labelbot import github_api
-from labelbot import parse
 
 
 def lambda_handler(event, context):
@@ -22,12 +25,15 @@ def lambda_handler(event, context):
 
     app_id = int(os.getenv("APP_ID"))
     secret_key = os.getenv("SECRET_KEY")
-    authenticated = authenticate_request(secret_key, event["body"], event["headers"]["X-Hub-Signature"])
+    authenticated = auth.authenticate_request(
+        secret_key, event["body"], event["headers"]["X-Hub-Signature"]
+    )
     if not authenticated:
         return {"statuscode": 403}
+
     bucket_name = os.getenv("BUCKET_NAME")
     bucket_key = os.getenv("BUCKET_KEY")
-    pem = get_pem(bucket_name, bucket_key)
+    pem = auth.get_pem(bucket_name, bucket_key)
 
     jwt_token = auth.generate_jwt_token(pem, app_id)
     access_token = auth.generate_installation_access_token(jwt_token, installation_id)
@@ -37,25 +43,3 @@ def lambda_handler(event, context):
     )
 
     return {"statusCode": 200 if success else 403, "body": json.dumps("temp")}
-
-
-def get_pem(bucket_name, key):
-    """Reads key from s3"""
-    s3 = boto3.resource("s3")
-    s3.Bucket(bucket_name).download_file(key, "/tmp/key.pem")
-    with open("/tmp/key.pem", "rb") as f:
-        pem = f.read()
-    return pem
-
-
-def authenticate_request(key: str, body: str, signature: str) -> bool:
-    """ Chacks if the X-Hub-Signature header exists, and if it does, verifies that the body 
-    matches the hash sent from github."""
-    if signature is None:
-        return False
-
-    sha_body = hmac.new(
-        key.encode("utf8"), body.encode("utf8"), hashlib.sha1
-    ).hexdigest()
-    alg, sha_github = signature.split("=")
-    return hmac.compare_digest(sha_body, sha_github)
diff --git a/labelbot/github_api.py b/labelbot/github_api.py
index d1a052b..66be23d 100644
--- a/labelbot/github_api.py
+++ b/labelbot/github_api.py
@@ -6,7 +6,7 @@
 import json
 import sys
 import base64
-from typing import Sequence, List
+from typing import Iterable, List
 import requests
 
 from labelbot import parse
@@ -60,7 +60,7 @@ def set_allowed_labels(
 
 
 def set_labels(
-    labels: Sequence[str], owner: str, repo: str, issue_nr: int, access_token: str
+    labels: Iterable[str], owner: str, repo: str, issue_nr: int, access_token: str
 ) -> bool:
     """Unconditionally set the provided labels on a repository issue.
 
diff --git a/setup.py b/setup.py
index 0ac9d8b..da892e9 100644
--- a/setup.py
+++ b/setup.py
@@ -3,12 +3,18 @@ from setuptools import setup, find_packages
 with open("README.md", mode="r", encoding="utf-8") as f:
     readme = f.read()
 
-test_requirements = ["pytest>=4.0.0", "codecov", "pytest-cov", "responses","pytest-mock"]
+test_requirements = [
+    "pytest>=4.0.0",
+    "codecov",
+    "pytest-cov",
+    "responses",
+    "pytest-mock",
+]
 required = ["python_jwt", "jwcrypto", "requests", "boto3"]
 
 setup(
     name="labelbot",
-    version="0.0.4",
+    version="0.0.6",
     description=(
         "A GitHub label bot for allowing unprivileged users to label issues "
         "with allowed labels."


Test patch:
diff --git a/tests/test_auth.py b/tests/test_auth.py
index e1df380..c1984a0 100644
--- a/tests/test_auth.py
+++ b/tests/test_auth.py
@@ -33,3 +33,21 @@ def test_auth_jwt_token():
     """Tests that jwt_token is truthy"""
     result = auth.generate_jwt_token(rsa_key.encode("utf8"), 12334)
     assert result
+
+
+class TestAuthenticateRequest:
+    SECRET = "d653a60adc0a16a93e99f0620a67f4a67ef901df"
+    BODY = "Hello, World!"
+    SIGN = "sha1=8727505c9c036b2337a06d2e63f091a7aa41ae60"
+
+    def test_correct_hash(self):
+        result = auth.authenticate_request(self.SECRET, self.BODY, self.SIGN)
+        assert result
+
+    def test_incorrect_hash(self):
+        result = auth.authenticate_request(self.SECRET, self.BODY.lower(), self.SIGN)
+        assert not result
+
+    def test_no_signature(self):
+        result = auth.authenticate_request(self.SECRET, self.BODY, None)
+        assert not result
diff --git a/tests/test_bot.py b/tests/test_bot.py
index 9c6aecf..8e1b66c 100644
--- a/tests/test_bot.py
+++ b/tests/test_bot.py
@@ -1,34 +1,32 @@
 from labelbot import bot
+import responses
 import pytest
-SECRET = "d653a60adc0a16a93e99f0620a67f4a67ef901df"
-BODY = "Hello, World!"
-SIGN = "sha1=8727505c9c036b2337a06d2e63f091a7aa41ae60"
-
-class TestAuthenticateRequest:
-    def test_correct_hash(self):
-        result = bot.authenticate_request(SECRET,BODY, SIGN)
-        assert result
-
-    def test_incorrect_hash(self):
-        result = bot.authenticate_request(SECRET,BODY.lower(), SIGN)
-        assert not result
-
-    def test_no_signature(self):
-        result = bot.authenticate_request(SECRET,BODY, None)
-        assert not result
-
 
 
 @pytest.fixture
 def env_setup(mocker):
-    values = {"APP_ID": "243554", "SECRET_KEY": "66535665634", "BUCKET_NAME": "My_bucket", "BUCKET_KEY": "my_file"}
+    values = {
+        "APP_ID": "243554",
+        "SECRET_KEY": "66535665634",
+        "BUCKET_NAME": "My_bucket",
+        "BUCKET_KEY": "my_file",
+    }
     mocker.patch("os.getenv", autospec=True, side_effect=values.get)
     yield values
 
-def test_lambda_handler_authentication(env_setup):
-    event = {"headers": {"X-Hub-Signature": "sha1=4afefa55e46cc2ac696127dae55b49aeb999b7e8"},"body": jsonstring}
+
+@responses.activate
+def test_lambda_handler_authentication_failure(env_setup):
+    """Test that lambda_handler correctly handles unauthorized requests (here,
+    all of the secrets are bogus).
+    """
+    event = {
+        "headers": {"X-Hub-Signature": "sha1=4afefa55e46cc2ac696127dae55b49aeb999b7e8"},
+        "body": jsonstring,
+    }
     result = bot.lambda_handler(event, None)
-    assert result
+    assert result["statuscode"] == 403
+
 
 jsonstring = """{
   "action": "reopened",
@@ -199,5 +197,3 @@ jsonstring = """{
     "node_id": "MDIzOkludGVncmF0aW9uSW5zdGFsbGF0aW9uODI1OTU4"
   }
 }"""
-
-