Skip to content
This repository has been archived by the owner on Jul 8, 2022. It is now read-only.

Commit

Permalink
Don't check for CLA if user is in trusted list (#122)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mariatta authored and brettcannon committed Oct 12, 2017
1 parent f90ed24 commit 6549da1
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 6 deletions.
6 changes: 6 additions & 0 deletions README.md
Expand Up @@ -84,6 +84,10 @@ occurring without being given something (in the film it's the knights
requiring a shrubbery, in real life it's lawyers requiring a signed
CLA).

## Trusted Users List
Users in the Trusted Users List will not be checked for CLA. This can be
useful if the user is a bot or an app.

## Deployment
### Running on Heroku

Expand All @@ -93,6 +97,8 @@ CLA).
2. Set the `GH_AUTH_TOKEN` environment variable to the GitHub oauth
token to be used by the bot
3. Set up the Heroku project to get the code for the bot
4. Trusted users can be added to the `CLA_TRUSTED_USERS` environment variable
as comma-separated list.

### Adding to a GitHub repository (Python-specific instructions)
1. Add the appropriate labels (`CLA signed` and `CLA not signed`)
Expand Down
5 changes: 4 additions & 1 deletion ni/__main__.py
@@ -1,6 +1,7 @@
"""Implement a server to check if a contribution is covered by a CLA(s)."""
import asyncio
import http

from typing import Awaitable, Callable

# ONLY third-party libraries that don't break the abstraction promise may be
Expand All @@ -24,7 +25,9 @@ async def respond(request: web.Request) -> web.Response:
contribution = await ContribHost.process(server, request, client)
usernames = await contribution.usernames()
server.log("Usernames: " + str(usernames))
cla_status = await cla_records.check(client, usernames)
trusted_users = server.trusted_users()
usernames_to_check = usernames - trusted_users
cla_status = await cla_records.check(client, usernames_to_check)
server.log("CLA status: " + str(cla_status))
# With a work queue, one could make the updating of the
# contribution a work item and return an HTTP 202 response.
Expand Down
9 changes: 8 additions & 1 deletion ni/abc.py
@@ -1,5 +1,4 @@
import abc
import asyncio
import enum
import http
from typing import AbstractSet, Any, Optional, Tuple
Expand Down Expand Up @@ -59,6 +58,14 @@ def log_exception(self, exc: BaseException) -> None:
def log(self, message: str) -> None:
"""Log the message."""

@abc.abstractmethod
def trusted_users(self) -> AbstractSet[str]:
"""Return a list of trusted users.
Trusted users will not be checked for CLA.
"""
return frozenset()


class ContribHost(abc.ABC):

Expand Down
12 changes: 11 additions & 1 deletion ni/heroku.py
@@ -1,7 +1,7 @@
import os
import sys
import traceback
from typing import Optional
from typing import AbstractSet, Optional

from . import abc as ni_abc

Expand Down Expand Up @@ -34,3 +34,13 @@ def log_exception(self, exc: BaseException) -> None:
def log(self, message: str) -> None:
"""Log a message to stderr."""
print(message, file=sys.stderr)

def trusted_users(self) -> AbstractSet[str]:
"""Return a list of trusted users.
Trusted users will not be checked for CLA.
"""
cla_trusted_users = os.environ.get('CLA_TRUSTED_USERS', '')

return frozenset([trusted.strip().lower()
for trusted in cla_trusted_users.split(",")])
12 changes: 12 additions & 0 deletions ni/test/test_heroku.py
Expand Up @@ -4,6 +4,8 @@
import random
import unittest

from unittest import mock

from .. import heroku


Expand Down Expand Up @@ -63,3 +65,13 @@ def test_log(self):
with contextlib.redirect_stderr(stderr):
self.server.log(message)
self.assertEqual(stderr.getvalue(), message + "\n")

@mock.patch.dict(os.environ, {'CLA_TRUSTED_USERS': "miss-islington,bedevere-bot"})
def test_trusted_users(self):
self.assertEqual(self.server.trusted_users(),
frozenset(["miss-islington", "bedevere-bot"])
)

@mock.patch.dict(os.environ, {'CLA_TRUSTED_USERS': ""})
def test_no_trusted_users(self):
self.assertEqual(self.server.trusted_users(), frozenset({''}))
108 changes: 105 additions & 3 deletions ni/test/test_main.py
@@ -1,5 +1,4 @@
import http
import unittest
import unittest.mock as mock

from .. import __main__
Expand Down Expand Up @@ -41,7 +40,7 @@ async def process(self, server, request, session):

async def usernames(self):
"""Return an iterable of all the contributors' usernames."""
return self._usernames
return frozenset(self._usernames)

async def update(self, status):
"""Update the contribution with the status of CLA coverage."""
Expand All @@ -62,7 +61,7 @@ def test_response(self):
responder = __main__.handler(util.FakeSession, server, cla)
response = self.run_awaitable(responder(request))
self.assertEqual(response.status, 200)
self.assertEqual(cla.usernames, usernames)
self.assertEqual(cla.usernames, frozenset(usernames))
self.assertEqual(contrib.status, status)

def test_ResponseExit(self):
Expand Down Expand Up @@ -118,3 +117,106 @@ def test_contrib_secret_missing(self):
response = self.run_awaitable(responder(request))
# Fails due to secret being provided but response not signed.
self.assertEqual(response.status, 500)

