From cc9bedbeb91eff638f1c5b21e35a0de859309404 Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Mon, 13 Aug 2018 11:40:50 -0400 Subject: [PATCH] refactor approval service key to not need approver --- auth/test/test_registry_jwt.py | 2 +- config_app/config_endpoints/api/superuser.py | 46 +++++- .../api/superuser_models_pre_oci.py | 16 ++ .../kube-deploy-modal.component.ts | 1 + .../request-service-key-dialog.html | 137 ++++++++++++++++++ .../request-service-key-dialog.js | 124 ++++++++++++++++ data/model/service_keys.py | 2 +- endpoints/api/superuser_models_pre_oci.py | 2 +- initdb.py | 5 +- test/test_endpoints.py | 6 +- util/generatepresharedkey.py | 3 +- 11 files changed, 331 insertions(+), 13 deletions(-) create mode 100644 config_app/js/components/request-service-key-dialog/request-service-key-dialog.html create mode 100644 config_app/js/components/request-service-key-dialog/request-service-key-dialog.js diff --git a/auth/test/test_registry_jwt.py b/auth/test/test_registry_jwt.py index 78ac029bc..944c12dc5 100644 --- a/auth/test/test_registry_jwt.py +++ b/auth/test/test_registry_jwt.py @@ -155,7 +155,7 @@ def test_mixing_keys_e2e(initialized_db): # Approve the key and try again. admin_user = model.user.get_user('devtable') - model.service_keys.approve_service_key(key.kid, admin_user, ServiceKeyApprovalType.SUPERUSER) + model.service_keys.approve_service_key(key.kid, ServiceKeyApprovalType.SUPERUSER, approver=admin_user) valid_token = _token(token_data, key_id='newkey', private_key=private_key) diff --git a/config_app/config_endpoints/api/superuser.py b/config_app/config_endpoints/api/superuser.py index d35ccca55..be20d484b 100644 --- a/config_app/config_endpoints/api/superuser.py +++ b/config_app/config_endpoints/api/superuser.py @@ -3,15 +3,17 @@ import pathvalidate import os import subprocess -from flask import request, jsonify +from flask import request, jsonify, make_response +from data.database import ServiceKeyApprovalType +from data.model import ServiceKeyDoesNotExist from util.config.validator import EXTRA_CA_DIRECTORY from config_app.config_endpoints.exception import InvalidRequest from config_app.config_endpoints.api import resource, ApiResource, nickname from config_app.config_endpoints.api.superuser_models_pre_oci import pre_oci_model from config_app.config_util.ssl import load_certificate, CertInvalidException -from config_app.c_app import config_provider, INIT_SCRIPTS_LOCATION +from config_app.c_app import app, config_provider, INIT_SCRIPTS_LOCATION logger = logging.getLogger(__name__) @@ -146,4 +148,44 @@ class SuperUserServiceKeyManagement(ApiResource): 'keys': [key.to_dict() for key in keys], }) +@resource('/v1/superuser/approvedkeys/') +class SuperUserServiceKeyApproval(ApiResource): + """ Resource for approving service keys. """ + schemas = { + 'ApproveServiceKey': { + 'id': 'ApproveServiceKey', + 'type': 'object', + 'description': 'Information for approving service keys', + 'properties': { + 'notes': { + 'type': 'string', + 'description': 'Optional approval notes', + }, + }, + }, + } + + @nickname('approveServiceKey') + @validate_json_request('ApproveServiceKey') + def post(self, kid): + notes = request.get_json().get('notes', '') + approver = app.config.get('SUPER_USERS', [])[0] # get the first superuser created in the config tool + try: + key = pre_oci_model.approve_service_key(kid, ServiceKeyApprovalType.SUPERUSER, notes=notes) + + # Log the approval of the service key. + key_log_metadata = { + 'kid': kid, + 'service': key.service, + 'name': key.name, + 'expiration_date': key.expiration_date, + } + + log_action('service_key_approve', None, key_log_metadata) + except ServiceKeyDoesNotExist: + raise NotFound() + except ServiceKeyAlreadyApproved: + pass + + return make_response('', 201) diff --git a/config_app/config_endpoints/api/superuser_models_pre_oci.py b/config_app/config_endpoints/api/superuser_models_pre_oci.py index 3002d5686..22cdbf821 100644 --- a/config_app/config_endpoints/api/superuser_models_pre_oci.py +++ b/config_app/config_endpoints/api/superuser_models_pre_oci.py @@ -17,6 +17,13 @@ def _create_key(key): return ServiceKey(key.name, key.kid, key.service, key.jwk, key.metadata, key.created_date, key.expiration_date, key.rotation_duration, approval) +class ServiceKeyDoesNotExist(Exception): + pass + + +class ServiceKeyAlreadyApproved(Exception): + pass + class PreOCIModel(SuperuserDataInterface): """ @@ -27,5 +34,14 @@ class PreOCIModel(SuperuserDataInterface): keys = model.service_keys.list_all_keys() return [_create_key(key) for key in keys] + def approve_service_key(self, kid, approval_type, notes=''): + try: + key = model.service_keys.approve_service_key(kid, approval_type, notes=notes) + return _create_key(key) + except model.ServiceKeyDoesNotExist: + raise ServiceKeyDoesNotExist + except model.ServiceKeyAlreadyApproved: + raise ServiceKeyAlreadyApproved + pre_oci_model = PreOCIModel() diff --git a/config_app/js/components/kube-deploy-modal/kube-deploy-modal.component.ts b/config_app/js/components/kube-deploy-modal/kube-deploy-modal.component.ts index d55e87d03..47eec013b 100644 --- a/config_app/js/components/kube-deploy-modal/kube-deploy-modal.component.ts +++ b/config_app/js/components/kube-deploy-modal/kube-deploy-modal.component.ts @@ -46,6 +46,7 @@ export class KubeDeployModalComponent { this.errorMessage = `Could cycle the deployments with the new configuration. Error: ${err.toString()}`; }) }).catch(err => { + console.log(err) this.state = 'error'; this.errorMessage = `Could not deploy the configuration. Error: ${err.toString()}`; }) diff --git a/config_app/js/components/request-service-key-dialog/request-service-key-dialog.html b/config_app/js/components/request-service-key-dialog/request-service-key-dialog.html new file mode 100644 index 000000000..27ba1ba13 --- /dev/null +++ b/config_app/js/components/request-service-key-dialog/request-service-key-dialog.html @@ -0,0 +1,137 @@ +
+ + +
+ \ No newline at end of file diff --git a/config_app/js/components/request-service-key-dialog/request-service-key-dialog.js b/config_app/js/components/request-service-key-dialog/request-service-key-dialog.js new file mode 100644 index 000000000..bd3351567 --- /dev/null +++ b/config_app/js/components/request-service-key-dialog/request-service-key-dialog.js @@ -0,0 +1,124 @@ +/** + * An element which displays a dialog for requesting or creating a service key. + */ +angular.module('quay').directive('requestServiceKeyDialog', function () { + var directiveDefinitionObject = { + priority: 0, + templateUrl: '/static/directives/request-service-key-dialog.html', + replace: false, + transclude: true, + restrict: 'C', + scope: { + 'requestKeyInfo': '=requestKeyInfo', + 'keyCreated': '&keyCreated' + }, + controller: function($scope, $element, $timeout, ApiService) { + var handleNewKey = function(key) { + var data = { + 'notes': 'Approved during setup of service ' + key.service + }; + + var params = { + 'kid': key.kid + }; + + ApiService.approveServiceKey(data, params).then(function(resp) { + $scope.keyCreated({'key': key}); + $scope.step = 2; + }, ApiService.errorDisplay('Could not approve service key')); + }; + + var checkKeys = function() { + var isShown = ($element.find('.modal').data('bs.modal') || {}).isShown; + if (!isShown) { + return; + } + + // TODO: filter by service. + ApiService.listServiceKeys().then(function(resp) { + var keys = resp['keys']; + for (var i = 0; i < keys.length; ++i) { + var key = keys[i]; + if (key.service == $scope.requestKeyInfo.service && !key.approval && key.rotation_duration) { + handleNewKey(key); + return; + } + } + + $timeout(checkKeys, 1000); + }, ApiService.errorDisplay('Could not list service keys')); + }; + + $scope.show = function() { + $scope.working = false; + $scope.step = 0; + $scope.requestKind = null; + $scope.preshared = { + 'name': $scope.requestKeyInfo.service + ' Service Key', + 'notes': 'Created during setup for service `' + $scope.requestKeyInfo.service + '`' + }; + + $element.find('.modal').modal({}); + }; + + $scope.hide = function() { + $scope.loading = false; + $element.find('.modal').modal('hide'); + }; + + $scope.showGenerate = function() { + $scope.step = 1; + }; + + $scope.startApproval = function() { + $scope.step = 1; + checkKeys(); + }; + + $scope.isDownloadSupported = function() { + var isSafari = /^((?!chrome).)*safari/i.test(navigator.userAgent); + if (isSafari) { + // Doesn't work properly in Safari, sadly. + return false; + } + + try { return !!new Blob(); } catch(e) {} + return false; + }; + + $scope.downloadPrivateKey = function(key) { + var blob = new Blob([key.private_key]); + FileSaver.saveAs(blob, key.service + '.pem'); + }; + + $scope.createPresharedKey = function() { + $scope.working = true; + + var data = { + 'name': $scope.preshared.name, + 'service': $scope.requestKeyInfo.service, + 'expiration': $scope.preshared.expiration || null, + 'notes': $scope.preshared.notes + }; + + ApiService.createServiceKey(data).then(function(resp) { + $scope.working = false; + $scope.step = 2; + $scope.createdKey = resp; + $scope.keyCreated({'key': resp}); + }, ApiService.errorDisplay('Could not create service key')); + }; + + $scope.updateNotes = function(content) { + $scope.preshared.notes = content; + }; + + $scope.$watch('requestKeyInfo', function(info) { + if (info && info.service) { + $scope.show(); + } + }); + } + }; + return directiveDefinitionObject; +}); \ No newline at end of file diff --git a/data/model/service_keys.py b/data/model/service_keys.py index faeb68269..eb460299b 100644 --- a/data/model/service_keys.py +++ b/data/model/service_keys.py @@ -145,7 +145,7 @@ def set_key_expiration(kid, expiration_date): service_key.save() -def approve_service_key(kid, approver, approval_type, notes=''): +def approve_service_key(kid, approval_type, approver=None, notes=''): try: with db_transaction(): key = db_for_update(ServiceKey.select().where(ServiceKey.kid == kid)).get() diff --git a/endpoints/api/superuser_models_pre_oci.py b/endpoints/api/superuser_models_pre_oci.py index daa97f83f..27a62b0c4 100644 --- a/endpoints/api/superuser_models_pre_oci.py +++ b/endpoints/api/superuser_models_pre_oci.py @@ -118,7 +118,7 @@ class PreOCIModel(SuperuserDataInterface): def approve_service_key(self, kid, approver, approval_type, notes=''): try: - key = model.service_keys.approve_service_key(kid, approver, approval_type, notes=notes) + key = model.service_keys.approve_service_key(kid, approval_type, approver=approver, notes=notes) return _create_key(key) except model.ServiceKeyDoesNotExist: raise ServiceKeyDoesNotExist diff --git a/initdb.py b/initdb.py index 047ae1356..988b52cc6 100644 --- a/initdb.py +++ b/initdb.py @@ -161,8 +161,7 @@ def __generate_service_key(kid, name, user, timestamp, approval_type, expiration rotation_duration=rotation_duration) if approval_type is not None: - model.service_keys.approve_service_key(key.kid, user, approval_type, - notes='The **test** approval') + model.service_keys.approve_service_key(key.kid, approval_type, notes='The **test** approval') key_metadata = { 'kid': kid, @@ -820,7 +819,7 @@ def populate_database(minimal=False, with_storage=False): key = model.service_keys.create_service_key('test_service_key', 'test_service_key', 'quay', _TEST_JWK, {}, None) - model.service_keys.approve_service_key(key.kid, new_user_1, ServiceKeyApprovalType.SUPERUSER, + model.service_keys.approve_service_key(key.kid, ServiceKeyApprovalType.SUPERUSER, notes='Test service key for local/test registry testing') # Add an app specific token. diff --git a/test/test_endpoints.py b/test/test_endpoints.py index 8eb02734e..9140e3e5f 100644 --- a/test/test_endpoints.py +++ b/test/test_endpoints.py @@ -559,7 +559,7 @@ class KeyServerTestCase(EndpointTestCase): }, data=jwk, expected_code=403) # Approve the key. - model.service_keys.approve_service_key('kid420', 1, ServiceKeyApprovalType.SUPERUSER) + model.service_keys.approve_service_key('kid420', ServiceKeyApprovalType.SUPERUSER, approver=1) # Rotate that new key with assert_action_logged('service_key_rotate'): @@ -598,7 +598,7 @@ class KeyServerTestCase(EndpointTestCase): def test_attempt_delete_service_key_with_expired_key(self): # Generate two keys, approving the first. private_key, _ = model.service_keys.generate_service_key('sample_service', None, kid='first') - model.service_keys.approve_service_key('first', 1, ServiceKeyApprovalType.SUPERUSER) + model.service_keys.approve_service_key('first', ServiceKeyApprovalType.SUPERUSER, approver=1) model.service_keys.generate_service_key('sample_service', None, kid='second') # Mint a JWT with our test payload @@ -661,7 +661,7 @@ class KeyServerTestCase(EndpointTestCase): expected_code=403, service='sample_service', kid='kid321') # Approve the second key. - model.service_keys.approve_service_key('kid123', 1, ServiceKeyApprovalType.SUPERUSER) + model.service_keys.approve_service_key('kid123', ServiceKeyApprovalType.SUPERUSER, approver=1) # Using the credentials of our approved key, delete our unapproved key with assert_action_logged('service_key_delete'): diff --git a/util/generatepresharedkey.py b/util/generatepresharedkey.py index 99b5eff22..27e17128a 100644 --- a/util/generatepresharedkey.py +++ b/util/generatepresharedkey.py @@ -18,8 +18,7 @@ def generate_key(service, name, expiration_date=None, notes=None): metadata=metadata, name=name) # Auto-approve the service key. - model.service_keys.approve_service_key(key.kid, None, ServiceKeyApprovalType.AUTOMATIC, - notes=notes or '') + model.service_keys.approve_service_key(key.kid, ServiceKeyApprovalType.AUTOMATIC, notes=notes or '') # Log the creation and auto-approval of the service key. key_log_metadata = {