Admin signoffs for changes to permissions

src/auslib/util/signoffs.py

@@ -1,7 +1,8 @@
 def serialize_signoff_requirements(requirements):
     dct = {}
     for rs in requirements:
-        signoffs_required = max(dct.get(rs["role"], 0), rs["signoffs_required"])
-        dct[rs["role"]] = signoffs_required
+        signoff_verifier = "role" if "role" in rs else "permission"
+        signoffs_required = max(dct.get(rs[signoff_verifier], 0), rs["signoffs_required"])
+        dct[rs[signoff_verifier]] = signoffs_required
 
     return dct

tests/admin/views/base.py

@@ -75,6 +75,7 @@ def my_userinfo(*args, **kwargs):
         dbo.setDomainAllowlist({"good.com": ("a", "b", "c", "d")})
         self.metadata.create_all(dbo.engine)
         dbo.permissions.t.insert().execute(permission="admin", username="bill", data_version=1)
+        dbo.permissions.t.insert().execute(permission="admin", username="zawadi", data_version=1)
         dbo.permissions.t.insert().execute(permission="permission", username="bob", data_version=1)
         dbo.permissions.t.insert().execute(
             permission="release", username="bob", options=dict(products=["fake", "a", "b"], actions=["create", "modify"]), data_version=1

tests/test_db.py

@@ -83,30 +83,34 @@ def getTempfile(self):
 
 class TestVerifySignoffs(unittest.TestCase):
     def testNoRequiredSignoffs(self):
-        verify_signoffs({}, {})
+        verify_signoffs({}, {}, "role")
 
     def testNoRequiredSignoffsWithSignoffs(self):
-        verify_signoffs({}, [{"role": "releng"}, {"role": "relman"}])
+        verify_signoffs({}, [{"role": "releng"}, {"role": "relman"}], "role")
 
     def testNoSignoffsGiven(self):
         required = [{"role": "releng", "signoffs_required": 1}]
         signoffs = []
-        self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs)
+        signoff_verifier = "role"
+        self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs, signoff_verifier)
 
     def testMissingSignoffFromOneRole(self):
         required = [{"role": "releng", "signoffs_required": 1}, {"role": "relman", "signoffs_required": 1}]
         signoffs = [{"role": "releng", "username": "joe"}]
-        self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs)
+        signoff_verifier = "role"
+        self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs, signoff_verifier)
 
     def testNotEnoughSignoffsFromOneRole(self):
         required = [{"role": "releng", "signoffs_required": 2}, {"role": "relman", "signoffs_required": 1}]
         signoffs = [{"role": "releng", "username": "joe"}, {"role": "relman", "username": "jane"}]
-        self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs)
+        signoff_verifier = "role"
+        self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs, signoff_verifier)
 
     def testExactlyEnoughSignoffsGiven(self):
         required = [{"role": "releng", "signoffs_required": 2}, {"role": "relman", "signoffs_required": 1}]
         signoffs = [{"role": "releng", "username": "joe"}, {"role": "releng", "username": "jane"}, {"role": "relman", "username": "nick"}]
-        verify_signoffs(required, signoffs)
+        signoff_verifier = "role"
+        verify_signoffs(required, signoffs, signoff_verifier)
 
     def testMoreThanEnoughSignoffsGiven(self):
         required = [{"role": "releng", "signoffs_required": 2}, {"role": "relman", "signoffs_required": 1}]
@@ -116,17 +120,20 @@ def testMoreThanEnoughSignoffsGiven(self):
             {"role": "relman", "username": "nick"},
             {"role": "relman", "username": "matt"},
         ]
-        verify_signoffs(required, signoffs)
+        signoff_verifier = "role"
+        verify_signoffs(required, signoffs, signoff_verifier)
 
     def testMultiplePotentialSignoffsForOneGroupWithoutEnoughSignoffs(self):
         required = [{"role": "releng", "signoffs_required": 2}, {"role": "releng", "signoffs_required": 1}, {"role": "relman", "signoffs_required": 1}]
         signoffs = [{"role": "releng", "username": "joe"}, {"role": "relman", "username": "nick"}]
-        self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs)
+        signoff_verifier = "role"
+        self.assertRaises(SignoffRequiredError, verify_signoffs, required, signoffs, signoff_verifier)
 
     def testMultiplePotentialSignoffsForOneGroupWithEnoughSignoffs(self):
         required = [{"role": "releng", "signoffs_required": 2}, {"role": "releng", "signoffs_required": 1}, {"role": "relman", "signoffs_required": 1}]
         signoffs = [{"role": "releng", "username": "joe"}, {"role": "releng", "username": "jane"}, {"role": "relman", "username": "nick"}]