def test_no_trusted_users(self):
usernames = ['miss-islington', 'bedevere-bot']
status = ni_abc.Status.signed
server = util.FakeServerHost()
server.trusted_usernames = ''
cla = FakeCLAHost(status)
contrib = FakeContribHost(usernames)
request = util.FakeRequest()
with mock.patch('ni.__main__.ContribHost', contrib):
responder = __main__.handler(util.FakeSession, server, cla)
response = self.run_awaitable(responder(request))
self.assertEqual(response.status, 200)
self.assertEqual(cla.usernames, frozenset(usernames))
self.assertEqual(contrib.status, status)

def test_not_comma_separated_trusted_users(self):
usernames = ['miss-islington', 'bedevere-bot']
status = ni_abc.Status.signed
server = util.FakeServerHost()
server.trusted_usernames = 'bedevere-bot'
cla = FakeCLAHost(status)
contrib = FakeContribHost(usernames)
request = util.FakeRequest()
with mock.patch('ni.__main__.ContribHost', contrib):
responder = __main__.handler(util.FakeSession, server, cla)
response = self.run_awaitable(responder(request))
self.assertEqual(response.status, 200)
self.assertEqual(cla.usernames, frozenset(['miss-islington']))

def test_comma_separated_trusted_users(self):
usernames = ['brettcannon', 'miss-islington', 'bedevere-bot']
status = ni_abc.Status.signed
server = util.FakeServerHost()
server.trusted_usernames = 'bedevere-bot,miss-islington'
cla = FakeCLAHost(status)
contrib = FakeContribHost(usernames)
request = util.FakeRequest()
with mock.patch('ni.__main__.ContribHost', contrib):
responder = __main__.handler(util.FakeSession, server, cla)
response = self.run_awaitable(responder(request))
self.assertEqual(response.status, 200)
self.assertEqual(cla.usernames, frozenset(['brettcannon']))

def test_comma_separated_trusted_users_with_spaces(self):
usernames = ['brettcannon', 'miss-islington', 'bedevere-bot']

status = ni_abc.Status.signed
server = util.FakeServerHost()
server.trusted_usernames = 'bedevere-bot, miss-islington'
cla = FakeCLAHost(status)
contrib = FakeContribHost(usernames)
request = util.FakeRequest()
with mock.patch('ni.__main__.ContribHost', contrib):
responder = __main__.handler(util.FakeSession, server, cla)
response = self.run_awaitable(responder(request))
self.assertEqual(response.status, 200)
self.assertEqual(cla.usernames, frozenset(['brettcannon']))

def test_trusted_users_ignored_case(self):
usernames = ['brettcannon', 'miss-islington', 'bedevere-bot']

status = ni_abc.Status.signed
server = util.FakeServerHost()
server.trusted_usernames = 'bedevere-bot, Miss-Islington'
cla = FakeCLAHost(status)
contrib = FakeContribHost(usernames)
request = util.FakeRequest()
with mock.patch('ni.__main__.ContribHost', contrib):
responder = __main__.handler(util.FakeSession, server, cla)
response = self.run_awaitable(responder(request))
self.assertEqual(response.status, 200)
self.assertEqual(cla.usernames, frozenset(['brettcannon']))

def test_no_usernames(self):
usernames = []
status = ni_abc.Status.not_signed
server = util.FakeServerHost()
server.trusted_usernames = 'bedevere-bot, miss-islington'
cla = FakeCLAHost(status)
contrib = FakeContribHost(usernames)
request = util.FakeRequest()
with mock.patch('ni.__main__.ContribHost', contrib):
responder = __main__.handler(util.FakeSession, server, cla)
response = self.run_awaitable(responder(request))
self.assertEqual(response.status, 200)
self.assertEqual(cla.usernames, frozenset([]))
self.assertEqual(contrib.status, status)

def test_all_trusted_users(self):
usernames = ['bedevere-bot', 'miss-islington']
status = ni_abc.Status.signed
server = util.FakeServerHost()
server.trusted_usernames = 'bedevere-bot, miss-islington'
cla = FakeCLAHost(status)
contrib = FakeContribHost(usernames)
request = util.FakeRequest()
with mock.patch('ni.__main__.ContribHost', contrib):
responder = __main__.handler(util.FakeSession, server, cla)
response = self.run_awaitable(responder(request))
self.assertEqual(response.status, 200)
self.assertEqual(cla.usernames, frozenset([]))
self.assertEqual(contrib.status, status)
5 changes: 5 additions & 0 deletions ni/test/util.py
Expand Up @@ -97,6 +97,7 @@ class FakeServerHost(ni_abc.ServerHost):
auth_token = 'some_auth_token'
secret = None
user_agent_name = 'Testing-Agent'
trusted_usernames = ''

def port(self):
"""Specify the port to bind the listening socket to."""
Expand All @@ -121,6 +122,10 @@ def log(self, message):
except AttributeError:
self.logged = [message]

def trusted_users(self):
return frozenset(frozenset([trusted.strip().lower()
for trusted in self.trusted_usernames.split(",")]))


class TestCase(unittest.TestCase):

Expand Down

0 comments on commit 6549da1

Please sign in to comment.