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

25 Jun 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@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
+@author: Abhishek Koneru <akoneru@redhat.com>
-@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


    