summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDan Callaghan <dcallagh@redhat.com>2013-12-19 15:06:28 +1000
committerGerrit Code Review <gerrit@beaker-project.org>2013-12-24 03:28:28 +0000
commitc6101de1f657b3127f55e69674305984a9414e23 (patch)
treeb65af59b1c9a067bb9b58fe7cc0eacdf9fb9ef66
parent9d893fe2366ee92fb4aad56322984cfa52e205d8 (diff)
"view" permission in system access policy
-rw-r--r--IntegrationTests/src/bkr/inttest/client/test_loan_management.py5
-rw-r--r--IntegrationTests/src/bkr/inttest/client/test_policy_list.py8
-rw-r--r--IntegrationTests/src/bkr/inttest/data_setup.py5
-rw-r--r--IntegrationTests/src/bkr/inttest/server/selenium/test_add_system.py28
-rw-r--r--IntegrationTests/src/bkr/inttest/server/selenium/test_csv_export.py3
-rw-r--r--IntegrationTests/src/bkr/inttest/server/selenium/test_csv_import.py47
-rw-r--r--IntegrationTests/src/bkr/inttest/server/selenium/test_system_access_policies.py22
-rw-r--r--IntegrationTests/src/bkr/inttest/server/selenium/test_system_search.py6
-rw-r--r--IntegrationTests/src/bkr/inttest/server/test_lab_controller.py3
-rw-r--r--Server/assets/jst/access-policy.html8
-rw-r--r--Server/bkr/server/CSV_import_export.py37
-rw-r--r--Server/bkr/server/controllers.py21
-rw-r--r--Server/bkr/server/model/inventory.py8
-rw-r--r--Server/bkr/server/model/types.py1
-rw-r--r--Server/bkr/server/reporting-queries/machine-hours-by-user-arch.sql10
-rw-r--r--Server/bkr/server/reporting-queries/recipe-hours-by-user-arch.sql10
-rw-r--r--Server/bkr/server/templates/system_form.kid6
-rw-r--r--Server/bkr/server/widgets.py3
-rw-r--r--documentation/user-guide/systems/sharing.rst7
-rw-r--r--documentation/whats-new/next/system-view-permission.rst54
20 files changed, 207 insertions, 85 deletions
diff --git a/IntegrationTests/src/bkr/inttest/client/test_loan_management.py b/IntegrationTests/src/bkr/inttest/client/test_loan_management.py
index 22b59cf..8b679b9 100644
--- a/IntegrationTests/src/bkr/inttest/client/test_loan_management.py
+++ b/IntegrationTests/src/bkr/inttest/client/test_loan_management.py
@@ -163,9 +163,8 @@ class SystemLoanTest(unittest.TestCase):
def test_user_with_borrow_permissions_only(self):
# Grant everyone borrow permissions on the system
with session.begin():
- policy = self.system.custom_access_policy = SystemAccessPolicy()
- policy.add_rule(permission=SystemPermission.loan_self,
- everybody=True)
+ self.system.custom_access_policy.add_rule(
+ permission=SystemPermission.loan_self, everybody=True)
# user2 should now be able to borrow and return the system
out = run_client(['bkr', 'loan-grant', self.system.fqdn],
diff --git a/IntegrationTests/src/bkr/inttest/client/test_policy_list.py b/IntegrationTests/src/bkr/inttest/client/test_policy_list.py
index 93d9684..8ae9df1 100644
--- a/IntegrationTests/src/bkr/inttest/client/test_policy_list.py
+++ b/IntegrationTests/src/bkr/inttest/client/test_policy_list.py
@@ -39,14 +39,16 @@ class PolicyListTest(unittest.TestCase):
expected_output = self.gen_expected_pretty_table(
(['control_system', 'X', self.group.group_name, 'No'],
['control_system', self.user2.user_name, 'X', 'No'],
- ['edit_system', self.user1.user_name, 'X', 'No'])) + '\n'
+ ['edit_system', self.user1.user_name, 'X', 'No'],
+ ['view', 'X', 'X', 'Yes'])) + '\n'
self.assertEquals(out, expected_output)
# For the second system
out = run_client(['bkr', 'policy-list', self.system_public.fqdn,
'--format','tabular'])
expected_output = self.gen_expected_pretty_table(
- (['control_system', 'X', 'X', 'Yes'],)) + '\n'
+ (['control_system', 'X', 'X', 'Yes'],
+ ['view', 'X', 'X', 'Yes'])) + '\n'
self.assertEquals(out, expected_output)
@@ -54,7 +56,7 @@ class PolicyListTest(unittest.TestCase):
out = run_client(['bkr', 'policy-list', self.system.fqdn,
'--format','json'])
out = json.loads(out)
- self.assertEquals(len(out['rules']), 3)
+ self.assertEquals(len(out['rules']), 4)
def test_list_policy_non_existent_system(self):
diff --git a/IntegrationTests/src/bkr/inttest/data_setup.py b/IntegrationTests/src/bkr/inttest/data_setup.py
index ff3867f..9d3a5b9 100644
--- a/IntegrationTests/src/bkr/inttest/data_setup.py
+++ b/IntegrationTests/src/bkr/inttest/data_setup.py
@@ -210,7 +210,7 @@ def create_distro_tree(distro=None, distro_name=None, osmajor=u'DansAwesomeLinux
def create_system(arch=u'i386', type=SystemType.machine, status=SystemStatus.automated,
owner=None, fqdn=None, shared=True, exclude_osmajor=[],
exclude_osversion=[], hypervisor=None, kernel_type=None,
- date_added=None, **kw):
+ date_added=None, private=False, **kw):
if owner is None:
owner = create_user()
if fqdn is None:
@@ -221,8 +221,9 @@ def create_system(arch=u'i386', type=SystemType.machine, status=SystemStatus.aut
status=status, **kw)
if date_added is not None:
system.date_added = date_added
- # Always give it a custom policy, so that tests can fill in rules they need.
system.custom_access_policy = SystemAccessPolicy()
+ if not private:
+ system.custom_access_policy.add_rule(SystemPermission.view, everybody=True)
if shared:
system.custom_access_policy.add_rule(
permission=SystemPermission.reserve, everybody=True)
diff --git a/IntegrationTests/src/bkr/inttest/server/selenium/test_add_system.py b/IntegrationTests/src/bkr/inttest/server/selenium/test_add_system.py
index 75c3ab4..623e866 100644
--- a/IntegrationTests/src/bkr/inttest/server/selenium/test_add_system.py
+++ b/IntegrationTests/src/bkr/inttest/server/selenium/test_add_system.py
@@ -1,6 +1,6 @@
from selenium.webdriver.support.ui import Select
-from bkr.server.model import session, System
+from bkr.server.model import session, System, SystemPermission
from bkr.inttest.server.selenium import WebDriverTestCase
from bkr.inttest.server.webdriver_utils import login
from bkr.inttest import data_setup, get_server_base
@@ -18,14 +18,12 @@ class AddSystem(WebDriverTestCase):
# the default values are the same as that presented by the Web UI
def add_system(self, fqdn='', lender='', serial='', status='Automated',
- lab_controller='None', type='Laptop', private=False,
+ lab_controller='None', type='Laptop',
vendor='', model='', location='', mac=''):
b = self.browser
b.find_element_by_name('fqdn').send_keys(fqdn)
b.find_element_by_name('lender').send_keys(lender)
Select(b.find_element_by_name('status')).select_by_visible_text(status)
- if private:
- b.find_element_by_name('private').click()
if status == 'Broken':
b.find_element_by_name('status_reason').send_keys(self.condition_report)
Select(b.find_element_by_name('lab_controller_id'))\
@@ -50,7 +48,6 @@ class AddSystem(WebDriverTestCase):
status = 'Automated',
lab_controller = 'lab-devel.rhts.eng.bos.redhat.com',
type = 'Machine',
- private = False,
vendor = 'Intel',
model = 'model',
location = 'brisbane',
@@ -65,7 +62,6 @@ class AddSystem(WebDriverTestCase):
self.assert_system_view_text('lender', system_details['lender'])
self.assert_system_view_text('model', system_details['model'])
self.assert_system_view_text('location', system_details['location'])
- self.assert_system_view_text('private', 'False')
with session.begin():
system = System.query.filter_by(fqdn=system_details['fqdn']).one()
self.assertEquals(unicode(system.status), system_details['status'])
@@ -77,7 +73,6 @@ class AddSystem(WebDriverTestCase):
status = 'Broken',
lab_controller = 'None',
type = 'Laptop',
- private = False,
vendor = 'Intel',
model = 'model',
location = 'brisbane',
@@ -94,7 +89,6 @@ class AddSystem(WebDriverTestCase):
self.assert_system_view_text('status_reason', self.condition_report)
self.assert_system_view_text('model', system_details['model'])
self.assert_system_view_text('location', system_details['location'])
- self.assert_system_view_text('private', 'False')
def test_case_3(self):
system_details = dict(fqdn = 'test-system-3',
@@ -103,7 +97,6 @@ class AddSystem(WebDriverTestCase):
status = 'Automated',
lab_controller = 'None',
type = 'Resource',
- private = True,
vendor = 'AMD',
model = 'model',
location = '',
@@ -119,7 +112,6 @@ class AddSystem(WebDriverTestCase):
self.assert_system_view_text('vendor', system_details['vendor'])
self.assert_system_view_text('model', system_details['model'])
self.assert_system_view_text('location', system_details['location'])
- self.assert_system_view_text('private', 'True')
def test_case_4(self):
system_details = dict(fqdn = 'test-system-4',
@@ -128,7 +120,6 @@ class AddSystem(WebDriverTestCase):
status = 'Broken',
lab_controller = 'None',
type = 'Prototype',
- private = True,
vendor = 'AMD',
model = 'model',
location = 'brisbane',
@@ -144,7 +135,6 @@ class AddSystem(WebDriverTestCase):
self.assert_system_view_text('vendor', system_details['vendor'])
self.assert_system_view_text('model', system_details['model'])
self.assert_system_view_text('location', system_details['location'])
- self.assert_system_view_text('private', 'True')
def test_case_5(self):
with session.begin():
@@ -155,7 +145,6 @@ class AddSystem(WebDriverTestCase):
status = 'Broken',
lab_controller = 'None',
type = 'Prototype',
- private = True,
vendor = 'AMD',
model = 'model',
location = 'brisbane',
@@ -178,3 +167,16 @@ class AddSystem(WebDriverTestCase):
error_msg = b.find_element_by_css_selector(
'.control-group.error .help-inline').text
self.assertEquals(error_msg, 'Please enter a value')
+
+ def test_grants_view_permission_to_everybody_by_default(self):
+ fqdn = data_setup.unique_name(u'test-add-system%s.example.invalid')
+ b = self.browser
+ b.get(get_server_base())
+ b.find_element_by_link_text('Add').click()
+ b.find_element_by_name('fqdn').send_keys(fqdn)
+ b.find_element_by_name('form').submit()
+ b.find_element_by_xpath('//h1[text()="%s"]' % fqdn)
+ with session.begin():
+ system = System.query.filter(System.fqdn == fqdn).one()
+ self.assertTrue(system.custom_access_policy.grants_everybody(
+ SystemPermission.view))
diff --git a/IntegrationTests/src/bkr/inttest/server/selenium/test_csv_export.py b/IntegrationTests/src/bkr/inttest/server/selenium/test_csv_export.py
index c33489d..df5ba37 100644
--- a/IntegrationTests/src/bkr/inttest/server/selenium/test_csv_export.py
+++ b/IntegrationTests/src/bkr/inttest/server/selenium/test_csv_export.py
@@ -127,8 +127,7 @@ class CSVExportTest(WebDriverTestCase):
def test_export_systems_obeys_secrecy(self):
with session.begin():
unprivileged_user = data_setup.create_user(password=u'asdf')
- secret_system = data_setup.create_system()
- secret_system.private = True
+ secret_system = data_setup.create_system(private=True)
login(self.browser, user=unprivileged_user.user_name, password=u'asdf')
csv_request = self.get_csv('system')
self.assert_(not any(row['fqdn'] == secret_system.fqdn
diff --git a/IntegrationTests/src/bkr/inttest/server/selenium/test_csv_import.py b/IntegrationTests/src/bkr/inttest/server/selenium/test_csv_import.py
index 617e0df..83fa291 100644
--- a/IntegrationTests/src/bkr/inttest/server/selenium/test_csv_import.py
+++ b/IntegrationTests/src/bkr/inttest/server/selenium/test_csv_import.py
@@ -5,7 +5,7 @@ from bkr.inttest.server.selenium import WebDriverTestCase
from bkr.inttest.server.webdriver_utils import login, logout, is_text_present
from bkr.inttest import data_setup, get_server_base, with_transaction
from bkr.inttest.assertions import assert_has_key_with_value
-from bkr.server.model import Arch, System, OSMajor
+from bkr.server.model import Arch, System, OSMajor, SystemPermission
from turbogears.database import session
import pkg_resources
import unittest
@@ -23,7 +23,6 @@ class CSVImportTest(WebDriverTestCase):
def import_csv(self, contents):
b = self.browser
- login(b)
b.get(get_server_base() + 'csv/csv_import')
csv_file = NamedTemporaryFile(prefix=self.__module__)
csv_file.write(contents)
@@ -32,6 +31,7 @@ class CSVImportTest(WebDriverTestCase):
b.find_element_by_name('csv_file').submit()
def test_system(self):
+ login(self.browser)
orig_date_modified = self.system.date_modified
self.import_csv((u'csv_type,fqdn,location,arch\n'
u'system,%s,Under my desk,ia64' % self.system.fqdn)
@@ -44,21 +44,57 @@ class CSVImportTest(WebDriverTestCase):
self.assert_(self.system.date_modified > orig_date_modified)
# attempting to import a system with no FQDN should fail
- logout(self.browser)
self.import_csv((u'csv_type,fqdn,location,arch\n'
u'system,'',Under my desk,ia64').encode('utf8'))
self.assertTrue(is_text_present(self.browser,
"Error importing system on line 2: "
"System must have an associated FQDN"))
# attempting to import a system with an invalid FQDN should fail
- logout(self.browser)
self.import_csv((u'csv_type,fqdn,location,arch\n'
u'system,invalid--fqdn,Under my desk,ia64').encode('utf8'))
self.assertTrue(is_text_present(self.browser,
"Error importing system on line 2: "
"System has an invalid FQDN: invalid--fqdn"))
+ def test_grants_view_permission_to_everybody_by_default(self):
+ fqdn = data_setup.unique_name(u'test-csv-import%s.example.invalid')
+ b = self.browser
+ login(b)
+ self.import_csv((u'csv_type,fqdn\n'
+ u'system,%s' % fqdn).encode('utf8'))
+ self.assertEquals(self.browser.find_element_by_xpath(
+ '//table[@id="csv-import-log"]//td').text,
+ 'No Errors')
+ with session.begin():
+ system = System.query.filter(System.fqdn == fqdn).one()
+ self.assertTrue(system.custom_access_policy.grants_everybody(
+ SystemPermission.view))
+
+ def test_system_secret_field(self):
+ login(self.browser)
+ self.import_csv((u'csv_type,fqdn,secret\n'
+ u'system,%s,True' % self.system.fqdn)
+ .encode('utf8'))
+ self.assertEquals(self.browser.find_element_by_xpath(
+ '//table[@id="csv-import-log"]//td').text,
+ 'No Errors')
+ with session.begin():
+ session.refresh(self.system.custom_access_policy)
+ self.assertFalse(self.system.custom_access_policy.grants_everybody(
+ SystemPermission.view))
+ self.import_csv((u'csv_type,fqdn,secret\n'
+ u'system,%s,False' % self.system.fqdn)
+ .encode('utf8'))
+ self.assertEquals(self.browser.find_element_by_xpath(
+ '//table[@id="csv-import-log"]//td').text,
+ 'No Errors')
+ with session.begin():
+ session.refresh(self.system.custom_access_policy)
+ self.assertTrue(self.system.custom_access_policy.grants_everybody(
+ SystemPermission.view))
+
def test_keyvalue(self):
+ login(self.browser)
orig_date_modified = self.system.date_modified
self.import_csv((u'csv_type,fqdn,key,key_value,deleted\n'
u'keyvalue,%s,COMMENT,UTF 8 –,False' % self.system.fqdn)
@@ -104,6 +140,7 @@ class CSVImportTest(WebDriverTestCase):
'!LAYER2 !DNS !PORTNO !IPADDR !GATEWAY !HOSTNAME !NETMASK ')
def test_missing_field(self):
+ login(self.browser)
orig_date_modified = self.system.date_modified
self.import_csv((u'csv_type,fqdn,location,arch\n'
u'system,%s,Under my desk' % self.system.fqdn)
@@ -111,6 +148,7 @@ class CSVImportTest(WebDriverTestCase):
self.assert_(is_text_present(self.browser, 'Missing fields on line 2: arch'))
def test_extraneous_field(self):
+ login(self.browser)
orig_date_modified = self.system.date_modified
self.import_csv((u'csv_type,fqdn,location,arch\n'
u'system,%s,Under my desk,ppc64,what is this field doing here' % self.system.fqdn)
@@ -119,6 +157,7 @@ class CSVImportTest(WebDriverTestCase):
# https://bugzilla.redhat.com/show_bug.cgi?id=972411
def test_malformed(self):
+ login(self.browser)
self.import_csv('gar\x00bage')
self.assertEquals(self.browser.find_element_by_xpath(
'//table[@id="csv-import-log"]//td').text,
diff --git a/IntegrationTests/src/bkr/inttest/server/selenium/test_system_access_policies.py b/IntegrationTests/src/bkr/inttest/server/selenium/test_system_access_policies.py
index 13d1ec0..541d84c 100644
--- a/IntegrationTests/src/bkr/inttest/server/selenium/test_system_access_policies.py
+++ b/IntegrationTests/src/bkr/inttest/server/selenium/test_system_access_policies.py
@@ -176,12 +176,14 @@ class SystemAccessPolicyHTTPTest(unittest.TestCase):
json = response.json()
self.assertEquals(json['id'], self.policy.id)
self.assertEquals([p['value'] for p in json['possible_permissions']],
- ['edit_policy', 'edit_system', 'loan_any', 'loan_self',
+ ['view', 'edit_policy', 'edit_system', 'loan_any', 'loan_self',
'control_system', 'reserve'])
self.assertItemsEqual(json['rules'], [
- {'id': self.policy.rules[0].id, 'permission': 'reserve',
+ {'id': self.policy.rules[0].id, 'permission': 'view',
'everybody': True, 'user': None, 'group': None},
- {'id': self.policy.rules[1].id, 'permission': 'edit_system',
+ {'id': self.policy.rules[1].id, 'permission': 'reserve',
+ 'everybody': True, 'user': None, 'group': None},
+ {'id': self.policy.rules[2].id, 'permission': 'edit_system',
'everybody': False, 'user': None,
'group': self.privileged_group.group_name},
])
@@ -214,8 +216,10 @@ class SystemAccessPolicyHTTPTest(unittest.TestCase):
response = put_json(get_server_base() +
'systems/%s/access-policy' % self.system.fqdn, session=s,
data={'rules': [
- # keep one existing rule, drop the other
- {'id': self.policy.rules[1].id, 'permission': 'edit_system',
+ # keep two existing rules, drop the other
+ {'id': self.policy.rules[0].id, 'permission': 'view',
+ 'everybody': True, 'user': None, 'group': None},
+ {'id': self.policy.rules[2].id, 'permission': 'edit_system',
'user': None, 'group': self.privileged_group.group_name},
# .. and add a new rule
{'permission': 'control_system', 'everybody': True,
@@ -224,9 +228,11 @@ class SystemAccessPolicyHTTPTest(unittest.TestCase):
response.raise_for_status()
with session.begin():
session.expire_all()
- self.assertEquals(len(self.policy.rules), 2)
+ self.assertEquals(len(self.policy.rules), 3)
self.assertEquals(self.policy.rules[0].permission,
- SystemPermission.edit_system)
+ SystemPermission.view)
self.assertEquals(self.policy.rules[1].permission,
+ SystemPermission.edit_system)
+ self.assertEquals(self.policy.rules[2].permission,
SystemPermission.control_system)
- self.assertEquals(self.policy.rules[1].everybody, True)
+ self.assertEquals(self.policy.rules[2].everybody, True)
diff --git a/IntegrationTests/src/bkr/inttest/server/selenium/test_system_search.py b/IntegrationTests/src/bkr/inttest/server/selenium/test_system_search.py
index d59c196..a8dc9f6 100644
--- a/IntegrationTests/src/bkr/inttest/server/selenium/test_system_search.py
+++ b/IntegrationTests/src/bkr/inttest/server/selenium/test_system_search.py
@@ -475,8 +475,7 @@ class SystemVisibilityTest(WebDriverTestCase):
def test_secret_system_not_visible(self):
with session.begin():
- secret_system = data_setup.create_system()
- secret_system.private = True
+ secret_system = data_setup.create_system(private=True)
b = self.browser
login(b, user=self.user.user_name, password=u'password')
b.get(get_server_base())
@@ -487,8 +486,7 @@ class SystemVisibilityTest(WebDriverTestCase):
# https://bugzilla.redhat.com/show_bug.cgi?id=582008
def test_secret_system_visible_when_loaned(self):
with session.begin():
- secret_system = data_setup.create_system()
- secret_system.private = True
+ secret_system = data_setup.create_system(private=True)
secret_system.loaned = self.user
b = self.browser
login(b, user=self.user.user_name, password=u'password')
diff --git a/IntegrationTests/src/bkr/inttest/server/test_lab_controller.py b/IntegrationTests/src/bkr/inttest/server/test_lab_controller.py
index 1038ca6..a4e129f 100644
--- a/IntegrationTests/src/bkr/inttest/server/test_lab_controller.py
+++ b/IntegrationTests/src/bkr/inttest/server/test_lab_controller.py
@@ -17,8 +17,7 @@ class TestLabController(unittest.TestCase):
def test_lookup_secret_fqdn(self):
with session.begin():
- system = data_setup.create_system()
- system.private = True
+ system = data_setup.create_system(private=True)
lab_controller_user = LabController.by_name(self.lc_fqdn).user
system2 = System.by_fqdn(str(system.fqdn), user=lab_controller_user)
self.assertEquals(system, system2)
diff --git a/Server/assets/jst/access-policy.html b/Server/assets/jst/access-policy.html
index f3e82ff..418d7f6 100644
--- a/Server/assets/jst/access-policy.html
+++ b/Server/assets/jst/access-policy.html
@@ -15,10 +15,9 @@
<!-- group rows go here -->
<% if (!readonly) { %>
<tr>
- <td>
+ <td colspan="<%- possible_permissions.length + 1 %>">
<input type="text" placeholder="Group name" />
</td>
- <td colspan="<%- possible_permissions.length %>"></td>
</tr>
<% } %>
</tbody>
@@ -29,8 +28,9 @@
<!-- user rows go here -->
<% if (!readonly) { %>
<tr>
- <td><input type="text" placeholder="Username" /></td>
- <td colspan="<%- possible_permissions.length %>"></td>
+ <td colspan="<%- possible_permissions.length + 1 %>">
+ <input type="text" placeholder="Username" />
+ </td>
</tr>
<% } %>
</tbody>
diff --git a/Server/bkr/server/CSV_import_export.py b/Server/bkr/server/CSV_import_export.py
index 74b36f1..11a76d4 100644
--- a/Server/bkr/server/CSV_import_export.py
+++ b/Server/bkr/server/CSV_import_export.py
@@ -13,7 +13,8 @@ from bkr.server.model import (System, SystemType, Activity, SystemActivity,
SystemStatus, Power, PowerType, Arch,
Provision, ProvisionFamily,
ProvisionFamilyUpdate,
- Key, Key_Value_Int, Key_Value_String)
+ Key, Key_Value_Int, Key_Value_String,
+ SystemAccessPolicy, SystemPermission)
from bkr.server.widgets import HorizontalForm, RadioButtonList
import csv
import datetime
@@ -133,7 +134,10 @@ class CSV(RPCRoot):
log.append('Error importing system on line %s: %s' %
(reader.line_num, str(e)))
continue
-
+ # new systems are visible to everybody by default
+ system.custom_access_policy = SystemAccessPolicy()
+ system.custom_access_policy.add_rule(
+ SystemPermission.view, everybody=True)
if system.can_edit(identity.current.user):
# Remove fqdn, can't change that via csv.
data.pop('fqdn')
@@ -364,14 +368,32 @@ class CSV_System(CSV):
system.type = systemtype
# import secret
if 'secret' in data:
+ # 'private' used to be a field on system (called 'secret' in the UI
+ # and CSV). The field is replaced by the 'view' permission in the
+ # access policy so CSV export no longer produces 'secret' in its
+ # output. However we still accept it on import for compatibility.
+ # It is mapped to the 'view everybody' rule in the access policy.
if not data['secret']:
log.append("%s: Invalid secret None" % system.fqdn)
return False
- newdata = smart_bool(data['secret'])
- if system.private != newdata:
- activity = SystemActivity(identity.current.user, 'CSV', 'Changed', 'secret', '%s' % system.private, '%s' % newdata)
- system.activity.append(activity)
- system.private = newdata
+ secret = smart_bool(data['secret'])
+ view_everybody = system.custom_access_policy.grants_everybody(
+ SystemPermission.view)
+ if secret and view_everybody:
+ # remove 'view everybody' rule
+ for rule in system.custom_access_policy.rules:
+ if rule.permission == SystemPermission.view and rule.everybody:
+ system.record_activity(user=identity.current.user, service=u'HTTP',
+ field=u'Access Policy Rule', action=u'Removed',
+ old=repr(rule))
+ session.delete(rule)
+ if not secret and not view_everybody:
+ # add rule granting 'view everybody'
+ new_rule = system.custom_access_policy.add_rule(everybody=True,
+ permission=SystemPermission.view)
+ system.record_activity(user=identity.current.user, service=u'HTTP',
+ field=u'Access Policy Rule', action=u'Added',
+ new=repr(new_rule))
return True
def __init__(self, system):
@@ -386,7 +408,6 @@ class CSV_System(CSV):
self.memory = system.memory
self.model = system.model
self.owner = system.owner
- self.secret = system.private
self.serial = system.serial
self.status = system.status
self.type = system.type
diff --git a/Server/bkr/server/controllers.py b/Server/bkr/server/controllers.py
index c6cba11..483c9a2 100644
--- a/Server/bkr/server/controllers.py
+++ b/Server/bkr/server/controllers.py
@@ -15,7 +15,8 @@ from bkr.server.model import (TaskBase, Device, System, SystemGroup,
ExcludeOSVersion, OSVersion,
Provision, ProvisionFamily,
ProvisionFamilyUpdate, SystemStatus,
- Key_Value_Int, Key_Value_String)
+ Key_Value_Int, Key_Value_String,
+ SystemAccessPolicy, SystemPermission)
from bkr.server.power import PowerTypes
from bkr.server.keytypes import KeyTypes
from bkr.server.CSV_import_export import CSV
@@ -1271,6 +1272,10 @@ class Root(RPCRoot):
flash( _(u"%s already exists!" % kw['fqdn']) )
redirect("/")
system = System(fqdn=kw['fqdn'],owner=identity.current.user)
+ # new systems are visible to everybody by default
+ system.custom_access_policy = SystemAccessPolicy()
+ system.custom_access_policy.add_rule(SystemPermission.view,
+ everybody=True)
if kw['lab_controller_id'] == 0:
kw['lab_controller'] = None
@@ -1314,16 +1319,6 @@ class Root(RPCRoot):
activity = SystemActivity(identity.current.user, u'WEBUI', u'Changed', unicode(field), current_val, new_val)
system.activity.append(activity)
- log_bool_fields = [ 'private' ]
- for field in log_bool_fields:
- try:
- current_val = unicode(getattr(system,field) and True or False)
- except KeyError:
- current_val = u""
- new_val = unicode(kw.get(field) or False)
- if current_val != new_val:
- activity = SystemActivity(identity.current.user, u'WEBUI', u'Changed', unicode(field), current_val, new_val )
- system.activity.append(activity)
system.status = kw['status']
system.location=kw['location']
system.model=kw['model']
@@ -1336,10 +1331,6 @@ class Root(RPCRoot):
system.status_reason = kw['status_reason']
system.date_modified = datetime.utcnow()
system.kernel_type = kw['kernel_type']
- if kw.get('private'):
- system.private=kw['private']
- else:
- system.private=False
# Change Lab Controller
system.lab_controller = kw['lab_controller']
diff --git a/Server/bkr/server/model/inventory.py b/Server/bkr/server/model/inventory.py
index cbe19c4..ce5fece 100644
--- a/Server/bkr/server/model/inventory.py
+++ b/Server/bkr/server/model/inventory.py
@@ -70,7 +70,6 @@ system_table = Table('system', metadata,
Column('type', SystemType.db_type(), nullable=False),
Column('status', SystemStatus.db_type(), nullable=False),
Column('status_reason',Unicode(255)),
- Column('private', Boolean, default=False),
Column('deleted', Boolean, default=False),
Column('memory', Integer),
Column('checksum', String(32)),
@@ -453,13 +452,14 @@ class System(SystemObject, ActivityMixin):
if user:
if not user.is_admin() and \
not user.has_permission(u'secret_visible'):
- query = query.filter(
- or_(System.private==False,
+ query = query.outerjoin(System.custom_access_policy).filter(
+ or_(SystemAccessPolicy.grants(user, SystemPermission.view),
System.owner == user,
System.loaned == user,
System.user == user))
else:
- query = query.filter(System.private==False)
+ query = query.outerjoin(System.custom_access_policy).filter(
+ SystemAccessPolicy.grants_everybody(SystemPermission.view))
return query
diff --git a/Server/bkr/server/model/types.py b/Server/bkr/server/model/types.py
index 6b747a8..a305b0d 100644
--- a/Server/bkr/server/model/types.py
+++ b/Server/bkr/server/model/types.py
@@ -131,6 +131,7 @@ class RecipeVirtStatus(DeclEnum):
class SystemPermission(DeclEnum):
symbols = [
+ ('view', u'view', dict(label=_(u'View'))),
('edit_policy', u'edit_policy', dict(label=_(u'Edit this policy'))),
('edit_system', u'edit_system', dict(label=_(u'Edit system details'))),
('loan_any', u'loan_any', dict(label=_(u'Loan to anyone'))),
diff --git a/Server/bkr/server/reporting-queries/machine-hours-by-user-arch.sql b/Server/bkr/server/reporting-queries/machine-hours-by-user-arch.sql
index 4aab86f..4de37f6 100644
--- a/Server/bkr/server/reporting-queries/machine-hours-by-user-arch.sql
+++ b/Server/bkr/server/reporting-queries/machine-hours-by-user-arch.sql
@@ -43,8 +43,14 @@ LEFT OUTER JOIN system_access_policy ON system.id = system_access_policy.system_
WHERE reservation.start_time < '2012-11-01 00:00:00'
AND (reservation.finish_time >= '2012-10-01 00:00:00'
OR reservation.finish_time IS NULL)
- -- limit to systems which everybody is allowed to reserve
- AND system.private = 0 AND EXISTS (
+ -- limit to systems which everybody is allowed to view and reserve
+ AND EXISTS (
+ SELECT 1
+ FROM system_access_policy_rule
+ WHERE policy_id = system_access_policy.id
+ AND permission = 'view'
+ AND user_id IS NULL AND group_id IS NULL)
+ AND EXISTS (
SELECT 1
FROM system_access_policy_rule
WHERE policy_id = system_access_policy.id
diff --git a/Server/bkr/server/reporting-queries/recipe-hours-by-user-arch.sql b/Server/bkr/server/reporting-queries/recipe-hours-by-user-arch.sql
index 1a7dd0c..17b292e 100644
--- a/Server/bkr/server/reporting-queries/recipe-hours-by-user-arch.sql
+++ b/Server/bkr/server/reporting-queries/recipe-hours-by-user-arch.sql
@@ -38,8 +38,14 @@ LEFT OUTER JOIN system_access_policy ON system.id = system_access_policy.system_
WHERE recipe.start_time < '2012-11-01 00:00:00'
AND (recipe.finish_time >= '2012-10-01 00:00:00'
OR recipe.finish_time IS NULL)
- -- limit to systems which everybody is allowed to reserve
- AND system.private = 0 AND EXISTS (
+ -- limit to systems which everybody is allowed to view and reserve
+ AND EXISTS (
+ SELECT 1
+ FROM system_access_policy_rule
+ WHERE policy_id = system_access_policy.id
+ AND permission = 'view'
+ AND user_id IS NULL AND group_id IS NULL)
+ AND EXISTS (
SELECT 1
FROM system_access_policy_rule
WHERE policy_id = system_access_policy.id
diff --git a/Server/bkr/server/templates/system_form.kid b/Server/bkr/server/templates/system_form.kid
index 07d2e90..6f201db 100644
--- a/Server/bkr/server/templates/system_form.kid
+++ b/Server/bkr/server/templates/system_form.kid
@@ -96,12 +96,6 @@ $(document).ready(function(){
</div>
</div>
<div class="control-group">
- ${label_for('private')}
- <div class="controls">
- ${display_system_property("private")}
- </div>
- </div>
- <div class="control-group">
${label_for('lab_controller_id')}
<div class="controls">
${display_system_property("lab_controller_id")}
diff --git a/Server/bkr/server/widgets.py b/Server/bkr/server/widgets.py
index fe6ed79..55f2161 100644
--- a/Server/bkr/server/widgets.py
+++ b/Server/bkr/server/widgets.py
@@ -1372,9 +1372,6 @@ class SystemForm(Form):
TextField(name='owner', label=_(u'Owner')),
TextField(name='loaned', label=_(u'Loaned To')),
TextField(name='contact', label=_(u'Contact')),
- CheckBox(name='private', label=_(u'Secret (NDA)'),
- help_text=_(u'Should this system be invisible to '
- 'all other users?')),
AutoCompleteField(name='group',
search_controller=url("/groups/by_name"),
search_param="name",
diff --git a/documentation/user-guide/systems/sharing.rst b/documentation/user-guide/systems/sharing.rst
index adf781c..e373652 100644
--- a/documentation/user-guide/systems/sharing.rst
+++ b/documentation/user-guide/systems/sharing.rst
@@ -47,6 +47,13 @@ The following permissions can be granted in an access policy:
============== =================== ===========================================
Name Label Description
============== =================== ===========================================
+view View The user can view the system and find it in
+ search results. If the system is loaned to
+ a user, they are permitted to view it for
+ the duration of the loan. If a user is not
+ permitted to view the system, they cannot
+ interact with it in any way and any other
+ permissions granted to them have no effect.
edit_policy Edit this policy The user can edit the access policy to grant
or revoke permissions, including adding new
users and groups to the policy. Users with
diff --git a/documentation/whats-new/next/system-view-permission.rst b/documentation/whats-new/next/system-view-permission.rst
new file mode 100644
index 0000000..d8ded9e
--- /dev/null
+++ b/documentation/whats-new/next/system-view-permission.rst
@@ -0,0 +1,54 @@
+"View" permission in system access policies
+===========================================
+
+System access policies have a new permission, "view", which controls who can
+see the system in Beaker's inventory. This replaces the previous "Secret" flag
+for systems giving finer grained control over visibility. The default for new
+systems is to grant "view" permission to everybody.
+
+Database changes
+----------------
+
+To add the new "view" permission, run the following SQL::
+
+ ALTER TABLE system_access_policy_rule
+ MODIFY permission ENUM('edit_policy', 'edit_system', 'loan_any',
+ 'loan_self', 'control_system', 'reserve', 'view');
+
+The following SQL will populate the access policies with a rule granting
+everybody "view" permission for existing systems that are not marked secret.
+
+::
+
+ INSERT INTO system_access_policy (system_id)
+ SELECT id FROM system
+ WHERE NOT EXISTS (SELECT 1 FROM system_access_policy
+ WHERE system_id = system.id);
+
+ INSERT INTO system_access_policy_rule
+ (policy_id, user_id, group_id, permission)
+ SELECT system_access_policy.id, NULL, NULL, 'view'
+ FROM system_access_policy
+ INNER JOIN system ON system_access_policy.system_id = system.id
+ WHERE NOT EXISTS (SELECT 1 FROM system_access_policy_rule
+ WHERE policy_id = system_access_policy.id
+ AND user_id IS NULL
+ AND group_id IS NULL
+ AND permission = 'view')
+ AND system.private = 0;
+
+To roll back, delete the newly created rules and adjust the permission enum::
+
+ DELETE FROM system_access_policy_rule
+ WHERE permission = 'view';
+
+ ALTER TABLE system_access_policy_rule
+ MODIFY permission ENUM('edit_policy', 'edit_system', 'loan_any',
+ 'loan_self', 'control_system', 'reserve');
+
+Once you are satisfied that the upgrade is successful, you can drop the
+obsoleted column. There is no rollback procedure for this step.
+
+::
+
+ ALTER TABLE system DROP private;