Jan Cholasta wrote:
On 7.3.2012 17:12, Rob Crittenden wrote:
Petr Vobornik wrote:
On 03/06/2012 09:56 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Jan Cholasta wrote:
Dne 18.1.2012 00:04, Rob Crittenden napsal(a):
Jan Cholasta wrote:
Dne 16.1.2012 22:02, Rob Crittenden napsal(a):
Rob Crittenden wrote:
Jan Cholasta wrote:
Dne 13.1.2012 20:53, Rob Crittenden napsal(a):
When viewing a certificate it will show the serial number as
hex
(dec).

# ipa service-show HTTP/rawhide.example.com
Principal: HTTP/rawhide.example....@example.com
Certificate: [snip]
Keytab: True
Managed by: rawhide.example.com
Subject: CN=rawhide.example.com,O=EXAMPLE.COM
Serial Number: 0x403 (1027)
Issuer: CN=EXAMPLE.COM Certificate Authority
Not Before: Fri Jan 13 15:00:44 2012 UTC
Not After: Thu Jan 13 15:00:44 2022 UTC
Fingerprint (MD5):
e5:43:17:0d:8d:af:d6:69:d8:fb:eb:ca:79:fb:47:69
Fingerprint (SHA1):
c2:9e:8e:de:42:c9:4a:29:cc:b0:a0:de:57:c7:b7:d8:f9:b5:fe:e6

rob


NACK

Displaying a host or a service in the webUI fails with "IPA
error
3009:
invalid 'serial_number': Decimal or hexadecimal number is
required
for
serial number".

I would suggest to do the nifty formatting of serial numbers on
the
client side, that would fix the webUI issue, allow non-IPA
clients to
parse the number without dissecting the string representation
of it
and
probably also save me a hack in the type conversion overhaul.
You
could
for example add a parameter flag like "format_serial_number" to
indicate
to the client that it should format the value as a serial
number.

Honza


Well, we want to do as little client formatting as possible. The
idea is
to have a very thin client.

It doesn't seem right to me to enforce this specific
representation of
what is really just an integer at the API level. Doing a little
formatting on the client side won't make the client(s) particularly
fat,
will it?

Yes. The current code just outputs labels and data. There is no
"if it
is this attribute then do that" logic.


IMHO there is too much stuff done on server that would make more
sense
to do on client anyway (especially CLI-specific stuff such as CSV
parsing). What is the reason we want such a thin client?

To avoid double work such that every time we want a formatting
change we
have to change it in multiple places. This lesson was learned in v1.

I believe there should be clear separation of presentation and
content,
but perhaps I'm a little bit too idealistic :-).

You have a point, serial number is defined as an integer. Perhaps we
should revisit this decision to display hex at all.




I'll look into fixing the UI side.

I don't see this error in services, it displays correctly. I'm not
sure
if it is my browser or what but hosts don't display much of
anything
for
me.

rob

I have just checked both master and ipa-2-2 and I'm getting the
same
error message (tested in Firefox 9.0.1) when viewing details of a
host
or a service with the usercertificate attribute set.

BTW, wouldn't it make sense to format serial numbers in the cert
plugin
in the same way?

Perhaps. Like I said, I'm not really in favor of this change.

rob

Maybe we can do a compromise of some sort. What about allowing the
client to specify with each request what representation/formatting
the
server should use for the resulting entries and attributes?

That would be mighty flexible but would open a new can of worms. I
think
long term I'd like to be able to request what attributes to see (ala
ldapsearch) but that too is a bit out of scope.

This comes down to Output being rather loosely defined and we already
have a ticket open on that. It basically just defines the broad
types of
data to be returned (string, list, dict, etc) but not the internal
components of complex types.

Took a new approach and created a new output attribute,
serial_number_hex, that is displayed separately.

UI portion added as well.


ACK for the UI part. I attached a patch which extends UI static testing
data - to keep things in solid state.

I think this approach is still evil (as the whole ticket) but I don't
have a better solution (in CLI).

We are in agreement.

Question:
Isn't the '0x' part a bit redundant? The label already says '(hex)'.
However I can buy a 'It is a convention.' argument.