-        verify_signoffs(required, signoffs)
+        signoff_verifier = "role"
+        verify_signoffs(required, signoffs, signoff_verifier)
 
 
 class TestAUSTransaction(unittest.TestCase, MemoryDatabaseMixin):
@@ -5321,6 +5328,7 @@ def setUp(self):
         self.metadata.create_all(self.db.engine)
         self.permissions = self.db.permissions
         self.user_roles = self.db.permissions.user_roles
+        self.db.setAdminRequiredSignoffs([{"permission": "admin", "signoffs_required": 2}])
         self.permissions.t.insert().execute(permission="admin", username="bill", data_version=1)
         self.permissions.t.insert().execute(permission="permission", username="bob", data_version=1)
         self.permissions.t.insert().execute(permission="permission", username="sean", data_version=1)
@@ -5362,7 +5370,13 @@ def testUserRolesHasCorrectTablesAndColumns(self):
         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",
+            signoffs=[{"sc_id": 1, "username": "bill", "role": "admin"}, {"sc_id": 1, "username": "zawadi", "role": "admin"}],
+            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")
         self.assertEqual(query.execute().fetchall(), [("release", "cathy", dict(products=["SeaMonkey"]), 1)])
@@ -5405,7 +5419,12 @@ def testGrantRoleToUserWhoDoesntHaveAPermission(self):
         )
 
     def testRevokePermission(self):
-        self.permissions.delete({"username": "bob", "permission": "release"}, changed_by="bill", old_data_version=1)
+        self.permissions.delete(
+            {"username": "bob", "permission": "release"},
+            changed_by="bill",
+            old_data_version=1,
+            signoffs=[{"sc_id": 5, "username": "bill", "role": "admin"}, {"sc_id": 5, "username": "zawadi", "role": "admin"}],
+        )
         query = self.permissions.t.select().where(self.permissions.username == "bob")
         query = query.where(self.permissions.permission == "release")
         self.assertEqual(len(query.execute().fetchall()), 0)
@@ -5425,7 +5444,12 @@ def testRevokeRoleWithoutPermission(self):
         self.assertRaises(PermissionDeniedError, self.permissions.revokeRole, username="bob", role="releng", changed_by="kirk", old_data_version=1)
 
     def testRevokingPermissionAlsoRevokeRoles(self):
-        self.permissions.delete({"username": "janet", "permission": "release"}, changed_by="bill", old_data_version=1)
+        self.permissions.delete(
+            {"username": "janet", "permission": "release"},
+            changed_by="bill",
+            old_data_version=1,
+            signoffs=[{"sc_id": 3, "username": "bill", "role": "admin"}, {"sc_id": 3, "username": "zawadi", "role": "admin"}],
+        )
         got = self.db.permissions.t.select().where(self.db.permissions.username == "janet").execute().fetchall()
         self.assertEqual(len(got), 0)
         got = self.user_roles.t.select().where(self.user_roles.username == "janet").execute().fetchall()

ui/src/components/SignoffSummary/index.jsx

