Enable sudo users to overwrite client ownership (#448)

* Prevent JS development files being sent to docker

* Make error handling more robust

* Extract auth interface

* Treat user as a required argument

* Move scope validation into connexion

* Enable sudo team members to override ownership

Author: c-w

.dockerignore

@@ -12,6 +12,8 @@ opwen_email_client/webapp/static/fonts/
 opwen_email_client/webapp/static/js/*.min.js
 !opwen_email_server/
 !opwen_statuspage/
+opwen_statuspage/build/
+opwen_statuspage/node_modules/
 !tests/
 !makefile
 !requirements*.txt

.travis.yml

@@ -18,7 +18,7 @@ before_script: |
   export DOCKER_TAG="$TRAVIS_TAG"
 
   if [[ "$TEST_MODE" = "live" ]]; then
-    export REGISTRATION_CREDENTIALS="$GITHUB_AUTH_USERNAME:$GITHUB_AUTH_TOKEN"
+    export REGISTRATION_CREDENTIALS="$GITHUB_AUTH_TOKEN"
     export LOKOLE_QUEUE_BROKER_SCHEME="azureservicebus"
     export LOKOLE_RESOURCE_SUFFIX="$SUFFIX"
     export APPINSIGHTS_INSTRUMENTATIONKEY="$SUFFIX"

docker/integtest/1-register-client.sh

@@ -15,7 +15,7 @@ wait_for_client() {
   local i
 
   for i in $(seq 1 "${max_retries}"); do
-    if curl -fs -u "${REGISTRATION_CREDENTIALS}" "http://nginx:8888/api/email/register/developer${client}.lokole.ca" | tee "${out_dir}/register${client}.json"; then
+    if authenticated_request "http://nginx:8888/api/email/register/developer${client}.lokole.ca" | tee "${out_dir}/register${client}.json"; then
       log "Client ${client} is registered"
       return
     fi
@@ -28,48 +28,48 @@ wait_for_client() {
 
 # workflow 3: register a new client called "developer"
 # normally this endpoint would be called during a new lokole device setup
-curl -fs \
+authenticated_request \
+  "http://nginx:8888/api/email/register/" \
   -H "Content-Type: application/json" \
-  -u "${REGISTRATION_CREDENTIALS}" \
-  -d '{"domain":"developer1.lokole.ca"}' \
-  "http://nginx:8888/api/email/register/"
+  -d '{"domain":"developer1.lokole.ca"}'
 
 wait_for_client 1
 
 # registering a client with bad credentials should fail
-if curl -fs \
+if REGISTRATION_CREDENTIALS="baduser:badpassword" authenticated_request \
+  "http://nginx:8888/api/email/register/" \
   -H "Content-Type: application/json" \
-  -u "baduser:badpassword" \
   -d '{"domain":"hacker.lokole.ca"}' \
+; then echo "Was able to register a client with bad basic auth credentials" >&2; exit 4; fi
+
+if REGISTRATION_CREDENTIALS="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" authenticated_request \
   "http://nginx:8888/api/email/register/" \
-; then echo "Was able to register a client with bad credentials" >&2; exit 4; fi
+  -H "Content-Type: application/json" \
+  -d '{"domain":"hacker.lokole.ca"}' \
+; then echo "Was able to register a client with bad bearer credentials" >&2; exit 4; fi
 
 # also register another client to simulate multi-client emails
-curl -fs \
+authenticated_request \
+  "http://nginx:8888/api/email/register/" \
   -H "Content-Type: application/json" \
-  -u "${REGISTRATION_CREDENTIALS}" \
-  -d '{"domain":"developer2.lokole.ca"}' \
-  "http://nginx:8888/api/email/register/"
+  -d '{"domain":"developer2.lokole.ca"}'
 
 wait_for_client 2
 
 # after creating a client, creating the same one again should fail but we should be able to delete it
-curl -fs \
+authenticated_request \
+  "http://nginx:8888/api/email/register/" \
   -H "Content-Type: application/json" \
-  -u "${REGISTRATION_CREDENTIALS}" \
-  -d '{"domain":"developer3.lokole.ca"}' \
-  "http://nginx:8888/api/email/register/"
+  -d '{"domain":"developer3.lokole.ca"}'
 
 wait_for_client 3
 
-if curl -fs \
+if authenticated_request \
+  "http://nginx:8888/api/email/register/" \
   -H "Content-Type: application/json" \
-  -u "${REGISTRATION_CREDENTIALS}" \
   -d '{"domain":"developer3.lokole.ca"}' \
-  "http://nginx:8888/api/email/register/" \
 ; then echo "Was able to register a duplicate client" >&2; exit 5; fi
 
-curl -fs \
-  -u "${REGISTRATION_CREDENTIALS}" \
-  -X DELETE \
-  "http://nginx:8888/api/email/register/developer3.lokole.ca"
+authenticated_request \
+  "http://nginx:8888/api/email/register/developer3.lokole.ca" \
+  -X DELETE

docker/integtest/5-assert-on-results.sh

@@ -19,7 +19,7 @@ if [[ "${num_exceptions}" -ne "${num_exceptions_expected}" ]]; then
   exit 2
 fi
 
-num_users="$(curl -fs 'http://nginx:8888/api/email/metrics/users/developer1.lokole.ca' -u "${REGISTRATION_CREDENTIALS}" | jq -r '.users')"
+num_users="$(authenticated_request 'http://nginx:8888/api/email/metrics/users/developer1.lokole.ca' | jq -r '.users')"
 num_users_expected=1
 
 if [[ "${num_users}" -ne "${num_users_expected}" ]]; then

docker/integtest/utils.sh

@@ -9,6 +9,16 @@ log() {
   echo "$@" >&2
 }
 
+authenticated_request() {
+  local endpoint="$1"; shift
+
+  if [[ "${REGISTRATION_CREDENTIALS}" =~ ^[^:]+:.*$ ]]; then
+    curl -fs "${endpoint}" -u "${REGISTRATION_CREDENTIALS}" "$@"
+  else
+    curl -fs "${endpoint}" -H "Authorization: Bearer ${REGISTRATION_CREDENTIALS}" "$@"
+  fi
+}
+
 az_connection_string() {
   if [[ -z "${AZURITE_HOST}" ]]; then
     echo "AccountName=${AZURITE_ACCOUNT};AccountKey=${AZURITE_KEY};"

install.py

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 """Script to set up a Debian Linux based system as a Lokole client."""
 from argparse import ArgumentParser
-from base64 import b64encode
 from json import dumps
 from json import loads
 from logging import StreamHandler
@@ -421,13 +420,11 @@ def is_enabled(self):
 
 class ClientSetup(Setup):
     def _run(self):
-        request_auth = b64encode(self.args.registration_credentials.encode('ascii')).decode('ascii')
-
         create_request_payload = dumps({'domain': self.client_domain}).encode('utf-8')
         create_request = Request(self.client_url_create)
         create_request.add_header('Content-Type', 'application/json; charset=utf-8')
         create_request.add_header('Content-Length', str(len(create_request_payload)))
-        create_request.add_header('Authorization', 'Basic {}'.format(request_auth))
+        create_request.add_header('Authorization', 'Bearer {}'.format(self.args.registration_credentials))
 
         try:
             with urlopen(create_request, create_request_payload):  # nosec
@@ -440,7 +437,7 @@ def _run(self):
 
         while True:
             get_request = Request(self.client_url_details)
-            get_request.add_header('Authorization', 'Basic {}'.format(request_auth))
+            get_request.add_header('Authorization', 'Bearer {}'.format(self.args.registration_credentials))
             try:
                 with urlopen(get_request) as response:  # nosec
                     response_body = response.read().decode('utf-8')
@@ -476,6 +473,9 @@ def client_url_details(self):
 
     @property
     def is_enabled(self):
+        if ':' in self.args.registration_credentials:
+            self.abort('Registration credential should be set to Github access token, not username and password')
+
         return self.args.sim_type != 'LocalOnly'
 
 
@@ -826,8 +826,7 @@ def cli():
         'Example: "34 * * * *" for once per hour at the 34th minute.'
     ))
     parser.add_argument('registration_credentials', nargs='?', help=(
-        'Username and password (separated by a colon ":") for '
-        'registering with the Lokole server.'
+        'Github access token for registering with the Lokole server.'
     ))
     parser.add_argument('--app_root', default=getenv('OPWEN_APP_ROOT', ''), help=(
         'The URL prefix at which the app will be accessible.'

opwen_email_server/actions.py

@@ -12,8 +12,7 @@
 from opwen_email_server.constants import events
 from opwen_email_server.constants import mailbox
 from opwen_email_server.constants import sync
-from opwen_email_server.services.auth import AzureAuth
-from opwen_email_server.services.auth import NoAuth
+from opwen_email_server.services.auth import Auth
 from opwen_email_server.services.sendgrid import SendSendgridEmail
 from opwen_email_server.services.storage import AzureObjectsStorage
 from opwen_email_server.services.storage import AzureObjectStorage
@@ -214,9 +213,7 @@ def _decode_attachments(cls, email: dict) -> dict:
 
 
 class ReceiveInboundEmail(_Action):
-    def __init__(self, auth: Union[AzureAuth, NoAuth], raw_email_storage: AzureTextStorage, next_task: Callable[[str],
-                                                                                                                None]):
-
+    def __init__(self, auth: Auth, raw_email_storage: AzureTextStorage, next_task: Callable[[str], None]):
         self._auth = auth
         self._raw_email_storage = raw_email_storage
         self._next_task = next_task
@@ -289,7 +286,7 @@ def _action(self, resource_id):  # type: ignore
 
 
 class DownloadClientEmails(_Action):
-    def __init__(self, auth: AzureAuth, client_storage: AzureObjectsStorage, email_storage: AzureObjectStorage,
+    def __init__(self, auth: Auth, client_storage: AzureObjectsStorage, email_storage: AzureObjectStorage,
                  pending_storage: AzureTextStorage):
 
         self._auth = auth
@@ -347,8 +344,7 @@ def _mark_emails_as_delivered(self, domain: str, email_ids: Iterable[str]) -> No
 
 
 class UploadClientEmails(_Action):
-    def __init__(self, auth: AzureAuth, next_task: Callable[[str], None]):
-
+    def __init__(self, auth: Auth, next_task: Callable[[str], None]):
         self._auth = auth
         self._next_task = next_task
 
@@ -367,9 +363,8 @@ def _action(self, client_id, upload_info):  # type: ignore
 
 
 class RegisterClient(_Action):
-    def __init__(self, auth: AzureAuth, client_storage: AzureObjectsStorage, setup_mailbox: Callable[[str, str], None],
+    def __init__(self, auth: Auth, client_storage: AzureObjectsStorage, setup_mailbox: Callable[[str, str], None],
                  setup_mx_records: Callable[[str], None], client_id_source: Callable[[], str]):
-
         self._auth = auth
         self._client_storage = client_storage
         self._setup_mailbox = setup_mailbox
@@ -389,25 +384,25 @@ def _action(self, domain, owner):  # type: ignore
 
 
 class CreateClient(_Action):
-    def __init__(self, auth: AzureAuth, task: Callable[[str, str], None]):
+    def __init__(self, auth: Auth, task: Callable[[str, str], None]):
         self._auth = auth
         self._task = task
 
-    def _action(self, client, **auth_args):  # type: ignore
+    def _action(self, client, user, **auth_args):  # type: ignore
         domain = client['domain']
         if not is_lowercase(domain):
             return 'domain must be lowercase', 400
         if self._auth.client_id_for(domain) is not None:
             return 'client already exists', 409
 
-        self._task(domain, auth_args.get('user'))
+        self._task(domain, user)
 
         self.log_event(events.CLIENT_CREATED, {'domain': domain})  # noqa: E501  # yapf: disable
         return 'accepted', 201
 
 
 class ListClients(_Action):
-    def __init__(self, auth: AzureAuth):
+    def __init__(self, auth: Auth):
         self._auth = auth
 
     def _action(self, **auth_args):  # type: ignore
@@ -420,19 +415,19 @@ def _action(self, **auth_args):  # type: ignore
 
 
 class GetClient(_Action):
-    def __init__(self, auth: AzureAuth, client_storage: AzureObjectsStorage):
+    def __init__(self, auth: Auth, client_storage: AzureObjectsStorage):
         self._auth = auth
         self._client_storage = client_storage
 
-    def _action(self, domain, **auth_args):  # type: ignore
+    def _action(self, domain, user, **auth_args):  # type: ignore
         if not is_lowercase(domain):
             return 'domain must be lowercase', 400
 
         client_id = self._auth.client_id_for(domain)
         if client_id is None:
             return 'client does not exist', 404
 
-        if not self._auth.is_owner(domain, auth_args.get('user')):
+        if not self._auth.is_owner(domain, user):
             return 'client does not belong to the user', 403
 
         access_info = self._client_storage.access_info()
@@ -447,26 +442,25 @@ def _action(self, domain, **auth_args):  # type: ignore
 
 
 class DeleteClient(_Action):
-    def __init__(self, auth: AzureAuth, delete_mailbox: Callable[[str, str], None],
-                 delete_mx_records: Callable[[str], None], mailbox_storage: AzureTextStorage,
-                 pending_storage: AzureTextStorage, user_storage: AzureObjectStorage):
-
+    def __init__(self, auth: Auth, delete_mailbox: Callable[[str, str], None], delete_mx_records: Callable[[str], None],
+                 mailbox_storage: AzureTextStorage, pending_storage: AzureTextStorage,
+                 user_storage: AzureObjectStorage):
         self._auth = auth
         self._delete_mailbox = delete_mailbox
         self._delete_mx_records = delete_mx_records
         self._mailbox_storage = mailbox_storage
         self._pending_storage = pending_storage
         self._user_storage = user_storage
 
-    def _action(self, domain, **auth_args):  # type: ignore
+    def _action(self, domain, user, **auth_args):  # type: ignore
         if not is_lowercase(domain):
             return 'domain must be lowercase', 400
 
         client_id = self._auth.client_id_for(domain)
         if client_id is None:
             return 'client does not exist', 404
 
-        if not self._auth.is_owner(domain, auth_args.get('user')):
+        if not self._auth.is_owner(domain, user):
             return 'client does not belong to the user', 403
 
         self._delete_mailbox(client_id, domain)
@@ -486,12 +480,12 @@ def _delete_index(cls, storage: Union[AzureTextStorage, AzureObjectStorage], dom
 
 
 class CalculateNumberOfUsersMetric(_Action):
-    def __init__(self, auth: AzureAuth, user_storage: AzureObjectStorage):
+    def __init__(self, auth: Auth, user_storage: AzureObjectStorage):
         self._auth = auth
         self._user_storage = user_storage
 
-    def _action(self, domain, **auth_args):  # type: ignore
-        if not self._auth.is_owner(domain, auth_args.get('user')):
+    def _action(self, domain, user, **auth_args):  # type: ignore
+        if not self._auth.is_owner(domain, user):
             return 'client does not belong to the user', 403
 
         users = sum(1 for _ in self._user_storage.iter(f'{domain}/'))
@@ -502,12 +496,12 @@ def _action(self, domain, **auth_args):  # type: ignore
 
 
 class CalculatePendingEmailsMetric(_Action):
-    def __init__(self, auth: AzureAuth, pending_storage: AzureTextStorage):
+    def __init__(self, auth: Auth, pending_storage: AzureTextStorage):
         self._auth = auth
         self._pending_storage = pending_storage
 
-    def _action(self, domain, **auth_args):  # type: ignore
-        if not self._auth.is_owner(domain, auth_args.get('user')):
+    def _action(self, domain, user, **auth_args):  # type: ignore
+        if not self._auth.is_owner(domain, user):
             return 'client does not belong to the user', 403
 
         pending_emails = sum(1 for _ in self._pending_storage.iter(f'{domain}/'))

opwen_email_server/config.py

@@ -54,7 +54,7 @@
 REGISTRATION_USERNAME = env('LOKOLE_REGISTRATION_USERNAME', '')
 REGISTRATION_PASSWORD = env('LOKOLE_REGISTRATION_PASSWORD', '')
 REGISTRATION_GITHUB_ORGANIZATION = env('LOKOLE_REGISTRATION_GITHUB_ORGANIZATION', 'ascoderu')
-REGISTRATION_GITHUB_TEAM = env('LOKOLE_REGISTRATION_GITHUB_ORGANIZATION', 'lokole-registration')
+REGISTRATION_SUDO_TEAM = env('LOKOLE_REGISTRATION_SUDO_TEAM', 'lokole-sudo')
 
 MAX_WIDTH_IMAGES = env.int('LOKOLE_MAX_WIDTH_EMAIL_IMAGES', 200)
 MAX_HEIGHT_IMAGES = env.int('LOKOLE_MAX_HEIGHT_EMAIL_IMAGES', 200)

opwen_email_server/constants/events.py

@@ -8,7 +8,6 @@
 UNREGISTERED_CLIENT = 'unregistered_client'  # type: Final
 UNKNOWN_USER = 'unknown_user'  # type: Final
 BAD_PASSWORD = 'bad_password'  # type: Final  # nosec
-MISSING_SCOPES = 'missing_scopes'  # type: Final
 UNKNOWN_COMPRESSION_FORMAT = 'unknown_compression_format'  # type: Final
 EMAILS_DELIVERED_TO_CLIENT = 'emails_delivered_to_client'  # type: Final
 EMAILS_FORMATTED_FOR_CLIENT = 'emails_formatted_for_client'  # type: Final

opwen_email_server/integration/azure.py

@@ -1,4 +1,5 @@
 from opwen_email_server import config
+from opwen_email_server.services.auth import Auth
 from opwen_email_server.services.auth import AzureAuth
 from opwen_email_server.services.auth import NoAuth
 from opwen_email_server.services.storage import AzureFileStorage
@@ -19,15 +20,18 @@ def get_guid_source() -> NewGuid:
 
 
 @singleton
-def get_auth() -> AzureAuth:
-    return AzureAuth(storage=AzureObjectStorage(
-        account=config.TABLES_ACCOUNT,
-        key=config.TABLES_KEY,
-        host=config.TABLES_HOST,
-        secure=config.TABLES_SECURE,
-        container=config.CONTAINER_AUTH,
-        provider=config.STORAGE_PROVIDER,
-    ))
+def get_auth() -> Auth:
+    return AzureAuth(
+        storage=AzureObjectStorage(
+            account=config.TABLES_ACCOUNT,
+            key=config.TABLES_KEY,
+            host=config.TABLES_HOST,
+            secure=config.TABLES_SECURE,
+            container=config.CONTAINER_AUTH,
+            provider=config.STORAGE_PROVIDER,
+        ),
+        sudo_scope=config.REGISTRATION_SUDO_TEAM,
+    )
 
 
 @singleton