Re: running logical replication as the subscription owner

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Ajin Cherian <itsajin(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Jelte Fennema <postgres(at)jeltef(dot)nl>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: running logical replication as the subscription owner
Date: 2023-05-22 12:35:53
Message-ID: CAD21AoD=CqeoAFczUoRhfgCxWavhZ2=oYt8reid0uBb1+-1piA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, May 17, 2023 at 10:10 AM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>
> On Mon, May 15, 2023 at 10:47 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Mon, May 15, 2023 at 5:44 PM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
> > >
> > > On Fri, May 12, 2023 at 9:55 PM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
> > > >
> > > > If nobody else is working on this, I can come up with a patch to fix this
> > > >
> > >
> > > Attaching a patch which attempts to fix this.
> > >
> >
> > Thank you for the patch! I think we might want to have tests for it.
> >
> I have updated the patch with a test case as well.

Thank you for updating the patch! Here are review comments:

+ /*
+ * Make sure that the copy command runs as the table owner, unless
+ * the user has opted out of that behaviour.
+ */
+ run_as_owner = MySubscription->runasowner;
+ if (!run_as_owner)
+ SwitchToUntrustedUser(rel->rd_rel->relowner, &ucxt);
+
/* Now do the initial data copy */
PushActiveSnapshot(GetTransactionSnapshot());

I think we should switch users before the acl check in
LogicalRepSyncTableStart().

---
+# Create a trigger on table alice.unpartitioned that writes
+# to a table that regress_alice does not have permission.
+$node_subscriber->safe_psql(
+ 'postgres', qq(
+SET SESSION AUTHORIZATION regress_alice;
+CREATE OR REPLACE FUNCTION alice.alice_audit()
+RETURNS trigger AS
+\$\$
+ BEGIN
+ insert into public.admin_audit values(2);
+ RETURN NEW;
+ END;
+\$\$
+LANGUAGE 'plpgsql';
+CREATE TRIGGER ALICE_TRIGGER AFTER INSERT ON alice.unpartitioned FOR EACH ROW
+EXECUTE PROCEDURE alice.alice_audit();
+ALTER TABLE alice.unpartitioned ENABLE ALWAYS TRIGGER ALICE_TRIGGER;
+));

While this approach works, I'm not sure we really need a trigger for
this test. I've attached a patch for discussion that doesn't use
triggers for the regression tests. We create a new subscription owned
by a user who doesn't have the permission to the target table. The
test passes only if run_as_owner = true works.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
fix_run_as_owner.patch application/octet-stream 4.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message reid.thompson 2023-05-22 12:42:51 Re: Add the ability to limit the amount of memory that can be allocated to backends.
Previous Message Peter Eisentraut 2023-05-22 12:34:31 Re: Order changes in PG16 since ICU introduction