Granular ConfigDB object permissions

acs-auth/lib/api_v2.js

@@ -142,6 +142,11 @@ export class APIv2 {
 
         const grant = req.method == "PUT" ? req.body : null;
         if (grant && !valid_grant(grant)) fail(422);
+        /* XXX There is a race condition here but I can't see how to
+         * avoid it. Our permitted list is built here, before the txn,
+         * so in principle is out of date by the time we use it. But we
+         * need to sample the ConfigDB information somewhere, and we
+         * don't have cross-service transactions. */
         const permitted = await this.data.check_targ(req.auth, Perm.WriteACL, true);
 
         const rv = await this.data.request({ type: "grant", uuid, grant, permitted });

acs-auth/lib/loader.js

@@ -38,29 +38,12 @@ export class Loader {
                         Object.entries(targs)
                             .flatMap(([target, plural]) =>
                                 ({ principal, permission, target, plural }))));
-        const uuids = new Set([
-            ...grants.map(g => g.principal),
-            ...grants.map(g => g.permission),
-            ...grants.map(g => g.target),
-            ...krbs.map(k => k.uuid),
-        ]);
+        const permitted = {
+            acl:    await data.check_targ(req.auth, Perm.WriteACL, true),
+            id:     await data.check_targ(req.auth, Perm.WriteACL, true),
+        };
 
-        const ids   = krbs.map(k => k.uuid);
-        const perms = new Set(grants.map(g => g.permission));
-
-        const perm_ok = await data.check_targ(req.auth, Perm.WriteACL, true);
-        const id_ok = await data.check_targ(req.auth, Perm.WriteKrb, true);
-
-        for (const id of ids) {
-            if (!id_ok?.(id))
-                req.fail(403, "Can't write identity for %s", id);
-        }
-        for (const perm of perms) {
-            if (!perm_ok?.(perm))
-                req.fail(403, "Can't grant permission for %s", perm);
-        }
-
-        const rv = await this.data.request({ type: "dump", grants, krbs, uuids });
+        const rv = await this.data.request({ type: "dump", grants, krbs, permitted });
         res.status(rv.status).end();
     }
 }

acs-auth/lib/model.js

