[SNOW-147] Filter out deleted ACLs from ACL_LATEST

[jaymedina]

problem

aclsnapshots does not capture deleted ACLs, and there was an issue in the way the latest ACLs were retrieved. Both are addressed in the solution below.

solution

• Remove R script creating the ACL_LATEST dynamic table
• Add V script re-introducing the ACL_LATEST dynamic table, refactored
• Add a snapshot_date window to filter out for only the latest ACLs, which should minimize the risk of including deleted ACLs

Original Process

1. Unpack
2. Dedup

Original Data Structure

In this example, principal ID 200 gets READ privileges removed in 2025.

owner-id	record-access	timestamp
2	{100: [read, download], 200: [read]}	12-1-2024
2	{100: [read, download]}	1-1-2025

Data Structure (After Unpacking & Deduplicating)

Notice how unpacking before deduplicating, and defining a duplicate as a repeated entry of owner_id access_type and prinicpal_id, lets the ACL with pid 200 sneak into the final latest table, even though their permissions were removed in 1-1-2025.

owner-id	access-type	pid	timestamp
2	read	100	1-1-2025
2	download	100	1-1-2025
2	read	200	12-1-2024

---

New Process

1. Dedup
2. Unpack

Data Structure (After Deduplicating & Unpacking)

When we first deduplicate only based on the owner_id, and then unpack the access_type and principal_id, only the ACL from 1-1-2025 is kept in the latest table.

owner-id	access-type	pid	timestamp
2	read	100	1-1-2025
2	download	100	1-1-2025

testing

For testing, I created a temporary table for `ACL_LATEST` that I ran all the queries against.

CREATE OR REPLACE TEMPORARY TABLE temp_acl_latest AS
WITH dedup_acl AS (
    SELECT
        *,
        parse_json(resource_access) AS acl
    FROM synapse_data_warehouse.SYNAPSE_RAW.ACLSNAPSHOTS
    WHERE
        SNAPSHOT_DATE >= CURRENT_TIMESTAMP - INTERVAL '14 days'
    QUALIFY
        ROW_NUMBER() OVER (
            PARTITION BY OWNER_ID
            ORDER BY CHANGE_TIMESTAMP DESC, SNAPSHOT_TIMESTAMP DESC
        ) = 1
),
dedup_acl_expanded AS (
    SELECT
        CREATED_ON,
        CHANGE_TIMESTAMP,
        CHANGE_TYPE,
        OWNER_ID,
        OWNER_TYPE,
        SNAPSHOT_DATE,
        SNAPSHOT_TIMESTAMP,
        COALESCE(
            array_sort(value:"accesstype"::variant),
            array_sort(value:"accessType"::variant),
            array_sort(value:"accesstype#1"::variant),
            array_sort(value:"accesstype#2"::variant),
            array_sort(value:"accesstype#3"::variant)
        ) AS access_type,
        COALESCE(
            value:"principalId"::number,
            value:"principalid"::number,
            value:"principalid#1"::number
        ) AS principal_id
    FROM 
        dedup_acl,
        LATERAL FLATTEN(acl, outer => TRUE)
)
SELECT
    *
FROM
    dedup_acl_expanded;

Next, I queried for the discrepancies between the temporary table and the current `acl_latest`

WITH temp_counts AS (
    SELECT owner_id, COUNT(accesstype) AS temp_count
    FROM temp_acl_latest
    GROUP BY owner_id
),
acl_counts AS (
    SELECT owner_id, COUNT(access_type) AS acl_count
    FROM synapse_data_warehouse.synapse.acl_latest
    GROUP BY owner_id
)
SELECT 
    COALESCE(t.owner_id, a.owner_id) AS owner_id,
    t.temp_count,
    a.acl_count,
    ABS(COALESCE(t.temp_count, 0) - COALESCE(a.acl_count, 0)) AS difference
FROM temp_counts t
FULL OUTER JOIN acl_counts a
    ON t.owner_id = a.owner_id
WHERE COALESCE(t.temp_count, 0) != COALESCE(a.acl_count, 0);

Lastly, I would arbitrarily select entities (`owner_id`) from the previous query and compare the ACL results between the new solution table and the current ACL_LATEST table, against the entity itself on Synapse.

select owner_id, change_timestamp, access_type, principal_id
from temp_acl_latest 
where owner_id = 24246541
order by change_timestamp desc, principal_id desc;

select *
from synapse_data_warehouse.synapse.acl_latest
where owner_id = 24246541
order by change_timestamp desc, principal_id desc;

Here are some example results that confirm the results from `temp_acl_latest` are more accurate

1. syn3482905 no longer exists, and ACL does not show up in new solution, but does in acl_latest

2. same with syn27899252

3. syn63997960 has 6 ACLs on synapse, matching the new solution

new sol:

original sol has 3 ACLs that were removed:

Leveraging NODE_LATEST/TEAM_LATEST for further verification

I looked to see what nodes might be missing from the new solution, and found that all the ones listed are nodes that don't exist anymore (a separate problem for node_latest).

SELECT *
FROM synapse.node_latest
WHERE benefactor_id = id
  AND id NOT IN (
    SELECT DISTINCT owner_id
    FROM temp_acl_latest
  );

Likeswise, I did the same for teams and saw that MOST of the missing IDs are because they don't exist in the original aclsnapshots table OR because of the 14 day cutoff in our new solution (see below for the ones that ARE in aclsnapshots, but don't make it through the 14 day cutoff):

with ids_in_team_latest_not_in_acl_latest as (
select id
from synapse.team_latest
where id not in (
    SELECT DISTINCT owner_id
        FROM temp_acl_latest
      )
order by snapshot_timestamp desc, change_timestamp desc
)
select *
from synapse_raw.aclsnapshots
where owner_id in (select id from ids_in_team_latest_not_in_acl_latest);

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[jaymedina]

Marking this as RFR pending conversation here.

[BryanFauble]

It looks like commits unrelated to your work are now showing up in the diff :/ Is this what Phil was eluding to in the discussion from #138 (comment) ?

[jaymedina]

Oops, just saw this, yes this is related to that discussion. Some duplicate commits from dev were dropped as Phil was cleaning up the branches, and it's showing up as a new commit being introduced to dev from here. I made a new branch just to start from a clean slate.

[BryanFauble]

The logic of this query makes sense! I was able to run it and I saw the results that I was expecting.

[jaymedina]

Closing this due to too many merge conflicts between this branch and the new dev. New PR here #140