Yes, I did it for convention, plus to avoid confusion for the case where
it looks like a decimal number but isn't, e.g. 10. If you saw:

Serial number: 16
Serial number (hex): 10

It might be confusing. 0x10 would be clearer.

rob

This patch works for me, but let me repeat myself:

 > BTW, wouldn't it make sense to format serial numbers in the cert
 > plugin in the same way?

Honza


Updated.

rob

>From 317b0e50c545b351d83dc48e509b5fd602bf9915 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Tue, 6 Mar 2012 15:53:07 -0500
Subject: [PATCH] Display serial number as HEX (DECIMAL) when showing
 certificates.

https://fedorahosted.org/freeipa/ticket/1991
---
 install/ui/certificate.js                |    8 ++++++
 ipalib/plugins/cert.py                   |   36 ++++++++++++++---------------
 ipalib/plugins/host.py                   |    3 ++
 ipalib/plugins/internal.py               |    1 +
 ipalib/plugins/service.py                |    4 +++
 ipaserver/plugins/dogtag.py              |    6 +++++
 ipaserver/plugins/selfsign.py            |    1 +
 tests/test_xmlrpc/test_host_plugin.py    |    5 ++++
 tests/test_xmlrpc/test_service_plugin.py |    3 ++
 tests/test_xmlrpc/xmlrpc_test.py         |    2 +
 10 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/install/ui/certificate.js b/install/ui/certificate.js
index 9645aad397ee1ee10f2d3751c560575af28e18de..b5356224fa6d48f0e6ce6be597fe923e59178460 100755
--- a/install/ui/certificate.js
+++ b/install/ui/certificate.js
@@ -319,6 +319,7 @@ IPA.cert.view_dialog = function(spec) {
 
     that.subject = IPA.cert.parse_dn(spec.subject);
     that.serial_number = spec.serial_number || '';
+    that.serial_number_hex = spec.serial_number_hex || '';
     that.issuer = IPA.cert.parse_dn(spec.issuer);
     that.issued_on = spec.issued_on || '';
     that.expires_on = spec.expires_on || '';
@@ -368,6 +369,12 @@ IPA.cert.view_dialog = function(spec) {
         }).appendTo(tr);
 
         tr = $('<tr/>').appendTo(table);