@@ -76,8 +76,30 @@ export default class Model extends Queries {
     }
 
     dump_request (r) {
+        const uuids = [...new Set([
+            ...r.grants.map(g => g.principal),
+            ...r.grants.map(g => g.permission),
+            ...r.grants.map(g => g.target),
+            ...r.krbs.map(k => k.uuid),
+        ])];
+
+        const ids = r.krbs.map(k => k.uuid);
+        if (!ids.every(u => r.permitted.id?.(u)))
+            return { status: 403 };
+
+        const gto = [...new Set(r.grants.map(g => g.principal))];
+
         return this.txn(async q => {
-            const n_uuid = await q.dump_load_uuids(r.uuids);
+            const existing = await q.dump_existing_perms(gto);
+            const perms = [...new Set([
+                ...existing,
+                ...r.grants.map(g => g.permission),
+            ])];
+            this.log("Need WriteACL on %o", perms);
+            if (!perms.every(p => r.permitted.acl?.(p)))
+                return { status: 403 };
+
+            const n_uuid = await q.dump_load_uuids(uuids);
             if (n_uuid.length)
                 this.log("Inserted new UUIDs: %o", n_uuid);
 

acs-auth/lib/queries.js

@@ -211,14 +211,24 @@ export default class Queries {
         `, [group]);
     }
 
+    dump_existing_perms (princs) {
+        return this.q_list(`
+            select distinct p.uuid
+            from unnest($1::uuid[]) q(uuid) 
+                join uuid u on u.uuid = q.uuid
+                join ace e on e.principal = u.id
+                join uuid p on p.id = e.permission
+        `, [princs]);
+    }
+
     dump_load_uuids (uuids) {
         return this.q_list(`
             insert into uuid (uuid)
             select u.uuid
             from unnest($1::uuid[]) u(uuid)
             on conflict do nothing
             returning uuid
-        `, [[...uuids]]);
+        `, [uuids]);
     }
 
     dump_load_krbs (krbs) {
@@ -232,26 +242,39 @@ export default class Queries {
         `, [krbs]);
     }
 
-    /* XXX This form of loading gives no way for a revised version of a
-     * dump to remove old grants. This is a problem across the board
-     * with our dump loading logic but is more important here. I think
-     * the only solution that works is to introduce a 'source' for these
-     * entries such that loading a new dumps clears old data from that
-     * source. Moving the grants into the ConfigDB would make it easier
-     * to solve this in general. */
     dump_load_aces (aces) {
         return this.q_rows(`
-            insert into ace (principal, permission, target, plural)
-            select u.id, p.id, t.id, 
-                coalesce((g.g->'plural')::boolean, false)
-            from unnest($1::jsonb[]) g(g)
-                join uuid u on u.uuid::text = g.g->>'principal'
-                join uuid p on p.uuid::text = g.g->>'permission'
-                join uuid t on t.uuid::text = g.g->>'target'
-            on conflict (principal, permission, target) do update
-                set plural = excluded.plural
-                where ace.plural != excluded.plural
-            returning principal, permission, target
+            with n_ace as (
+                select u.id princ, p.id perm, t.id targ,
+                    coalesce((g.g->'plural')::boolean, false) plural
+                from unnest($1::jsonb[]) g(g)
+                    join uuid u on u.uuid = (g.g->>'principal')::uuid
+                    join uuid p on p.uuid = (g.g->>'permission')::uuid
+                    join uuid t on t.uuid = (g.g->>'target')::uuid
+            ),
+            existing as (
+                select e.id 
+                from n_ace n 
+                    join ace e on e.principal = n.princ
+                        and e.permission = n.perm
+                        and e.target = n.targ
+            ),
+            d_ace as (
+                delete from ace
+                where principal in (select princ from n_ace)
+                    and id not in (select id from existing)
+                returning id
+            ),
+            i_ace as (
+                insert into ace (principal, permission, target, plural)
+                select princ, perm, targ, plural from n_ace
+                on conflict (principal, permission, target) do update
+                    set plural = excluded.plural
+                    where ace.plural != excluded.plural
+                returning id
+            )
+            select id from d_ace
+                union all select id from i_ace
         `, [aces]);
     }
 }

acs-configdb/lib/api-v1.js

@@ -78,52 +78,21 @@ export class APIv1 {
             });
         }
 
-        api.post("/app", this.apps_post.bind(this));
-        api.get("/app/:app", this.app_get.bind(this));
+        const deny = (req, res) => res.status(403).end();
+
+        /* Deny access to these compat endpoints at this point. Nothing
+         * is using them and it's awkward to keep them working. */
+        api.post("/app", deny);
+        api.get("/app/:app", deny);
 
         api.get("/app/:app/search", this.config_search.bind(this));
         api.get("/app/:app/class/:class/search", this.config_search.bind(this));
 
         api.post("/load", this.dump_load.bind(this));
-        api.get("/save", this.dump_save.bind(this));
-    }
-
-    async apps_post(req, res) {
-        const ok = await this.auth.check_acl(req.auth, Perm.Manage_Obj, Class.App, true);
-        if (!ok) return res.status(403).end();
-
-        const uuid = req.body.uuid;
-
-        let rv = await this.model.object_create(uuid, Class.App);
-
-        if (rv < 300) {
-            /* XXX this overwrites any existing information */
-            /* ignore errors */
-            await this.model.config_put(
-                {app: App.Info, object: uuid},
-                {name: req.body.name, primaryClass: Class.App});
-        }
-
-        res.status(rv).end();
-    }
-
-    async app_get(req, res) {
-        const ok = await this.auth.check_acl(req.auth, Perm.Manage_Obj, Class.App, true);
-        if (!ok) return res.status(403).end();
-
-        const uuid = req.params.app;
-        /* XXX What verification do we want here? Do we insist that an
-         * App-UUID is a member of Application, or do we allow any
-         * object? */
-        const klass = await this.model.object_class(uuid);
-        if (klass != Class.App)
-            res.status(404).end();
-
-        const info = await this.model.config_get(
-            {app: App.Info, object: uuid});
-        const name = info?.config?.name ?? "";
 
-        res.status(200).json({uuid, name});
+        /* Deny access to /save; it isn't useful until it can be
+         * reimplemented to support v2 dumps and ownerships. */
+        api.get("/save", deny);
     }
 
     config_search_parse(query) {
@@ -155,7 +124,7 @@ export class APIv1 {
 
     async config_search(req, res) {
         const {app, "class": klass} = req.params;
-        const ok = await this.auth.check_acl(req.auth, Perm.Read_App, app, true);
+        const ok = await this.auth.check_acl(req.auth, Perm.ReadApp, app, true);
         if (!ok) return res.status(403).end();
 
         const query = this.config_search_parse(req.query);
@@ -184,13 +153,4 @@ export class APIv1 {
         req.url = "/load";
         req.app.handle(req, res);
     }
-
-    async dump_save(req, res) {
-        const ok = await this.auth.check_acl(req.auth, Perm.Manage_Obj, UUIDs.Null)
-            && await this.auth.check_acl(req.auth, Perm.Read_App, UUIDs.Null);
-        if (!ok) return res.status(403).end();
-
-        const dump = await this.model.dump_save();
-        res.status(200).json(dump);
-    }
 }