-
Notifications
You must be signed in to change notification settings - Fork 152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Admin signoffs for changes to permissions #3088
base: main
Are you sure you want to change the base?
Admin signoffs for changes to permissions #3088
Conversation
… without restrictions
diff --git a/tests/test_db.py b/tests/test_db.py
index b8477ffc..f520bd46 100644
--- a/tests/test_db.py
+++ b/tests/test_db.py
@@ -5362,7 +5362,13 @@ class TestPermissions(unittest.TestCase, MemoryDatabaseMixin):
self.assertEqual(set(history_columns), set(expected))
def testGrantPermissions(self):
- self.permissions.insert("bob", username="cathy", permission="release", options=dict(products=["SeaMonkey"]))
+ self.permissions.insert(
+ "bob",
+ username="cathy",
+ signoffs=[{"sc_id": 1, "username": "bill", "role": "admin"}, {"sc_id": 1, "username": "zawadi", "role": "admin"}],
+ permission="release",
+ options=dict(products=["SeaMonkey"]),
+ )
query = self.permissions.t.select().where(self.permissions.username == "cathy")
query = query.where(self.permissions.permission == "release")
self.assertEqual(query.execute().fetchall(), [("release", "cathy", dict(products=["SeaMonkey"]), 1)])
|
To debug the tests, you can run this script in the root of your clone of diff --git a/tests/test_db.py b/tests/test_db.py
index b8477ffc..84e98564 100644
--- a/tests/test_db.py
+++ b/tests/test_db.py
@@ -5362,6 +5362,7 @@ class TestPermissions(unittest.TestCase, MemoryDatabaseMixin):
self.assertEqual(set(history_columns), set(expected))
def testGrantPermissions(self):
+ raise Exception("breakpoint")
self.permissions.insert("bob", username="cathy", permission="release", options=dict(products=["SeaMonkey"]))
query = self.permissions.t.select().where(self.permissions.username == "cathy")
query = query.where(self.permissions.permission == "release") |
… permission based signoffs
… with permission based signoffs
bd6e7c5
to
acc8f7c
Compare
diff --git a/src/auslib/db.py b/src/auslib/db.py
index 3861d5c6..91a46b1f 100644
--- a/src/auslib/db.py
+++ b/src/auslib/db.py
@@ -2649,7 +2649,7 @@ class Permissions(AUSTable):
if not row:
continue
else:
- potential_required_signoffs["rs"].extend([dict(permission="admin", signoffs_required=2)])
+ potential_required_signoffs["rs"].extend(self.db.permissionsRequiredSignoffs)
return potential_required_signoffs
def assertPermissionExists(self, permission):
@@ -3016,6 +3016,7 @@ class AUSDatabase(object):
self.setDburi(dburi, mysql_traditional_mode, releases_history_buckets, releases_history_class, async_releases_history_class)
self.log = logging.getLogger(self.__class__.__name__)
self.systemAccounts = []
+ self.permissionsRequiredSignoffs = []
def setDburi(
self,
@@ -3133,9 +3134,8 @@ class AUSDatabase(object):
def productRequiredSignoffs(self):
return self.productRequiredSignoffsTable
- @property
- def permissionsRequiredSignoffs(self):
- return self.permissionsRequiredSignoffsTable
+ def setPermissionsRequiredSignoffs(self, permissionsRequiredSignoffs):
+ self.permissionsRequiredSignoffs = permissionsRequiredSignoffs
@property
def dockerflow(self):
diff --git a/uwsgi/admin.wsgi b/uwsgi/admin.wsgi
index 2daed094..314e23b0 100644
--- a/uwsgi/admin.wsgi
+++ b/uwsgi/admin.wsgi
@@ -46,6 +46,10 @@ if STAGING or LOCALDEV:
}
)
+if LOCALDEV:
+ PERMISSIONS_REQUIRED_SIGNOFFS = [{"permission": "admin", "signoffs_required": 1}]
+else:
+ PERMISSIONS_REQUIRED_SIGNOFFS = [{"permission": "admin", "signoffs_required": 2}]
# Logging needs to be set-up before importing the application to make sure that
# logging done from other modules uses our Logger.
@@ -148,6 +152,7 @@ else:
dbo.setDb(os.environ["DBURI"], buckets)
dbo.setSystemAccounts(SYSTEM_ACCOUNTS)
dbo.setDomainAllowlist(DOMAIN_ALLOWLIST)
+dbo.setPermissionsRequiredSignoffs(PERMISSIONS_REQUIRED_SIGNOFFS)
application.config["ALLOWLISTED_DOMAINS"] = DOMAIN_ALLOWLIST
application.config["PAGE_TITLE"] = "Balrog Administration"
application.config["SECRET_KEY"] = os.environ["SECRET_KEY"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make sure I understand: this patch is adding the general ability to require signoffs based on permissions (even though we're only specifying one admin
signoff for permission changes at the moment).
Assuming that's right, does this mean that we could theoretically set up required signoffs from roles and permissions in the same place?
…ert required columns to or
@bhearsum Not with the way I have currently implemented the change it checks for either the permission or the signoff. But it could be modified to check a signoff against both. |
OK, that's fine. That's for the explanation! |
c0b2b74
to
2369aa1
Compare
Require 2 admin signoffs for changes to permissions. Fixes #2194