+        $('<td>'+IPA.messages.objects.cert.serial_number_hex+':</td>').appendTo(tr);
+        $('<td/>', {
+            text: that.serial_number_hex
+        }).appendTo(tr);
+
+        tr = $('<tr/>').appendTo(table);
         $('<td/>', {
             'colspan': 2,
             'html': '<h3>'+IPA.messages.objects.cert.issued_by+'</h3>'
@@ -785,6 +792,7 @@ IPA.cert.status_widget = function(spec) {
             'title': title,
             'subject': result['subject'],
             'serial_number': result['serial_number'],
+            'serial_number_hex': result['serial_number_hex'],
             'issuer': result['issuer'],
             'issued_on': result['valid_not_before'],
             'expires_on': result['valid_not_after'],
diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
index 130ebc79f844d959fb67719fc56815bec53d95c0..7a3888121b21567d7c3ada0bddb01187aa4bd775 100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -232,37 +232,32 @@ class cert_request(VirtualCommand):
     )
 
     has_output_params = (
-        Str('certificate?',
+        Str('certificate',
             label=_('Certificate'),
-            flags=['no_create', 'no_update', 'no_search'],
         ),
-        Str('subject?',
+        Str('subject',
             label=_('Subject'),
-            flags=['no_create', 'no_update', 'no_search'],
         ),
-        Str('issuer?',
+        Str('issuer',
             label=_('Issuer'),
-            flags=['no_create', 'no_update', 'no_search'],
         ),
-        Str('valid_not_before?',
+        Str('valid_not_before',
             label=_('Not Before'),
-            flags=['no_create', 'no_update', 'no_search'],
         ),
-        Str('valid_not_after?',
+        Str('valid_not_after',
             label=_('Not After'),
-            flags=['no_create', 'no_update', 'no_search'],
         ),
-        Str('md5_fingerprint?',
+        Str('md5_fingerprint',
             label=_('Fingerprint (MD5)'),
-            flags=['no_create', 'no_update', 'no_search'],
         ),
-        Str('sha1_fingerprint?',
+        Str('sha1_fingerprint',
             label=_('Fingerprint (SHA1)'),
-            flags=['no_create', 'no_update', 'no_search'],
         ),
-        Str('serial_number?',
+        Str('serial_number',
             label=_('Serial number'),
-            flags=['no_create', 'no_update', 'no_search'],
+        ),
+        Str('serial_number_hex',
+            label=_('Serial number (hex)'),
         ),
     )
 
@@ -456,9 +451,12 @@ class cert_show(VirtualCommand):
         Str('sha1_fingerprint',
             label=_('Fingerprint (SHA1)'),
         ),
-        Str('revocation_reason?',
+        Str('revocation_reason',
             label=_('Revocation reason'),
         ),
+        Str('serial_number_hex',
+            label=_('Serial number (hex)'),
+        ),
     )
 
     takes_options = (
@@ -565,10 +563,10 @@ class cert_remove_hold(VirtualCommand):
     takes_args = _serial_number
 
     has_output_params = (
-        Flag('unrevoked?',
+        Flag('unrevoked',
             label=_('Unrevoked'),
         ),
-        Str('error_string?',
+        Str('error_string',
             label=_('Error'),
         ),
     )
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 3814215cb62bc2376c817b3163e8acf07f5a294b..9db98e713e52ae8671a4813e59885c6f4a03c2ba 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -146,6 +146,9 @@ host_output_params = (
     Str('serial_number',
         label=_('Serial Number'),
     ),
+    Str('serial_number_hex',
+        label=_('Serial Number (hex)'),
+    ),
     Str('issuer',
         label=_('Issuer'),
     ),
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index deff866eee1c073f3f786686fd2e74f9261ba6b4..160b40154f46e6a9906971ddb51b97744d07c045 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -313,6 +313,7 @@ class i18n_messages(Command):
                 "revoke_confirmation": _("To confirm your intention to revoke this certificate, select a reason from the pull-down list, and click the \"Revoke\" button."),
                 "revoked": _("Certificate Revoked"),
                 "serial_number": _("Serial Number"),
+                "serial_number_hex": _("Serial Number (hex)"),
                 "sha1_fingerprint": _("SHA1 Fingerprint"),
                 "superseded": _("Superseded"),
                 "unspecified": _("Unspecified"),
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index 71e4ac4657193612860ed88729aa7b848838a908..e75d71f03183688e3f01aa3a0fdfa76ec4838505 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -96,6 +96,9 @@ output_params = (
     Str('serial_number',
         label=_('Serial Number'),
     ),
+    Str('serial_number_hex',
+        label=_('Serial Number (hex)'),
+    ),
     Str('issuer',
         label=_('Issuer'),
     ),
@@ -190,6 +193,7 @@ def set_certificate_attrs(entry_attrs):
     cert = x509.load_certificate(cert, datatype=x509.DER)
     entry_attrs['subject'] = unicode(cert.subject)
     entry_attrs['serial_number'] = unicode(cert.serial_number)
+    entry_attrs['serial_number_hex'] = u'0x%X' % cert.serial_number
     entry_attrs['issuer'] = unicode(cert.issuer)
     entry_attrs['valid_not_before'] = unicode(cert.valid_not_before_str)
     entry_attrs['valid_not_after'] = unicode(cert.valid_not_after_str)
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index b31058c1418f7f73854f7ac06701fb6f821a2e40..b56e04f4d8675c34cc5e7db42a1b45402ef79084 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -609,6 +609,7 @@ def parse_profile_submit_result_xml(doc):
         if len(serial_number) == 1:
             serial_number = int(serial_number[0].text, 16) # parse as hex
             response_request['serial_number'] = serial_number
+            response['serial_number_hex'] = u'0x%X' % serial_number
 
         certificate = request.xpath('b64[1]')
         if len(certificate) == 1:
@@ -834,6 +835,7 @@ def parse_display_cert_xml(doc):
     if len(serial_number) == 1:
         serial_number = int(serial_number[0].text, 16) # parse as hex
         response['serial_number'] = serial_number
+        response['serial_number_hex'] = u'0x%X' % serial_number
 
     pkcs7_chain = doc.xpath('//xml/header/pkcs7ChainBase64[1]')
     if len(pkcs7_chain) == 1:
@@ -1026,6 +1028,7 @@ def parse_revoke_cert_xml(doc):
         if len(serial_number) == 1:
             serial_number = int(serial_number[0].text, 16) # parse as hex
             response_record['serial_number'] = serial_number
+            response['serial_number_hex'] = u'0x%X' % serial_number
 
         error_string = record.xpath('error[1]')
         if len(error_string) == 1:
@@ -1187,6 +1190,7 @@ def parse_unrevoke_cert_xml(doc):
     if len(serial_number) == 1:
         serial_number = int(serial_number[0].text, 16) # parse as hex
         response['serial_number'] = serial_number
+        response['serial_number_hex'] = u'0x%X' % serial_number
 
     return response
 
@@ -1471,6 +1475,7 @@ class ra(rabase.rabase):
         if parse_result.has_key('serial_number'):
             # see module documentation concerning serial numbers and XMLRPC
             cmd_result['serial_number'] = unicode(parse_result['serial_number'])
+            cmd_result['serial_number_hex'] = u'0x%X' % int(cmd_result['serial_number'])
 
         if parse_result.has_key('revocation_reason'):
             cmd_result['revocation_reason'] = parse_result['revocation_reason']
@@ -1539,6 +1544,7 @@ class ra(rabase.rabase):
         if request.has_key('serial_number'):
             # see module documentation concerning serial numbers and XMLRPC
             cmd_result['serial_number'] = unicode(request['serial_number'])
+            cmd_result['serial_number_hex'] = u'0x%X' % request['serial_number']
 
         if request.has_key('certificate'):
             cmd_result['certificate'] = request['certificate']
diff --git a/ipaserver/plugins/selfsign.py b/ipaserver/plugins/selfsign.py
index 2f13b1fd583cba851f86ce0fe1897a09c2b1550f..bbf8fa78a0e267f8d023a1e5a77b7a41582e4f21 100644
--- a/ipaserver/plugins/selfsign.py
+++ b/ipaserver/plugins/selfsign.py
@@ -221,6 +221,7 @@ class ra(rabase.rabase):
 
         cmd_result = {}
         cmd_result['serial_number'] = unicode(serial) # convert long to decimal unicode string
+        cmd_result['serial_number_hex'] = u'0x%X' % serial
         cmd_result['certificate']   = unicode(cert)
         cmd_result['subject']       = unicode(subject)
 
diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 4f24b6e3905705aa0403eb9fe4de09e164dd3555..7068d9a3f02ec9b7dfe00e0551888ac084738da7 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/tests/test_xmlrpc/test_host_plugin.py
@@ -26,6 +26,7 @@ from ipalib import api, errors, x509
 from ipalib.dn import *
 from tests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid, fuzzy_digits
 from tests.test_xmlrpc.xmlrpc_test import fuzzy_hash, fuzzy_date, fuzzy_issuer
+from tests.test_xmlrpc.xmlrpc_test import fuzzy_hex
 from tests.test_xmlrpc import objectclasses
 import base64
 
@@ -253,6 +254,7 @@ class test_host(Declarative):
                     subject=lambda x: DN(x) == \
                         DN(('CN',api.env.host),('O',api.env.realm)),
                     serial_number=fuzzy_digits,
+                    serial_number_hex=fuzzy_hex,
                     md5_fingerprint=fuzzy_hash,
                     sha1_fingerprint=fuzzy_hash,
                     issuer=fuzzy_issuer,
@@ -284,6 +286,7 @@ class test_host(Declarative):
                     subject=lambda x: DN(x) == \
                         DN(('CN',api.env.host),('O',api.env.realm)),
                     serial_number=fuzzy_digits,
+                    serial_number_hex=fuzzy_hex,
                     md5_fingerprint=fuzzy_hash,
                     sha1_fingerprint=fuzzy_hash,
                     issuer=fuzzy_issuer,
@@ -482,6 +485,7 @@ class test_host(Declarative):
                     subject=lambda x: DN(x) == \
                         DN(('CN',api.env.host),('O',api.env.realm)),
                     serial_number=fuzzy_digits,
+                    serial_number_hex=fuzzy_hex,
                     md5_fingerprint=fuzzy_hash,
                     sha1_fingerprint=fuzzy_hash,
                     macaddress=[u'00:50:56:30:F6:5F'],
@@ -511,6 +515,7 @@ class test_host(Declarative):
                     subject=lambda x: DN(x) == \
                         DN(('CN',api.env.host),('O',api.env.realm)),
                     serial_number=fuzzy_digits,
+                    serial_number_hex=fuzzy_hex,
                     md5_fingerprint=fuzzy_hash,
                     sha1_fingerprint=fuzzy_hash,
                     macaddress=[u'00:50:56:30:F6:5F', u'00:50:56:2C:8D:82'],
diff --git a/tests/test_xmlrpc/test_service_plugin.py b/tests/test_xmlrpc/test_service_plugin.py
index e97fb7c878ab6b9ec0ef04e5e78b40416c219f92..7eccd206647838567ea50a7cc0521ba7494a42a2 100644
--- a/tests/test_xmlrpc/test_service_plugin.py
+++ b/tests/test_xmlrpc/test_service_plugin.py
@@ -24,6 +24,7 @@ Test the `ipalib/plugins/service.py` module.
 from ipalib import api, errors, x509
 from tests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid, fuzzy_hash
 from tests.test_xmlrpc.xmlrpc_test import fuzzy_digits, fuzzy_date, fuzzy_issuer
+from tests.test_xmlrpc.xmlrpc_test import fuzzy_hex
 from tests.test_xmlrpc import objectclasses
 import base64
 from ipalib.dn import *
@@ -380,6 +381,7 @@ class test_service(Declarative):
                     subject=lambda x: DN(x) == \
                         DN(('CN',api.env.host),('O',api.env.realm)),
                     serial_number=fuzzy_digits,
+                    serial_number_hex=fuzzy_hex,
                     md5_fingerprint=fuzzy_hash,
                     sha1_fingerprint=fuzzy_hash,
                     issuer=fuzzy_issuer,
@@ -407,6 +409,7 @@ class test_service(Declarative):
                     subject=lambda x: DN(x) == \
                         DN(('CN',api.env.host),('O',api.env.realm)),
                     serial_number=fuzzy_digits,
+                    serial_number_hex=fuzzy_hex,
                     md5_fingerprint=fuzzy_hash,
                     sha1_fingerprint=fuzzy_hash,
                     issuer=fuzzy_issuer,
diff --git a/tests/test_xmlrpc/xmlrpc_test.py b/tests/test_xmlrpc/xmlrpc_test.py
index fd30cc63b6ef16f9b12a2fa7ed5197f476fd4521..716ce03a0f74a452917440f90a7c9f6c397cf224 100644
--- a/tests/test_xmlrpc/xmlrpc_test.py
+++ b/tests/test_xmlrpc/xmlrpc_test.py
@@ -53,6 +53,8 @@ fuzzy_date = Fuzzy('^[a-zA-Z]{3} [a-zA-Z]{3} \d{2} \d{2}:\d{2}:\d{2} \d{4} UTC$'
 
 fuzzy_issuer = Fuzzy(type=basestring, test=lambda issuer: valid_issuer(issuer, api.env.realm))
 
+fuzzy_hex = Fuzzy('^0x[0-9a-fA-F]+$', type=basestring)
+
 # Matches password - password consists of all printable characters without whitespaces
 # The only exception is space, but space cannot be at the beggingin or end of the pwd
 fuzzy_password = Fuzzy('^\S([\S ]*\S)*$')
-- 
1.7.6

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to