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

control-plane: change authz grants of unchanged_draft_specs view #1246

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

psFried
Copy link
Member

@psFried psFried commented Oct 17, 2023

Description:

Initially, the unchanged_draft_specs view was written as being owned by the authenticated role in postgres, to ensure that it always used the RLS policies of the caller. But Supabase has revoked the superuser attribute of the postgres role and no longer provides a way to authenticate as a superuser. This means that alter view ... set owner to authenticated no longer works because authenticated does not have the necessary permission to create a view. See this thread for more.

In this case, we're able to work around this by granting select permission to the authenticated role. This works because both draft_specs_ext and live_specs_ext perform their own enforcement of authZ, so it's safe to bypass RLS.

Also changes the name of the migration to fix a conflict.

Workflow steps:

I re-tested authZ of the view by:

  • create aliceCo/ tenant and create a hello-world capture
  • start editing that capture and hit the re-fresh button
  • confirm that there's 2 rows in unchanged_draft_specs
  • create bobCo/ tenant and repeat the above steps
  • confirm that there's 4 rows in unchanged_draft_specs when querying as superuser, but only 2 rows when using flowctl raw get to query as alice or bob.

This change is Reviewable

Initially, the `unchanged_draft_specs` view was written as being owned by
the `authenticated` role in postgres, to ensure that it always used the RLS
policies of the caller. But Supabase has revoked the `superuser` attribute
of the `postgres` role and no longer provides a way to authenticate as a
superuser. This means that `alter view ... set owner to authenticated` no
longer works because `authenticated` does not have the necessary permission
to create a view. See [this thread](https://github.com/orgs/supabase/discussions/9314)
for more.

In this case, we're able to work around this by granting select permission
to the `authenticated` role. This works because both `draft_specs_ext` and
`live_specs_ext` perform their own enforcement of authZ, so it's safe to
bypass RLS.

Also changes the name of the migration to fix a conflict.
@psFried psFried requested a review from jgraettinger October 17, 2023 20:51
Copy link
Member

@jgraettinger jgraettinger left a comment

Choose a reason for hiding this comment

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

LGTM

@psFried psFried merged commit 3098456 into master Oct 17, 2023
@psFried psFried deleted the phil/prune-migration-fix branch October 17, 2023 21:49
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