[Pki-devel] [PATCH] 98-2 Fixes for comments on patch 98
Fraser Tweedale
ftweedal at redhat.com
Wed Jun 25 06:49:07 UTC 2014
On Tue, Jun 24, 2014 at 05:32:25PM -0400, Abhishek Koneru wrote:
> 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.
>
ACK
> 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 at redhat.com>
> 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
> + at author: Abhishek Koneru <akoneru at redhat.com>
>
> - at 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
>
More information about the Pki-devel
mailing list