[ARVADOS] updated: 507110dc0aa1329ac4e5aad59c347a49e9f77364
git at public.curoverse.com
git at public.curoverse.com
Sat Aug 8 11:32:55 EDT 2015
Summary of changes:
sdk/pam/arvados_pam/__init__.py | 134 ++++++++++++++++++++--------------------
sdk/pam/setup.py | 1 +
sdk/pam/tests/test_pam.py | 62 +++++++++++--------
3 files changed, 106 insertions(+), 91 deletions(-)
via 507110dc0aa1329ac4e5aad59c347a49e9f77364 (commit)
via 1702335792308d7c1d578c143a5c99b943f112a9 (commit)
from 4fc613797f88dbb33c234ba7cd13965b1236bfee (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 507110dc0aa1329ac4e5aad59c347a49e9f77364
Author: Tom Clegg <tom at curoverse.com>
Date: Sat Aug 8 11:32:44 2015 -0400
6934: Improve and test logging
diff --git a/sdk/pam/arvados_pam/__init__.py b/sdk/pam/arvados_pam/__init__.py
index b7361f5..c002f3e 100644
--- a/sdk/pam/arvados_pam/__init__.py
+++ b/sdk/pam/arvados_pam/__init__.py
@@ -91,9 +91,7 @@ class AuthEvent(object):
else:
log_token = '<invalid>'
- log_label = [self.client_host, self.api_host, self.vm_uuid, self.username, log_token]
- if self.vm_uuid:
- log_label += [self.vm_uuid]
+ log_label = [self.service, self.api_host, self.vm_uuid, self.client_host, self.username, log_token]
if self.user:
log_label += [self.user.get('uuid'), self.user.get('full_name')]
return str(log_label) + ': ' + outcome
diff --git a/sdk/pam/tests/test_pam.py b/sdk/pam/tests/test_pam.py
index 1eb436a..70956c9 100644
--- a/sdk/pam/tests/test_pam.py
+++ b/sdk/pam/tests/test_pam.py
@@ -3,6 +3,7 @@
import arvados
import arvados_pam
import mock
+import re
import StringIO
import unittest
@@ -74,10 +75,19 @@ class AuthTest(unittest.TestCase):
filters=[['hostname','=',cfg['virtual_machine_hostname']]])
self.api.assert_called_with(
'v1', host=cfg['ARVADOS_API_HOST'], token=self.request['token'], cache=None)
+ self.assertEqual(1, len(self.syslogged))
+ for i in ['test_service',
+ self.request['username'],
+ self.config['test_service']['ARVADOS_API_HOST'],
+ self.response['virtual_machines']()['items'][0]['uuid']]:
+ self.assertRegexpMatches(self.syslogged[0], re.escape(i))
+ self.assertRegexpMatches(self.syslogged[0], re.escape(self.request['token'][0:15]), 'token prefix not logged')
+ self.assertNotRegexpMatches(self.syslogged[0], re.escape(self.request['token'][15:30]), 'too much token logged')
def test_fail_vm_lookup(self):
self.response['virtual_machines'] = self._raise
self.assertFalse(self.attempt())
+ self.assertRegexpMatches(self.syslogged[0], 'Test-induced failure')
def test_vm_hostname_not_found(self):
self.response['virtual_machines'] = lambda: {
@@ -156,5 +166,11 @@ class AuthTest(unittest.TestCase):
self.addCleanup(patcher.stop)
self.api.side_effect = [self.api_client]
+ self.syslogged = []
+ patcher = mock.patch('syslog.syslog')
+ self.syslog = patcher.start()
+ self.addCleanup(patcher.stop)
+ self.syslog.side_effect = lambda s: self.syslogged.append(s)
+
def _raise(self, exception=Exception("Test-induced failure"), *args, **kwargs):
raise exception
commit 1702335792308d7c1d578c143a5c99b943f112a9
Author: Tom Clegg <tom at curoverse.com>
Date: Sat Aug 8 11:17:38 2015 -0400
6934: Replace custom config with yaml. Simplify code path.
diff --git a/sdk/pam/arvados_pam/__init__.py b/sdk/pam/arvados_pam/__init__.py
index 4db6e58..b7361f5 100644
--- a/sdk/pam/arvados_pam/__init__.py
+++ b/sdk/pam/arvados_pam/__init__.py
@@ -4,6 +4,7 @@ sys.argv=['']
import arvados
import os
import syslog
+import yaml
def auth_log(msg):
"""Send errors to default auth log"""
@@ -12,108 +13,111 @@ def auth_log(msg):
syslog.closelog()
def config_file():
- return file('/etc/default/arvados_pam')
+ return file('/etc/default/arvados_pam.conf').read()
def config():
- txt = config_file().read()
- c = dict()
- for x in txt.splitlines(False):
- if not x.strip().startswith('#'):
- kv = x.split('=', 2)
- c[kv[0].strip()] = kv[1].strip()
- return c
+ return yaml.load(config_file())
class AuthEvent(object):
- def __init__(self, client_host, api_host, shell_host, username, token):
+ def __init__(self, config, service, client_host, username, token):
+ self.config = config
+ self.service = service
self.client_host = client_host
- self.api_host = api_host
- self.shell_hostname = shell_host
self.username = username
self.token = token
- self.vm = None
+
+ self.api_host = None
+ self.vm_uuid = None
self.user = None
def can_login(self):
+ """Return truthy IFF credentials should be accepted."""
ok = False
try:
+ self.api_host = self.config[self.service]['ARVADOS_API_HOST']
self.arv = arvados.api('v1', host=self.api_host, token=self.token, cache=None)
- self._lookup_vm()
- if self._check_login_permission():
- self.result = 'Authenticated'
- ok = True
- else:
- self.result = 'Denied'
+
+ vmname = self.config[self.service]['virtual_machine_hostname']
+ vms = self.arv.virtual_machines().list(filters=[['hostname','=',vmname]]).execute()
+ if vms['items_available'] > 1:
+ raise Exception("lookup hostname %s returned %d records" % (vmname, vms['items_available']))
+ if vms['items_available'] == 0:
+ raise Exception("lookup hostname %s not found" % vmname)
+ vm = vms['items'][0]
+ if vm['hostname'] != vmname:
+ raise Exception("lookup hostname %s returned hostname %s" % (vmname, vm['hostname']))
+ self.vm_uuid = vm['uuid']
+
+ self.user = self.arv.users().current().execute()
+
+ filters = [
+ ['link_class','=','permission'],
+ ['name','=','can_login'],
+ ['head_uuid','=',self.vm_uuid],
+ ['tail_uuid','=',self.user['uuid']]]
+ for l in self.arv.links().list(filters=filters, limit=10000).execute()['items']:
+ if (l['properties']['username'] == self.username and
+ l['tail_uuid'] == self.user['uuid'] and
+ l['head_uuid'] == self.vm_uuid and
+ l['link_class'] == 'permission' and
+ l['name'] == 'can_login'):
+ return self._report(True)
+
+ return self._report(False)
+
except Exception as e:
- self.result = 'Error: ' + repr(e)
+ return self._report(e)
+
+ def _report(self, result):
+ """Log the result. Return truthy IFF result is True.
+
+ result must be True, False, or an exception.
+ """
+ self.result = result
auth_log(self.message())
- return ok
-
- def _lookup_vm(self):
- """Load the VM record for this host into self.vm. Raise if not possible."""
-
- vms = self.arv.virtual_machines().list(filters=[['hostname','=',self.shell_hostname]]).execute()
- if vms['items_available'] > 1:
- raise Exception("ambiguous VM hostname matched %d records" % vms['items_available'])
- if vms['items_available'] == 0:
- raise Exception("VM hostname not found")
- self.vm = vms['items'][0]
- if self.vm['hostname'] != self.shell_hostname:
- raise Exception("API returned record with wrong hostname")
-
- def _check_login_permission(self):
- """Check permission to log in. Return True if permission is granted."""
- self._lookup_vm()
- self.user = self.arv.users().current().execute()
- filters = [
- ['link_class','=','permission'],
- ['name','=','can_login'],
- ['head_uuid','=',self.vm['uuid']],
- ['tail_uuid','=',self.user['uuid']]]
- for l in self.arv.links().list(filters=filters, limit=10000).execute()['items']:
- if (l['properties']['username'] == self.username and
- l['tail_uuid'] == self.user['uuid'] and
- l['head_uuid'] == self.vm['uuid'] and
- l['link_class'] == 'permission' and
- l['name'] == 'can_login'):
- return True
- return False
+ return result == True
def message(self):
+ """Return a log message describing the event and its outcome."""
+ if isinstance(self.result, Exception):
+ outcome = 'Error: ' + repr(self.result)
+ elif self.result == True:
+ outcome = 'Allow'
+ else:
+ outcome = 'Deny'
+
if len(self.token) > 40:
log_token = self.token[0:15]
else:
log_token = '<invalid>'
- log_label = [self.client_host, self.api_host, self.shell_hostname, self.username, log_token]
- if self.vm:
- log_label += [self.vm.get('uuid')]
+
+ log_label = [self.client_host, self.api_host, self.vm_uuid, self.username, log_token]
+ if self.vm_uuid:
+ log_label += [self.vm_uuid]
if self.user:
log_label += [self.user.get('uuid'), self.user.get('full_name')]
- return str(log_label) + ': ' + self.result
+ return str(log_label) + ': ' + outcome
def pam_sm_authenticate(pamh, flags, argv):
try:
- user = pamh.get_user()
+ username = pamh.get_user()
except pamh.exception as e:
return e.pam_result
- if not user:
+ if not username:
return pamh.PAM_USER_UNKNOWN
try:
- resp = pamh.conversation(pamh.Message(pamh.PAM_PROMPT_ECHO_OFF, ''))
+ token = pamh.conversation(pamh.Message(pamh.PAM_PROMPT_ECHO_OFF, '')).resp
except pamh.exception as e:
return e.pam_result
- try:
- config = config()
- api_host = config['ARVADOS_API_HOST'].strip()
- shell_host = config['HOSTNAME'].strip()
- except Exception as e:
- auth_log("loading config: " + repr(e))
- return False
-
- if AuthEvent(pamh.rhost, api_host, shell_host, user, resp.resp).can_login():
+ if AuthEvent(config(),
+ service=pamh.service,
+ client_host=pamh.rhost,
+ username=username,
+ token=token).can_login():
return pamh.PAM_SUCCESS
else:
return pamh.PAM_AUTH_ERR
diff --git a/sdk/pam/setup.py b/sdk/pam/setup.py
index ed47388..4942e2d 100644
--- a/sdk/pam/setup.py
+++ b/sdk/pam/setup.py
@@ -31,6 +31,7 @@ setup(name='arvados-pam',
],
install_requires=[
'arvados-python-client>=0.1.20150801000000',
+ 'pyyaml',
],
test_suite='tests',
tests_require=['mock>=1.0', 'PyYAML'],
diff --git a/sdk/pam/tests/test_pam.py b/sdk/pam/tests/test_pam.py
index f59a4fd..1eb436a 100644
--- a/sdk/pam/tests/test_pam.py
+++ b/sdk/pam/tests/test_pam.py
@@ -9,37 +9,30 @@ import unittest
class ConfigTest(unittest.TestCase):
def test_ok_config(self):
self.assertConfig(
- "#comment\nARVADOS_API_HOST=xyzzy.example\nHOSTNAME=foo.shell\n#HOSTNAME=bogus\n",
- 'xyzzy.example',
- 'foo.shell')
-
- def test_config_missing_apihost(self):
- with self.assertRaises(KeyError):
- self.assertConfig('HOSTNAME=foo', '', 'foo')
-
- def test_config_missing_shellhost(self):
- with self.assertRaises(KeyError):
- self.assertConfig('ARVADOS_API_HOST=foo', 'foo', '')
-
- def test_config_empty_shellhost(self):
- self.assertConfig("ARVADOS_API_HOST=foo\nHOSTNAME=\n", 'foo', '')
-
- def test_config_strip_whitespace(self):
- self.assertConfig(" ARVADOS_API_HOST = foo \n\tHOSTNAME\t=\tbar\t\n", 'foo', 'bar')
+ """servicename:
+ ARVADOS_API_HOST: xyzzy.example
+ virtual_machine_hostname: foo.shell
+ """, 'servicename', 'xyzzy.example', 'foo.shell')
@mock.patch('arvados_pam.config_file')
- def assertConfig(self, txt, apihost, shellhost, config_file):
+ def assertConfig(self, txt, svcname, apihost, shellhost, config_file):
configfake = StringIO.StringIO(txt)
config_file.side_effect = [configfake]
c = arvados_pam.config()
- self.assertEqual(apihost, c['ARVADOS_API_HOST'])
- self.assertEqual(shellhost, c['HOSTNAME'])
+ self.assertEqual(apihost, c[svcname]['ARVADOS_API_HOST'])
+ self.assertEqual(shellhost, c[svcname]['virtual_machine_hostname'])
class AuthTest(unittest.TestCase):
+ default_config = {
+ 'test_service': {
+ 'ARVADOS_API_HOST': 'zzzzz.api_host.example',
+ 'virtual_machine_hostname': 'testvm2.shell',
+ }
+ }
+
default_request = {
- 'api_host': 'zzzzz.api_host.example',
- 'shell_host': 'testvm2.shell',
+ 'client_host': '::1',
'token': '3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi',
'username': 'active',
}
@@ -71,14 +64,16 @@ class AuthTest(unittest.TestCase):
}
def attempt(self):
- return arvados_pam.AuthEvent('::1', **self.request).can_login()
+ return arvados_pam.AuthEvent(config=self.config, service='test_service', **self.request).can_login()
def test_success(self):
self.assertTrue(self.attempt())
+
+ cfg = self.config['test_service']
self.api_client.virtual_machines().list.assert_called_with(
- filters=[['hostname','=',self.request['shell_host']]])
+ filters=[['hostname','=',cfg['virtual_machine_hostname']]])
self.api.assert_called_with(
- 'v1', host=self.request['api_host'], token=self.request['token'], cache=None)
+ 'v1', host=cfg['ARVADOS_API_HOST'], token=self.request['token'], cache=None)
def test_fail_vm_lookup(self):
self.response['virtual_machines'] = self._raise
@@ -149,6 +144,7 @@ class AuthTest(unittest.TestCase):
self.assertFalse(self.attempt())
def setUp(self):
+ self.config = self.default_config.copy()
self.request = self.default_request.copy()
self.response = self.default_response.copy()
self.api_client = mock.MagicMock(name='api_client')
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list