Revision history for this message
| William Grant (wgrant) wrote : | # |
| 1 | === added file 'lib/canonical/zeca/ftests/test_locate_key.py' |
| 2 | --- lib/canonical/zeca/ftests/test_locate_key.py 1970-01-01 00:00:00 +0000 |
| 3 | +++ lib/canonical/zeca/ftests/test_locate_key.py 2009-08-20 01:01:54 +0000 |
| 4 | @@ -0,0 +1,43 @@ |
| 5 | +# Copyright 2009 Canonical Ltd. This software is licensed under the |
| 6 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
| 7 | + |
| 8 | +import unittest |
| 9 | +import os.path |
| 10 | + |
| 11 | +from canonical.zeca.zeca import locate_key |
| 12 | + |
| 13 | + |
| 14 | +class LocateKeyTestCase(unittest.TestCase): |
| 15 | + root = os.path.join(os.path.dirname(__file__), 'keys') |
| 16 | + |
| 17 | + def assertKeyFile(self, suffix, filename): |
| 18 | + """Verify that a suffix maps to the given filename.""" |
| 19 | + |
| 20 | + if filename is not None: |
| 21 | + filename = os.path.join(self.root, filename) |
| 22 | + self.assertEqual(locate_key(self.root, suffix), filename) |
| 23 | + |
| 24 | + def test_exact_fingerprint_match(self): |
| 25 | + self.assertKeyFile( |
| 26 | + '0xA419AE861E88BC9E04B9C26FBA2B9389DFD20543.get', |
| 27 | + '0xA419AE861E88BC9E04B9C26FBA2B9389DFD20543.get') |
| 28 | + |
| 29 | + def test_keyid_glob_match(self): |
| 30 | + self.assertKeyFile( |
| 31 | + '0xDFD20543.get', |
| 32 | + '0xA419AE861E88BC9E04B9C26FBA2B9389DFD20543.get') |
| 33 | + |
| 34 | + def test_keyid_without_prefix_glob_match(self): |
| 35 | + self.assertKeyFile( |
| 36 | + 'DFD20543.get', |
| 37 | + '0xA419AE861E88BC9E04B9C26FBA2B9389DFD20543.get') |
| 38 | + |
| 39 | + def test_keyid_no_match(self): |
| 40 | + self.assertKeyFile('0xDEADBEEF.get', None) |
| 41 | + |
| 42 | + |
| 43 | +def test_suite(): |
| 44 | + suite = unittest.TestSuite() |
| 45 | + suite.addTest(unittest.makeSuite(LocateKeyTestCase)) |
| 46 | + return suite |
| 47 | + |
| 48 | |
| 49 | === modified file 'lib/canonical/zeca/zeca.py' |
| 50 | --- lib/canonical/zeca/zeca.py 2009-08-15 03:47:48 +0000 |
| 51 | +++ lib/canonical/zeca/zeca.py 2009-08-20 01:07:44 +0000 |
| 52 | @@ -9,12 +9,13 @@ |
| 53 | |
| 54 | - 'index' : returns key index information |
| 55 | - 'get': returns an ASCII armored public key |
| 56 | - |
| 57 | -It does not depend on GPG; it simply serves the information stored in |
| 58 | -files at a given HOME (default to /home/keys/) with the following name |
| 59 | -format: |
| 60 | - |
| 61 | -0x<keyid>.<operation> |
| 62 | + - 'add': adds a key to the collection (does not update the index) |
| 63 | + |
| 64 | +It only depends on GPG for key submission; for retrieval and searching |
| 65 | +it just looks for files in the root (eg. /var/tmp/zeca). The files |
| 66 | +are named like this: |
| 67 | + |
| 68 | +0x<keyid|fingerprint>.<operation> |
| 69 | |
| 70 | Example: |
| 71 | |
| 72 | @@ -51,6 +52,33 @@ |
| 73 | GREETING = 'Copyright 2004-2008 Canonical Ltd.\n' |
| 74 | |
| 75 | |
| 76 | +def locate_key(root, suffix): |
| 77 | + """Find a key file in the root with the given suffix. |
| 78 | + |
| 79 | + This does some globbing to possibly find a fingerprint-named key |
| 80 | + file when given a key ID. |
| 81 | + |
| 82 | + :param root: The root directory in which to look. |
| 83 | + :param suffix: The key ID or fingerprint, of the form |
| 84 | + 0x<FINGERPRINT|KEYID>.<METHOD> |
| 85 | + :returns: An absolute path to the key file. |
| 86 | + """ |
| 87 | + path = os.path.join(root, suffix) |
| 88 | + |
| 89 | + if not os.path.exists(path): |
| 90 | + # GPG might request a key ID from us, but we name the keys by |
| 91 | + # fingerprint. Let's glob. |
| 92 | + if suffix.startswith('0x'): |
| 93 | + suffix = suffix[2:] |
| 94 | + keys = glob.glob(os.path.join(root, '*'+suffix)) |
| 95 | + if len(keys) == 1: |
| 96 | + path = keys[0] |
| 97 | + else: |
| 98 | + return None |
| 99 | + |
| 100 | + return path |
| 101 | + |
| 102 | + |
| 103 | class Zeca(Resource): |
| 104 | def getChild(self, name, request): |
| 105 | if name == '': |
| 106 | @@ -96,22 +124,11 @@ |
| 107 | |
| 108 | filename = '%s.%s' % (keyid, action) |
| 109 | |
| 110 | - try: |
| 111 | - path = os.path.join(self.root, filename) |
| 112 | - fp = open(path) |
| 113 | - except IOError: |
| 114 | - # GPG might request a key ID from us, but we name the keys by |
| 115 | - # fingerprint. Let's glob. |
| 116 | - if filename.startswith('0x'): |
| 117 | - filename = filename[2:] |
| 118 | - keys = glob.glob(os.path.join(self.root, '*'+filename)) |
| 119 | - if len(keys) == 1: |
| 120 | - content = open(keys[0]).read() |
| 121 | - else: |
| 122 | - content = 'Key Not Found' |
| 123 | + path = locate_key(self.root, filename) |
| 124 | + if path is not None: |
| 125 | + content = cgi.escape(open(path).read()) |
| 126 | else: |
| 127 | - content = cgi.escape(fp.read()) |
| 128 | - fp.close() |
| 129 | + content = 'Key Not Found' |
| 130 | |
| 131 | page += '<pre>\n%s\n</pre>\n</html>' % content |
| 132 |
On Wed, 2009-08-19 at 23:38 +0000, Celso Providelo wrote:
> Review: Needs Fixing code
> Willian,
>
> This change is definitely going in the right direction, thanks for proposing it!
>
> On the current diff I can only point the fact that, when found by glob, the key file content is not escaped, which will result in a broken HTML page.
Fixed. Oops.
> As we discussed on IRC, we could take the opportunity to extract the code used to locate key files and make this part of the application much nicer than it is, maybe plugging new key lookup mechanisms later.
>
> I'd suggest locate_key(root, term), 'root' is static, but it might be easier to pass it in. Doing that we can also cover most of its aspects with unittests.
I've extracted it into c.z.z.locate_key, with tests in
c.z.z.ftests.
I considered removing some of the tests from c.z.z.ftests.
they're short and relevant enough that it seems good to leave them
there.
> Looking forward to see the incremental patch.
Thanks for the review! The diff is attached.
--
William Grant