Skip to content
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

Adding row level security to sample and genomics tables #328

Merged
merged 4 commits into from
Aug 22, 2023

Conversation

davereinhart
Copy link
Contributor

Adds row-level security to the following tables in warehouse schema:

  • sample
  • genomic_sequence
  • consensus_genome
  • sequence_read_set

An access_role column has been added to each table to store an optional role name that the data is accessible to. The default behavior (with access_role=NULL) will remain unchanged, with no row-level security enforced.

Note: I did not add verify scripts for each table, as some of the them have foreign key constraints that can cause those fail if things aren't populated yet. I may revisit this if I have more time, but for now, testing sqitch deploy and revert should be sufficient.

@davereinhart davereinhart requested a review from a team as a code owner July 27, 2023 22:05
@davereinhart davereinhart marked this pull request as draft July 27, 2023 22:05
Adding security invoker to shipping views to enforce row-level security policies on the underlying tables. By specifying this option, the current user's permissions are applied rather than the default (view owner's) permissions.
Copy link
Contributor

@bencap bencap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good Dave and is working for me on my local (upgraded to 15.3). Unless I'm mistaken on the SSH tunnel, we will also need to update sqitch.conf to point to localhost and access ID3C via SSH to deploy this via Sqitch.

Just making sure I understand the Metabase side: We would have add a new Metabase user that has access to the marked Cascadia data (the default metabase user would not have that access). We'd then create a new Metabase database connection using that user to hit ID3C.
Then Metabase users with Cascadia privileges would be inside a Cascadia group, which would then be the only group with access to that new Cascadia database connection.

@davereinhart
Copy link
Contributor Author

This looks good Dave and is working for me on my local (upgraded to 15.3). Unless I'm mistaken on the SSH tunnel, we will also need to update sqitch.conf to point to localhost and access ID3C via SSH to deploy this via Sqitch.

Just making sure I understand the Metabase side: We would have add a new Metabase user that has access to the marked Cascadia data (the default metabase user would not have that access). We'd then create a new Metabase database connection using that user to hit ID3C. Then Metabase users with Cascadia privileges would be inside a Cascadia group, which would then be the only group with access to that new Cascadia database connection.

Thanks, you're right sqitch.conf does need to be updated to use an SSH tunnel. Port number will vary I'll just add a placeholder for that.

That's correct on Metabase data access, there will also be a Cascadia group role for direct connections.

Databases are no longer directly accessible, and each environment may have connections to ID3C configured differently, so replacing hard coded URIs with placeholders. `sqitch.template.conf` should be copied to `sqitch.conf` and the database connection URIs should updated in that file prior to running sqitch commands.
Copy link
Contributor

@bencap bencap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, we will need to ensure that no additional users are members of the dumper role before deploying to ensure that no one retains undesirable access to the receiving tables.

@davereinhart davereinhart merged commit 04f537e into master Aug 22, 2023
4 checks passed
@davereinhart davereinhart deleted the add-row-level-security branch August 22, 2023 21:26
@davereinhart
Copy link
Contributor Author

Deployed via sqitch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants