Re: [Pki-devel] [PATCH] 98-2 Fixes for comments on patch 98

Please review the patch with fixes for comments on patch 98. Issues addressed: 1. Added a try catch block for issue 1(as noted below) caught by ftweedal(nice catch!). 2. Made the data_format mandatory for file_input methods. 3. Added code to create a profile with PolicyOutput and PolicySets. -- Abhishek Please apply patch 97 before applying this patch.

On Fri, 2014-06-20 at 12:02 +1000, Fraser Tweedale wrote: > On Thu, Jun 19, 2014 at 09:58:43AM -0400, Abhishek Koneru wrote: > > Please review the attached patch which adds methods that allow users to > > pass the profile data in a file. > > > > Also attached two sample xml files, original.xml and modified.xml. > > Place them in /tmp before running the profile.py module. > > > > Thanks, > > Abhishek > > > > Hi Abhishek, > > Patch applied and __main__ example run. Seems to do what it says on > the tin; the usual "haven't used it in anger" caveat ^_^. Review > comments inline below. > > Fraser > > > + @pki.handle_exceptions() > > def modify_profile(self, profile_data): > > """ > > Modify an existing profile. > > """ > > - if profile_data is None: > > - raise ValueError("No ProfileData specified") > > + return self._send_profile_request(profile_data, 'modify') > > + > > + def _send_request_in_file(self, path_to_file, data_format, operation): > > + > > + if path_to_file is None: > > + raise ValueError("File path must be specified.") > > + > > + if data_format not in ['xml', 'json']: > > + raise ValueError("Unsupported data type: " + str(data_format)) > > + > > + if operation not in ['create', 'modify']: > > + raise ValueError("Invalid operation specified: " + str(operation)) > > + > > + data = None > > + with open(path_to_file) as input_file: > > + data = input_file.read() > > + > > + if data_format == 'xml': > > + self.headers['Content-type'] = 'application/xml' > > 1) > > Overwriting self.headers['Content-type'] is problematic. For > example, if the data cannot be parsed or lacks an 'id' key, an > exception will be raised and the ProfileClient will be stuck with > the wrong Content-Type header. > > Possible solutions: > > - use try/finally or a context manager to ensure the header gets > reset to 'application/json' even if an exception is raised. > - Modify the _put and _post methods to include an optional argument > to override the content-type header. To avoid special-cases too > many things, this arg could even be a dict that can be merged with > the default headers, e.g.: > > def _post(self, url, payload=None, headers=None): > self.account_client.login() > headers = dict(self.headers, **(headers or {})) > r = self.connection.post(url, payload, headers, query_params) > ... > > Then callers can supply custom headers for a single request > without potentially affecting other requests. > > > + > > + r = None > > + # Sending the data to the server. > > + if operation == 'create': > > + r = self._post(self.profiles_url, data) > > + else: > > + profile_id = None > > + if data_format == 'xml': > > + profile_id = etree.fromstring(data).get('id') > > + else: > > + profile_id = json.loads(data)['id'] > > + > > + if profile_id is None: > > + raise ValueError('Profile Id is not specified.') > > + url = self.profiles_url + '/' + profile_id > > + r = self._put(url, data) > > + > > + # Reset the Content-type header to json(As used by other methods). > > + if data_format == 'xml': > > + self.headers['Content-type'] = 'application/json' > > > > - url = self.profiles_url + '/' + str(profile_data.profile_id) > > - profile_object = json.dumps(profile_data, cls=encoder.CustomTypeEncoder, > > - sort_keys=True) > > - r = self._put(url, profile_object) > > return Profile.from_json(r.json()) > > > > + @pki.handle_exceptions() > > + def create_profile_using_file_input(self, path_to_file, data_format="xml"): > > + """ > > + Create a new profile from a profile object stored in a file. > > + Acceptable data formats - json, xml. > > + """ > > + return self._send_request_in_file(path_to_file, data_format, 'create') > > + > > 2) > > Default ``data_format="xml"`` makes it too easy for people to misuse > the API, e.g. a path to a JSON file, but no ``data_format`` kwarg > given, resulting in default "xml" being used. I suggest either > making it a compulsory argument, or making its default ``None`` and, > if no explicit ``data_format`` arg given, using the file extension. > > > > + @pki.handle_exceptions() > > + def modify_profile_using_file_input(self, path_to_file, data_format="xml"): > > + """ > > + Modify a profile from a profile object stored in a file. > > + Acceptable data formats - json, xml. > > + """ > > + return self._send_request_in_file(path_to_file, data_format, 'modify') > > + > > + @pki.handle_exceptions() > > def delete_profile(self, profile_id): > > """ > > Delete a profile. > > @@ -1185,6 +1253,23 @@ def main(): > > # pylint: disable-msg=W0703 > > except Exception as e: > > print str(e) > > + print > > + > > + # Creating a profile from file > > + print('Creating a profile using file input.') > > + print('------------------------------------') > > + original = profile_client.create_profile_using_file_input( > > + '/tmp/original.xml') > > + print(original) > > + print > > + > > + # Modifying a profile from file > > + print('Modifying a profile using file input.') > > + print('------------------------------------') > > + modified = profile_client.modify_profile_using_file_input( > > + '/tmp/modified.xml') > > + print(modified) > > + print > > 3) > > Nit-pick: could you delete the created profile here so that the > example can be run multiple times?

