From b12bda2d5554bc041467683ba3594d89ffcfde88 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Wed, 7 Sep 2016 16:33:35 -0400 Subject: [PATCH] Check FAS for packager status, not pkgdb. Before this, we were consulting pkgdb to see if the given user was the maintainer of any packages. That mostly works... but technically, we want to consult FAS to see if the user is in the packager group. We found this when @mprahl was unable to submit builds to rida. I added him to the packager group, but nothing changed! (As written, he had to actually own a package). This change fixes all that so that we query FAS instead of pkgdb. Tests are added. --- config.py | 9 +++++- requirements.txt | 1 + rida/auth.py | 60 ++++++++++++++++++++------------------ rida/config.py | 27 +++++++++++++++++ rida/views.py | 10 ++++--- tests/test_auth.py | 72 ++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 146 insertions(+), 33 deletions(-) create mode 100644 tests/test_auth.py diff --git a/config.py b/config.py index 72d4eb46..d863adc0 100644 --- a/config.py +++ b/config.py @@ -37,6 +37,8 @@ class BaseConfiguration(object): PKGDB_API_URL = 'https://admin.stg.fedoraproject.org/pkgdb/api' + FAS_URL = 'https://admin.stg.fedoraproject.org/accounts' + # Available backends are: console, file, journal. LOG_BACKEND = 'journal' @@ -50,7 +52,12 @@ class BaseConfiguration(object): class DevConfiguration(BaseConfiguration): LOG_BACKEND = 'console' HOST = '0.0.0.0' + FAS_USERNAME = 'put your fas username here' + #FAS_PASSWORD = 'put your fas password here....' + #FAS_PASSWORD = os.environ('FAS_PASSWORD') # you could store it here + #FAS_PASSWORD = commands.getoutput('pass your_fas_password').strip() class ProdConfiguration(BaseConfiguration): - pass + FAS_USERNAME = 'TODO' + #FAS_PASSWORD = 'another password' diff --git a/requirements.txt b/requirements.txt index 8d7e6d09..1959443b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,3 +10,4 @@ munch Flask-Script Flask-SQLAlchemy Flask-Migrate +python-fedora diff --git a/rida/auth.py b/rida/auth.py index 541bc4ca..82408052 100644 --- a/rida/auth.py +++ b/rida/auth.py @@ -23,10 +23,12 @@ """Auth system based on the client certificate and FAS account""" -from flask import Flask, request from werkzeug.serving import WSGIRequestHandler -import requests -import json + +from rida.errors import Unauthorized + +import fedora.client + class ClientCertRequestHandler(WSGIRequestHandler): """ @@ -49,32 +51,34 @@ class ClientCertRequestHandler(WSGIRequestHandler): environ["SSL_CLIENT_CERT_" + key] = val return environ -def is_packager(pkgdb_api_url): + +def get_username(environ): + """ Extract the user's username from the WSGI environment. """ + + if not "SSL_CLIENT_CERT_commonName" in environ: + raise Unauthorized("No SSL client cert CN could be found to work with") + + return environ["SSL_CLIENT_CERT_commonName"] + + +def assert_is_packager(username, fas_kwargs): + """ Assert that a user is a packager by consulting FAS. + + When user is not a packager (is not in the packager FAS group), an + exception is raised. + + Note that `fas_kwargs` must contain values for `base_url`, `username`, and + `password`. These are required arguments for authenticating with FAS. + (Rida needs its own service account/password to talk to FAS). """ - Returns the username of user associated with current request by checking - client cert's commonName and pkgdb database API. - When user is not a packager (is not in pkgdb), returns None. - """ - if not "SSL_CLIENT_CERT_commonName" in request.environ: - return None + FAS = fedora.client.AccountSystem(**fas_kwargs) + person = FAS.person_by_username(username) - username = request.environ["SSL_CLIENT_CERT_commonName"] + # Check that they have even applied in the first place... + if not 'packager' in person['group_roles']: + raise Unauthorized("%s is not in the packager group" % username) - acl_url = pkgdb_api_url + "/packager/package/" + username - - resp = requests.get(acl_url) - try: - resp.raise_for_status() - except: - return None - - try: - r = json.loads(resp.content.decode('utf-8')) - except: - return None - - if r["output"] == "ok": - return username - - return None + # Check more closely to make sure they're approved. + if person['group_roles']['packager']['role_status'] != 'approved': + raise Unauthorized("%s is not approved in the packager group" % username) diff --git a/rida/config.py b/rida/config.py index b3b124ff..49bba237 100644 --- a/rida/config.py +++ b/rida/config.py @@ -74,6 +74,9 @@ class Config(object): self._ssl_certificate_key_file = "" self._ssl_ca_certificate_file = "" self._pkgdb_api_url = "" + self._fas_url = "" + self._fas_username = "" + self._fas_password = "" self._log_backend = "" self._log_file = "" self._log_level = 0 @@ -257,6 +260,30 @@ class Config(object): def pkgdb_api_url(self, s): self._pkgdb_api_url = str(s) + @property + def fas_url(self): + return self._fas_url + + @fas_url.setter + def fas_url(self, s): + self._fas_url = str(s) + + @property + def fas_username(self): + return self._fas_username + + @fas_username.setter + def fas_username(self, s): + self._fas_username = str(s) + + @property + def fas_password(self): + return self._fas_password + + @fas_password.setter + def fas_password(self, s): + self._fas_password = str(s) + @property def log_backend(self): return self._log_backend diff --git a/rida/views.py b/rida/views.py index 17553cca..5c4eb33d 100644 --- a/rida/views.py +++ b/rida/views.py @@ -47,10 +47,12 @@ from errors import (ValidationError, Unauthorized, UnprocessableEntity, @app.route("/rida/module-builds/", methods=["POST"]) def submit_build(): """Handles new module build submissions.""" - username = rida.auth.is_packager(conf.pkgdb_api_url) - if not username: - raise Unauthorized("You must use your Fedora certificate " - "when submitting a new build") + + username = rida.auth.get_username(request.environ) + rida.auth.assert_is_packager(username, fas_kwargs=dict( + base_url=conf.fas_url, + username=conf.fas_username, + password=conf.fas_password)) try: r = json.loads(request.get_data().decode("utf-8")) diff --git a/tests/test_auth.py b/tests/test_auth.py new file mode 100644 index 00000000..99187645 --- /dev/null +++ b/tests/test_auth.py @@ -0,0 +1,72 @@ +# Copyright (c) 2016 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +# Written by Ralph Bean + +from nose.tools import raises, eq_ + + +import unittest +import mock + +import rida.auth +import rida.errors + + +class TestAuthModule(unittest.TestCase): + @raises(rida.errors.Unauthorized) + def test_get_username_failure(self): + rida.auth.get_username({}) + + def test_get_username_good(self): + # https://www.youtube.com/watch?v=G-LtddOgUCE + name = "Joey Jo Jo Junior Shabadoo" + environ = {'SSL_CLIENT_CERT_commonName': name} + result = rida.auth.get_username(environ) + eq_(result, name) + + @mock.patch('fedora.client.AccountSystem') + def test_assert_is_packager(self, AccountSystem): + FAS = mock.MagicMock() + FAS.person_by_username.return_value = { + 'group_roles': { + 'packager': { + 'role_status': 'approved', + }, + }, + } + AccountSystem.return_value = FAS + # This should not raise an exception + rida.auth.assert_is_packager('ralph', dict()) + + @raises(rida.errors.Unauthorized) + @mock.patch('fedora.client.AccountSystem') + def test_assert_is_packager_failure(self, AccountSystem): + FAS = mock.MagicMock() + FAS.person_by_username.return_value = { + 'group_roles': { + 'packager': { + 'role_status': 'FAILLLL', + }, + }, + } + AccountSystem.return_value = FAS + # This should not raise an exception + rida.auth.assert_is_packager('ralph', dict())