@@ -92,7 +92,13 @@ function SignoffSummary(props) {
                 <ListItemText
                   primary={
                     <Typography component="p" variant="body2">
-                      {`${count} member${count > 1 ? 's' : ''} of ${role}  -`}
+                      {role === 'admin'
+                        ? `${count} member${
+                            count > 1 ? 's' : ''
+                          } with full fledged ${role} permissions - `
+                        : `${count} member${
+                            count > 1 ? 's' : ''
+                          } of ${role} - `}
                     </Typography>
                   }
                   className={classes.listItemText}

ui/src/views/RequiredSignoffs/ListSignoffs/index.jsx

@@ -33,7 +33,6 @@ import {
 } from '../../../utils/constants';
 import { withUser } from '../../../utils/AuthContext';
 
-const getPermissionChangesLens = product => lensPath([product, 'permissions']);
 const getRulesOrReleasesChangesLens = product =>
   lensPath([product, 'channels']);
 const useStyles = makeStyles(theme => ({
@@ -101,10 +100,6 @@ function ListSignoffs({ user, ...props }) {
 
   const handleSignoffRoleChange = ({ target: { value } }) =>
     setSignoffRole(value);
-  const permissionChanges = view(
-    getPermissionChangesLens(product),
-    requiredSignoffs
-  );
   const rulesOrReleasesChanges = view(
     getRulesOrReleasesChangesLens(product),
     requiredSignoffs
@@ -269,16 +264,6 @@ function ListSignoffs({ user, ...props }) {
       {requiredSignoffs && (
         <Fragment>
           <div className={classes.toolbar}>
-            {permissionChanges && (
-              <Typography gutterBottom variant="h5">
-                Changes to Permissions
-              </Typography>
-            )}
-            {!permissionChanges && rulesOrReleasesChanges && (
-              <Typography gutterBottom variant="h5">
-                Changes to Rules / Releases
-              </Typography>
-            )}
             <div className={classes.dropdownDiv}>
               <TextField
                 className={classes.dropdown}
@@ -294,69 +279,16 @@ function ListSignoffs({ user, ...props }) {
               </TextField>
             </div>
           </div>
-          {permissionChanges && (
-            <SignoffCard
-              className={classes.card}
-              title={capitalCase(product)}
-              to={`/required-signoffs/${product}`}>
-              {Object.entries(permissionChanges).map(
-                ([name, role], index, arr) => {
-                  const key = `${name}-${index}`;
-
-                  return (
-                    <Fragment key={key}>
-                      <SignoffCardEntry
-                        key={name}
-                        name={name}
-                        entry={role}
-                        onCancelDelete={() =>
-                          handleCancelDelete(
-                            OBJECT_NAMES.PERMISSIONS_REQUIRED_SIGNOFF,
-                            role,
-                            name,
-                            product
-                          )
-                        }
-                        onSignoff={() =>
-                          handleSignoff(
-                            OBJECT_NAMES.PERMISSIONS_REQUIRED_SIGNOFF,
-                            role,
-                            name,
-                            product
-                          )
-                        }
-                        onRevoke={() =>
-                          handleRevoke(
-                            OBJECT_NAMES.PERMISSIONS_REQUIRED_SIGNOFF,
-                            role,
-                            name,
-                            product
-                          )
-                        }
-                      />
-                      <Divider
-                        className={classNames({
-                          [classes.lastDivider]: arr.length - 1 === index,
-                        })}
-                      />
-                    </Fragment>
-                  );
-                }
-              )}
-            </SignoffCard>
-          )}
           <div>
             {rulesOrReleasesChanges && (
               <Fragment>
-                {permissionChanges && (
-                  <Fragment>
-                    <br />
-                    <Typography gutterBottom variant="h5">
-                      Changes to Rules / Releases
-                    </Typography>
-                    <br />
-                  </Fragment>
-                )}
+                <Fragment>
+                  <br />
+                  <Typography gutterBottom variant="h5">
+                    Changes to Rules / Releases
+                  </Typography>
+                  <br />
+                </Fragment>
                 {Object.entries(rulesOrReleasesChanges).map(
                   ([channelName, roles]) => (
                     <SignoffCard
@@ -417,7 +349,7 @@ function ListSignoffs({ user, ...props }) {
                 )}
               </Fragment>
             )}
-            {!permissionChanges && !rulesOrReleasesChanges && (
+            {!rulesOrReleasesChanges && (
               <Typography>No required signoffs for {product}</Typography>
             )}
           </div>

ui/src/views/RequiredSignoffs/ViewSignoff/index.jsx

@@ -303,12 +303,6 @@ function ViewSignoff({ isNewSignoff, ...props }) {
                       control={<Radio />}
                       label="Channel"
                     />
-                    <FormControlLabel
-                      disabled={!isNewSignoff}
-                      value="permission"
-                      control={<Radio />}
-                      label="Permission"
-                    />
                   </RadioGroup>
                 </FormControl>
               </Grid>

uwsgi/admin.wsgi

@@ -46,6 +46,10 @@ if STAGING or LOCALDEV:
         }
     )
 
+if STAGING or LOCALDEV:
+    ADMIN_REQUIRED_SIGNOFFS = [{"permission": "admin", "signoffs_required": 1}]
+else:
+    ADMIN_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.setAdminRequiredSignoffs(ADMIN_REQUIRED_SIGNOFFS)
 application.config["ALLOWLISTED_DOMAINS"] = DOMAIN_ALLOWLIST
 application.config["PAGE_TITLE"] = "Balrog Administration"
 application.config["SECRET_KEY"] = os.environ["SECRET_KEY"]