From 3a031e463e6fb0b5658ad9e1bc37a018c99f32de Mon Sep 17 00:00:00 2001 From: Abhishek Koneru <akoneru(a)redhat.com&gt; Date: Thu, 19 Jun 2014 00:10:13 -0400 Subject: [PATCH] Added methods for providing file input for profile request. Added new methods to allow user to provide file input to perform operations like create profile/modify profile. The supported file formats a re xml and json. --- base/common/python/pki/__init__.py | 4 +- base/common/python/pki/profile.py | 265 ++++++++++++++++++++++++++++++++++--- 2 files changed, 252 insertions(+), 17 deletions(-) diff --git a/base/common/python/pki/__init__.py b/base/common/python/pki/__init__.py index e9b726cf763785b4a700ef314ff27774b13aba40..891d6ea6364b037f132ff3754b73b372c638b0f7 100644 --- a/base/common/python/pki/__init__.py +++ b/base/common/python/pki/__init__.py @@ -168,7 +168,7 @@ class PKIException(Exception, ResourceMessage): ret = cls(json_value['Message'], json_value['Code'], json_value['ClassName']) for attr in json_value['Attributes']['Attribute']: - print(str(attr)) + print str(attr) ret.add_attribute(attr["name"], attr["value"]) return ret @@ -299,7 +299,7 @@ class PropertyFile(object): def show(self): """ Show contents of property file.""" for line in self.lines: - print(line) + print line def insert_line(self, index, line): """ Insert line in property file """ diff --git a/base/common/python/pki/profile.py b/base/common/python/pki/profile.py index 4f08ee5ba9527855f63b2785c32f9aaddc8d1289..2d4ce5ace11bc6417032fb89ecf6b18707b6fca0 100644 --- a/base/common/python/pki/profile.py +++ b/base/common/python/pki/profile.py @@ -1,12 +1,27 @@ #!/usr/bin/python """ -Created on May 13,, 2014 +@author: Abhishek Koneru <akoneru(a)redhat.com&gt; -@author: akoneru + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; version 2 of the License. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License along + with this program; if not, write to the Free Software Foundation, Inc., + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + + Copyright (C) 2014 Red Hat, Inc. + All rights reserved. """ import json import types +from lxml import etree import pki import pki.client as client @@ -433,6 +448,38 @@ class PolicyDefault(object): def policy_params(self, value): setattr(self, 'params', value) + def add_attribute(self, profile_attribute): + self.policy_attributes.append(profile_attribute) + + def remove_attribute(self, profile_attribute_name): + for attr in self.policy_attributes: + if attr.name == profile_attribute_name: + self.policy_attributes.remove(attr) + break + + def get_attribute(self, profile_attribute_name): + for attr in self.policy_attributes: + if attr.name == profile_attribute_name: + return attr + + return None + + def add_parameter(self, profile_parameter): + self.policy_params.append(profile_parameter) + + def remove_parameter(self, profile_parameter_name): + for param in self.policy_params: + if param.name == profile_parameter_name: + self.policy_params.remove(param) + break + + def get_parameter(self, profile_parameter_name): + for param in self.policy_params: + if param.name == profile_parameter_name: + return param + + return None + @classmethod def from_json(cls, json_value): policy_def = cls() @@ -531,6 +578,22 @@ class PolicyConstraint(object): def policy_constraint_values(self, value): setattr(self, 'constraint', value) + def add_constraint_value(self, policy_constraint_value): + self.policy_constraint_values.append(policy_constraint_value) + + def remove_attribute(self, policy_constraint_value_name): + for attr in self.policy_constraint_values: + if attr.name == policy_constraint_value_name: + self.policy_constraint_values.remove(attr) + break + + def get_attribute(self, policy_constraint_value_name): + for constraint in self.policy_constraint_values: + if constraint.name == policy_constraint_value_name: + return constraint + + return None + @classmethod def from_json(cls, json_value): policy_constraint = cls() @@ -995,31 +1058,99 @@ class ProfileClient(object): """ return self._modify_profile_state(profile_id, 'disable') + def _send_profile_request(self, profile_data, operation): + + if profile_data is None: + raise ValueError("No ProfileData specified") + + if operation not in ['create', 'modify']: + raise ValueError("Invalid operation specified: " + str(operation)) + + profile_object = json.dumps(profile_data, cls=encoder.CustomTypeEncoder, + sort_keys=True) + r = None + if operation == 'create': + r = self._post(self.profiles_url, profile_object) + else: + if profile_data.profile_id is None: + raise ValueError("Profile Id is not specified.") + url = self.profiles_url + '/' + str(profile_data.profile_id) + r = self._put(url, profile_object) + + return Profile.from_json(r.json()) + + @pki.handle_exceptions() def create_profile(self, profile_data): """ Create a new profile for the given ProfileData object. """ - if profile_data is None: - raise ValueError("No ProfileData specified") - - profile_object = json.dumps(profile_data, cls=encoder.CustomTypeEncoder, - sort_keys=True) - r = self._post(self.profiles_url, profile_object) - return Profile.from_json(r.json()) + return self._send_profile_request(profile_data, 'create') + @pki.handle_exceptions() def modify_profile(self, profile_data): """ Modify an existing profile. """ - if profile_data is None: - raise ValueError("No ProfileData specified") + return self._send_profile_request(profile_data, 'modify') + + def _send_request_in_file(self, path_to_file, data_format, operation): + + if path_to_file is None: + raise ValueError("File path must be specified.") + + if data_format not in ['xml', 'json']: + raise ValueError("Unsupported data type: " + str(data_format)) + + if operation not in ['create', 'modify']: + raise ValueError("Invalid operation specified: " + str(operation)) + + data = None + with open(path_to_file) as input_file: + data = input_file.read() + + if data_format == 'xml': + self.headers['Content-type'] = 'application/xml' + + r = None + try: + # Sending the data to the server. + if operation == 'create': + r = self._post(self.profiles_url, data) + else: + profile_id = None + if data_format == 'xml': + profile_id = etree.fromstring(data).get('id') + else: + profile_id = json.loads(data)['id'] + + if profile_id is None: + raise ValueError('Profile Id is not specified.') + url = self.profiles_url + '/' + profile_id + r = self._put(url, data) + finally: + # Reset the Content-type header to json(As used by other methods). + if data_format == 'xml': + self.headers['Content-type'] = 'application/json' - url = self.profiles_url + '/' + str(profile_data.profile_id) - profile_object = json.dumps(profile_data, cls=encoder.CustomTypeEncoder, - sort_keys=True) - r = self._put(url, profile_object) return Profile.from_json(r.json()) + @pki.handle_exceptions() + def create_profile_using_file_input(self, path_to_file, data_format): + """ + Create a new profile from a profile object stored in a file. + Acceptable data formats - json, xml. + """ + return self._send_request_in_file(path_to_file, data_format, 'create') + + @pki.handle_exceptions() + def modify_profile_using_file_input(self, path_to_file, data_format): + """ + Modify a profile from a profile object stored in a file. + Acceptable data formats - json, xml. + """ + return self._send_request_in_file(path_to_file, data_format, 'modify') + + @pki.handle_exceptions() def delete_profile(self, profile_id): """ Delete a profile. @@ -1108,6 +1239,7 @@ def main(): renewal=False, xml_output=False, authorization_acl="") + # Adding a profile input profile_input = ProfileInput("i1", "subjectNameInputImpl") profile_input.add_attribute(ProfileAttribute("sn_uid")) profile_input.add_attribute(ProfileAttribute("sn_e")) @@ -1121,6 +1253,92 @@ def main(): profile_data.add_input(profile_input) + # Adding a profile output + profile_output = ProfileOutput("o1", name="Certificate Output", + class_id="certOutputImpl") + profile_output.add_attribute(ProfileAttribute("pretty_cert")) + profile_output.add_attribute(ProfileAttribute("b64_cert")) + + profile_data.add_output(profile_output) + + # Create a Policy set with a list of profile policies + policy_list = [] + + # Creating profile policy + policy_default = PolicyDefault("Subject Name Default", + "userSubjectNameDefaultImpl", + "This default populates a User-Supplied " + "Certificate Subject Name to the request.") + + attr_descriptor = Descriptor(syntax="string", description="Subject Name") + policy_attribute = ProfileAttribute("name", descriptor=attr_descriptor) + policy_default.add_attribute(policy_attribute) + + policy_constraint = PolicyConstraint("Subject Name Constraint", + "This constraint accepts the subject " + "name that matches UID=.*", + "subjectNameConstraintImpl") + constraint_descriptor = Descriptor(syntax="string", + description="Subject Name Pattern") + policy_constraint_value = PolicyConstraintValue("pattern", + "UID=.*", + constraint_descriptor) + policy_constraint.add_constraint_value(policy_constraint_value) + + policy_list.append(ProfilePolicy("1", policy_default, policy_constraint)) + + # Creating another profile policy + # Defining the policy default + policy_default = PolicyDefault("Validity Default", "validityDefaultImpl", + "This default populates a Certificate " + "Validity to the request. The default " + "values are Range=180 in days") + attr_descriptor = Descriptor(syntax="string", description="Not Before") + policy_attribute = ProfileAttribute("notBefore", descriptor=attr_descriptor) + policy_default.add_attribute(policy_attribute) + + attr_descriptor = Descriptor(syntax="string", description="Not After") + policy_attribute = ProfileAttribute("notAfter", descriptor=attr_descriptor) + policy_default.add_attribute(policy_attribute) + + profile_param = ProfileParameter("range", 180) + profile_param2 = ProfileParameter("startTime", 0) + policy_default.add_parameter(profile_param) + policy_default.add_parameter(profile_param2) + + #Defining the policy constraint + policy_constraint = PolicyConstraint("Validity Constraint", + "This constraint rejects the validity " + "that is not between 365 days.", + "validityConstraintImpl") + constraint_descriptor = Descriptor(syntax="integer", + description="Validity Range (in days)", + default_value=365) + policy_constraint_value = PolicyConstraintValue("range", 365, + constraint_descriptor) + policy_constraint.add_constraint_value(policy_constraint_value) + + constraint_descriptor = Descriptor(syntax="boolean", default_value=False, + description="Check Not Before against" + " current time") + policy_constraint_value = PolicyConstraintValue("notBeforeCheck", False, + constraint_descriptor) + policy_constraint.add_constraint_value(policy_constraint_value) + + constraint_descriptor = Descriptor(syntax="boolean", default_value=False, + description="Check Not After against" + " Not Before") + policy_constraint_value = PolicyConstraintValue("notAfterCheck", False, + constraint_descriptor) + policy_constraint.add_constraint_value(policy_constraint_value) + + policy_list.append(ProfilePolicy("2", policy_default, policy_constraint)) + + policy_set = PolicySet("userCertSet", policy_list) + + profile_data.add_policy_set(policy_set) + + # Create a new profile created_profile = profile_client.create_profile(profile_data) print(created_profile) print @@ -1185,6 +1403,23 @@ def main(): # pylint: disable-msg=W0703 except Exception as e: print str(e) + print + + # Creating a profile from file + print('Creating a profile using file input.') + print('------------------------------------') + original = profile_client.create_profile_using_file_input( + '/tmp/original.xml', 'xml') + print(original) + print + + # Modifying a profile from file + print('Modifying a profile using file input.') + print('------------------------------------') + modified = profile_client.modify_profile_using_file_input( + '/tmp/modified.xml', 'xml') + print(modified) + print if __name__ == "__main__": -- 1.8.5.3