public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v9 4/9] wal_compression_method: default to zlib.. 6+ messages / 4 participants [nested] [flat]
* [PATCH v9 4/9] wal_compression_method: default to zlib.. @ 2021-03-11 23:36 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Justin Pryzby @ 2021-03-11 23:36 UTC (permalink / raw) this is meant to exercise the CIs, and not meant to be merged --- src/backend/access/transam/xlog.c | 2 +- src/backend/utils/misc/guc.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f7909aa1d7..3254c42243 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -98,7 +98,7 @@ char *XLogArchiveCommand = NULL; bool EnableHotStandby = false; bool fullPageWrites = true; bool wal_log_hints = false; -int wal_compression = WAL_COMPRESSION_NONE; +int wal_compression = WAL_COMPRESSION_ZLIB; char *wal_consistency_checking_string = NULL; bool *wal_consistency_checking = NULL; bool wal_init_zero = true; diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index ce1149bed5..14a2203225 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -4822,7 +4822,7 @@ static struct config_enum ConfigureNamesEnum[] = NULL }, &wal_compression, - WAL_COMPRESSION_NONE, wal_compression_options, + WAL_COMPRESSION_ZLIB, wal_compression_options, NULL, NULL, NULL }, -- 2.17.0 --neYutvxvOLaeuPCA Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0005-re-add-wal_compression_method-lz4.patch" ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PoC] Federated Authn/z with OAUTHBEARER @ 2023-01-12 19:08 mahendrakar s <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: mahendrakar s @ 2023-01-12 19:08 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: Andrey Chudnovsky <[email protected]>; [email protected]; Michael Paquier <[email protected]>; pgsql-hackers; [email protected] Hi All, Changes added to Jacob's patch(v2) as per the discussion in the thread. The changes allow the customer to send the OAUTH BEARER token through psql connection string. Example: psql -U [email protected] -d 'dbname=postgres oauth_bearer_token=abc' To configure OAUTH, the pg_hba.conf line look like: local all all oauth provider=oauth_provider issuer="https://example.com"; scope="openid email" We also added hook to libpq to pass on the metadata about the issuer. Thanks, Mahendrakar. On Sat, 17 Dec 2022 at 04:48, Jacob Champion <[email protected]> wrote: > > On Mon, Dec 12, 2022 at 9:06 PM Andrey Chudnovsky > <[email protected]> wrote: > > If your concern is extension not honoring the DBA configured values: > > Would a server-side logic to prefer HBA value over extension-provided > > resolve this concern? > > Yeah. It also seals the role of the extension here as "optional". > > > We are definitely biased towards the cloud deployment scenarios, where > > direct access to .hba files is usually not offered at all. > > Let's find the middle ground here. > > Sure. I don't want to make this difficult in cloud scenarios -- > obviously I'd like for Timescale Cloud to be able to make use of this > too. But if we make this easy for a lone DBA (who doesn't have any > institutional power with the providers) to use correctly and securely, > then it should follow that the providers who _do_ have power and > resources will have an easy time of it as well. The reverse isn't > necessarily true. So I'm definitely planning to focus on the DBA case > first. > > > A separate reason for creating this pre-authentication hook is further > > extensibility to support more metadata. > > Specifically when we add support for OAUTH flows to libpq, server-side > > extensions can help bridge the gap between the identity provider > > implementation and OAUTH/OIDC specs. > > For example, that could allow the Github extension to provide an OIDC > > discovery document. > > > > I definitely see identity providers as institutional actors here which > > can be given some power through the extension hooks to customize the > > behavior within the framework. > > We'll probably have to make some compromises in this area, but I think > they should be carefully considered exceptions and not a core feature > of the mechanism. The gaps you point out are just fragmentation, and > adding custom extensions to deal with it leads to further > fragmentation instead of providing pressure on providers to just > implement the specs. Worst case, we open up new exciting security > flaws, and then no one can analyze them independently because no one > other than the provider knows how the two sides work together anymore. > > Don't get me wrong; it would be naive to proceed as if the OAUTHBEARER > spec were perfect, because it's clearly not. But if we need to make > extensions to it, we can participate in IETF discussions and make our > case publicly for review, rather than enshrining MS/GitHub/Google/etc. > versions of the RFC and enabling that proliferation as a Postgres core > feature. > > > Obtaining a token is an asynchronous process with a human in the loop. > > Not sure if expecting a hook function to return a token synchronously > > is the best option here. > > Can that be an optional return value of the hook in cases when a token > > can be obtained synchronously? > > I don't think the hook is generally going to be able to return a token > synchronously, and I expect the final design to be async-first. As far > as I know, this will need to be solved for the builtin flows as well > (you don't want a synchronous HTTP call to block your PQconnectPoll > architecture), so the hook should be able to make use of whatever > solution we land on for that. > > This is hand-wavy, and I don't expect it to be easy to solve. I just > don't think we have to solve it twice. > > Have a good end to the year! > --Jacob Attachments: [application/gzip] v3-0001-libpq-add-OAUTHBEARER-SASL-mechanism-and-call-back-hooks.patch.gz (8.5K, ../../CABkiuWrzxZLAcHexs9TS2rJ6A_snC4vnxyhY0BTr-ymD8gJDmw@mail.gmail.com/3-v3-0001-libpq-add-OAUTHBEARER-SASL-mechanism-and-call-back-hooks.patch.gz) download [application/gzip] v3-0002-backend-add-OAUTHBEARER-SASL-mechanishm.patch.gz (8.6K, ../../CABkiuWrzxZLAcHexs9TS2rJ6A_snC4vnxyhY0BTr-ymD8gJDmw@mail.gmail.com/4-v3-0002-backend-add-OAUTHBEARER-SASL-mechanishm.patch.gz) download [application/gzip] v3-0004-common-jsonapi-support-FRONTEND-clients.patch.gz (5.9K, ../../CABkiuWrzxZLAcHexs9TS2rJ6A_snC4vnxyhY0BTr-ymD8gJDmw@mail.gmail.com/5-v3-0004-common-jsonapi-support-FRONTEND-clients.patch.gz) download [application/gzip] v3-0003-simple-oauth_provider-extension.patch.gz (1.7K, ../../CABkiuWrzxZLAcHexs9TS2rJ6A_snC4vnxyhY0BTr-ymD8gJDmw@mail.gmail.com/6-v3-0003-simple-oauth_provider-extension.patch.gz) download ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PoC] Federated Authn/z with OAUTHBEARER @ 2023-01-15 20:03 Andrey Chudnovsky <[email protected]> parent: mahendrakar s <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Andrey Chudnovsky @ 2023-01-15 20:03 UTC (permalink / raw) To: mahendrakar s <[email protected]>; +Cc: Jacob Champion <[email protected]>; [email protected]; Michael Paquier <[email protected]>; pgsql-hackers; [email protected] More information on the latest patch. 1. We aligned the implementation with the barebone SASL for OAUTH described here - https://www.rfc-editor.org/rfc/rfc7628 The flow can be explained in the diagram below: +----------------------+ +----------+ | +-------+ | Postgres | | PQconnect ->| | | | | | | | +-----------+ | | | ---------- Empty Token---------> | > | | | | libpq | <-- Error(Discovery + Scope ) -- | < | Pre-Auth | | +------+ | | | Hook | | +- < | Hook | | | +-----------+ | | +------+ | | | | v | | | | | [get token]| | | | | | | | | | | + | | | +-----------+ | PQconnect > | | --------- Access Token --------> | > | Validator | | | | <---------- Auth Result -------- | < | Hook | | | | | +-----------+ | +-------+ | | +----------------------+ +----------+ 2. Removed Device Code implementation in libpq. Several reasons: - Reduce scope and focus on the protocol first. - Device code implementation uses iddawc dependency. Taking this dependency is a controversial step which requires broader discussion. - Device code implementation without iddaws would significantly increase the scope of the patch, as libpq needs to poll the token endpoint, setup different API calls, e.t.c. - That flow should canonically only be used for clients which can't invoke browsers. If it is the only flow to be implemented, it can be used in the context when it's not expected by the OAUTH protocol. 3. Temporarily removed test suite. We are actively working on aligning the tests with the latest changes. Will add a patch with tests soon. We will change the "V3" prefix to make it the next after the previous iterations. Thanks! Andrey. On Thu, Jan 12, 2023 at 11:08 AM mahendrakar s <[email protected]> wrote: > > Hi All, > > Changes added to Jacob's patch(v2) as per the discussion in the thread. > > The changes allow the customer to send the OAUTH BEARER token through psql connection string. > > Example: > psql -U [email protected] -d 'dbname=postgres oauth_bearer_token=abc' > > To configure OAUTH, the pg_hba.conf line look like: > local all all oauth provider=oauth_provider issuer="https://example.com"; scope="openid email" > > We also added hook to libpq to pass on the metadata about the issuer. > > Thanks, > Mahendrakar. > > > On Sat, 17 Dec 2022 at 04:48, Jacob Champion <[email protected]> wrote: > > > > On Mon, Dec 12, 2022 at 9:06 PM Andrey Chudnovsky > > <[email protected]> wrote: > > > If your concern is extension not honoring the DBA configured values: > > > Would a server-side logic to prefer HBA value over extension-provided > > > resolve this concern? > > > > Yeah. It also seals the role of the extension here as "optional". > > > > > We are definitely biased towards the cloud deployment scenarios, where > > > direct access to .hba files is usually not offered at all. > > > Let's find the middle ground here. > > > > Sure. I don't want to make this difficult in cloud scenarios -- > > obviously I'd like for Timescale Cloud to be able to make use of this > > too. But if we make this easy for a lone DBA (who doesn't have any > > institutional power with the providers) to use correctly and securely, > > then it should follow that the providers who _do_ have power and > > resources will have an easy time of it as well. The reverse isn't > > necessarily true. So I'm definitely planning to focus on the DBA case > > first. > > > > > A separate reason for creating this pre-authentication hook is further > > > extensibility to support more metadata. > > > Specifically when we add support for OAUTH flows to libpq, server-side > > > extensions can help bridge the gap between the identity provider > > > implementation and OAUTH/OIDC specs. > > > For example, that could allow the Github extension to provide an OIDC > > > discovery document. > > > > > > I definitely see identity providers as institutional actors here which > > > can be given some power through the extension hooks to customize the > > > behavior within the framework. > > > > We'll probably have to make some compromises in this area, but I think > > they should be carefully considered exceptions and not a core feature > > of the mechanism. The gaps you point out are just fragmentation, and > > adding custom extensions to deal with it leads to further > > fragmentation instead of providing pressure on providers to just > > implement the specs. Worst case, we open up new exciting security > > flaws, and then no one can analyze them independently because no one > > other than the provider knows how the two sides work together anymore. > > > > Don't get me wrong; it would be naive to proceed as if the OAUTHBEARER > > spec were perfect, because it's clearly not. But if we need to make > > extensions to it, we can participate in IETF discussions and make our > > case publicly for review, rather than enshrining MS/GitHub/Google/etc. > > versions of the RFC and enabling that proliferation as a Postgres core > > feature. > > > > > Obtaining a token is an asynchronous process with a human in the loop. > > > Not sure if expecting a hook function to return a token synchronously > > > is the best option here. > > > Can that be an optional return value of the hook in cases when a token > > > can be obtained synchronously? > > > > I don't think the hook is generally going to be able to return a token > > synchronously, and I expect the final design to be async-first. As far > > as I know, this will need to be solved for the builtin flows as well > > (you don't want a synchronous HTTP call to block your PQconnectPoll > > architecture), so the hook should be able to make use of whatever > > solution we land on for that. > > > > This is hand-wavy, and I don't expect it to be easy to solve. I just > > don't think we have to solve it twice. > > > > Have a good end to the year! > > --Jacob ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PoC] Federated Authn/z with OAUTHBEARER @ 2023-01-17 22:43 Jacob Champion <[email protected]> parent: Andrey Chudnovsky <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Jacob Champion @ 2023-01-17 22:43 UTC (permalink / raw) To: Andrey Chudnovsky <[email protected]>; +Cc: mahendrakar s <[email protected]>; [email protected]; Michael Paquier <[email protected]>; pgsql-hackers; [email protected] On Sun, Jan 15, 2023 at 12:03 PM Andrey Chudnovsky <[email protected]> wrote: > 2. Removed Device Code implementation in libpq. Several reasons: > - Reduce scope and focus on the protocol first. > - Device code implementation uses iddawc dependency. Taking this > dependency is a controversial step which requires broader discussion. > - Device code implementation without iddaws would significantly > increase the scope of the patch, as libpq needs to poll the token > endpoint, setup different API calls, e.t.c. > - That flow should canonically only be used for clients which can't > invoke browsers. If it is the only flow to be implemented, it can be > used in the context when it's not expected by the OAUTH protocol. I'm not understanding the concern in the final point -- providers generally require you to opt into device authorization, at least as far as I can tell. So if you decide that it's not appropriate for your use case... don't enable it. (And I haven't seen any claims that opting into device authorization weakens the other flows in any way. So if we're going to implement a flow in libpq, I still think device authorization is the best choice, since it works on headless machines as well as those with browsers.) All of this points at a bigger question to the community: if we choose not to provide a flow implementation in libpq, is adding OAUTHBEARER worth the additional maintenance cost? My personal vote would be "no". I think the hook-only approach proposed here would ensure that only larger providers would implement it in practice, and in that case I'd rather spend cycles on generic SASL. > 3. Temporarily removed test suite. We are actively working on aligning > the tests with the latest changes. Will add a patch with tests soon. Okay. Case in point, the following change to the patch appears to be invalid JSON: > + appendStringInfo(&buf, > + "{ " > + "\"status\": \"invalid_token\", " > + "\"openid-configuration\": \"%s\"," > + "\"scope\": \"%s\" ", > + "\"issuer\": \"%s\" ", > + "}", Additionally, the "issuer" field added here is not part of the RFC. I've written my thoughts about unofficial extensions upthread but haven't received a response, so I'm going to start being more strident: Please, for the sake of reviewers, call out changes you've made to the spec, and why they're justified. The patches seem to be out of order now (and the documentation in the commit messages has been removed). Thanks, --Jacob ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PoC] Federated Authn/z with OAUTHBEARER @ 2023-01-18 01:53 Andrey Chudnovsky <[email protected]> parent: Jacob Champion <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Andrey Chudnovsky @ 2023-01-18 01:53 UTC (permalink / raw) To: Jacob Champion <[email protected]>; +Cc: mahendrakar s <[email protected]>; [email protected]; Michael Paquier <[email protected]>; pgsql-hackers; [email protected] > All of this points at a bigger question to the community: if we choose > not to provide a flow implementation in libpq, is adding OAUTHBEARER > worth the additional maintenance cost? > My personal vote would be "no". I think the hook-only approach proposed > here would ensure that only larger providers would implement it in > practice Flow implementations in libpq are definitely a long term plan, and I agree that it would democratise the adoption. In the previous posts in this conversation I outlined the ones I think we should support. However, I don't see why it's strictly necessary to couple those. As long as the SASL exchange for OAUTHBEARER mechanism is supported by the protocol, the Client side can evolve at its own pace. At the same time, the current implementation allows clients to start building provider-agnostic OAUTH support. By using iddawc or OAUTH client implementations in the respective platforms. So I wouldn't refer to "larger providers", but rather "more motivated clients" here. Which definitely overlaps, but keeps the system open. > I'm not understanding the concern in the final point -- providers > generally require you to opt into device authorization, at least as far > as I can tell. So if you decide that it's not appropriate for your use > case... don't enable it. (And I haven't seen any claims that opting into > device authorization weakens the other flows in any way. So if we're > going to implement a flow in libpq, I still think device authorization > is the best choice, since it works on headless machines as well as those > with browsers.) I agree with the statement that Device code is the best first choice if we absolutely have to pick one. Though I don't think we have to. While device flow can be used for all kinds of user-facing applications, it's specifically designed for input-constrained scenarios. As clearly stated in the Abstract here - https://www.rfc-editor.org/rfc/rfc8628 The authorization code with pkce flow is recommended by the RFSc and major providers for cases when it's feasible. The long term goal is to provide both, though I don't see why the backbone protocol implementation first wouldn't add value. Another point is user authentication is one side of the whole story and the other critical one is system-to-system authentication. Where we have Client Credentials and Certificates. With the latter it is much harder to get generically implemented, as provider-specific tokens need to be signed. Adding the other reasoning, I think libpq support for specific flows can get in the further iterations, after the protocol support. > in that case I'd rather spend cycles on generic SASL. I see 2 approaches to generic SASL: (a). Generic SASL is a framework used in the protocol, with the mechanisms implemented on top and exposed to the DBAs as auth types to configure in hba. This is the direction we're going here, which is well aligned with the existing hba-based auth configuration. (b). Generic SASL exposed to developers on the server- and client- side to extend on. It seems to be a much longer shot. The specific points of large ambiguity are libpq distribution model (which you pointed to) and potential pluggability of insecure mechanisms. I do see (a) as a sweet spot with a lot of value for various participants with much less ambiguity. > Additionally, the "issuer" field added here is not part of the RFC. I've > written my thoughts about unofficial extensions upthread but haven't > received a response, so I'm going to start being more strident: Please, > for the sake of reviewers, call out changes you've made to the spec, and > why they're justified. Thanks for your feedback on this. We had this discussion as well, and added that as a convenience for the client to identify the provider. I don't see a reason why an issuer would be absolutely necessary, so we will get your point that sticking to RFCs is a safer choice. > The patches seem to be out of order now (and the documentation in the > commit messages has been removed). Feedback taken. Work in progress. On Tue, Jan 17, 2023 at 2:44 PM Jacob Champion <[email protected]> wrote: > > On Sun, Jan 15, 2023 at 12:03 PM Andrey Chudnovsky > <[email protected]> wrote: > > 2. Removed Device Code implementation in libpq. Several reasons: > > - Reduce scope and focus on the protocol first. > > - Device code implementation uses iddawc dependency. Taking this > > dependency is a controversial step which requires broader discussion. > > - Device code implementation without iddaws would significantly > > increase the scope of the patch, as libpq needs to poll the token > > endpoint, setup different API calls, e.t.c. > > - That flow should canonically only be used for clients which can't > > invoke browsers. If it is the only flow to be implemented, it can be > > used in the context when it's not expected by the OAUTH protocol. > > I'm not understanding the concern in the final point -- providers > generally require you to opt into device authorization, at least as far > as I can tell. So if you decide that it's not appropriate for your use > case... don't enable it. (And I haven't seen any claims that opting into > device authorization weakens the other flows in any way. So if we're > going to implement a flow in libpq, I still think device authorization > is the best choice, since it works on headless machines as well as those > with browsers.) > > All of this points at a bigger question to the community: if we choose > not to provide a flow implementation in libpq, is adding OAUTHBEARER > worth the additional maintenance cost? > > My personal vote would be "no". I think the hook-only approach proposed > here would ensure that only larger providers would implement it in > practice, and in that case I'd rather spend cycles on generic SASL. > > > 3. Temporarily removed test suite. We are actively working on aligning > > the tests with the latest changes. Will add a patch with tests soon. > > Okay. Case in point, the following change to the patch appears to be > invalid JSON: > > > + appendStringInfo(&buf, > > + "{ " > > + "\"status\": \"invalid_token\", " > > + "\"openid-configuration\": \"%s\"," > > + "\"scope\": \"%s\" ", > > + "\"issuer\": \"%s\" ", > > + "}", > > Additionally, the "issuer" field added here is not part of the RFC. I've > written my thoughts about unofficial extensions upthread but haven't > received a response, so I'm going to start being more strident: Please, > for the sake of reviewers, call out changes you've made to the spec, and > why they're justified. > > The patches seem to be out of order now (and the documentation in the > commit messages has been removed). > > Thanks, > --Jacob ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PoC] Federated Authn/z with OAUTHBEARER @ 2023-01-25 04:46 mahendrakar s <[email protected]> parent: Andrey Chudnovsky <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: mahendrakar s @ 2023-01-25 04:46 UTC (permalink / raw) To: Andrey Chudnovsky <[email protected]>; +Cc: Jacob Champion <[email protected]>; [email protected]; Michael Paquier <[email protected]>; pgsql-hackers; [email protected] Hi All, The "issuer" field has been removed to align with the RFC implementation - https://www.rfc-editor.org/rfc/rfc7628. This patch "v6" is a single patch to support the OAUTH BEARER token through psql connection string. Below flow is supported. Added the documentation in the commit messages. +----------------------+ +----------+ | +-------+ | Postgres | | PQconnect ->| | | | | | | | +-----------+ | | | ---------- Empty Token---------> | > | | | | libpq | <-- Error(Discovery + Scope ) -- | < | Pre-Auth | | +------+ | | | Hook | | +- < | Hook | | | +-----------+ | | +------+ | | | | v | | | | | [get token]| | | | | | | | | | | + | | | +-----------+ | PQconnect > | | --------- Access Token --------> | > | Validator | | | | <---------- Auth Result -------- | < | Hook | | | | | +-----------+ | +-------+ | | +----------------------+ +----------+ Please note that we are working on modifying/adding new tests (from Jacob's Patch) with the latest changes. Will add a patch with tests soon. Thanks, Mahendrakar. On Wed, 18 Jan 2023 at 07:24, Andrey Chudnovsky <[email protected]> wrote: > > > All of this points at a bigger question to the community: if we choose > > not to provide a flow implementation in libpq, is adding OAUTHBEARER > > worth the additional maintenance cost? > > > My personal vote would be "no". I think the hook-only approach proposed > > here would ensure that only larger providers would implement it in > > practice > > Flow implementations in libpq are definitely a long term plan, and I > agree that it would democratise the adoption. > In the previous posts in this conversation I outlined the ones I think > we should support. > > However, I don't see why it's strictly necessary to couple those. > As long as the SASL exchange for OAUTHBEARER mechanism is supported by > the protocol, the Client side can evolve at its own pace. > > At the same time, the current implementation allows clients to start > building provider-agnostic OAUTH support. By using iddawc or OAUTH > client implementations in the respective platforms. > So I wouldn't refer to "larger providers", but rather "more motivated > clients" here. Which definitely overlaps, but keeps the system open. > > > I'm not understanding the concern in the final point -- providers > > generally require you to opt into device authorization, at least as far > > as I can tell. So if you decide that it's not appropriate for your use > > case... don't enable it. (And I haven't seen any claims that opting into > > device authorization weakens the other flows in any way. So if we're > > going to implement a flow in libpq, I still think device authorization > > is the best choice, since it works on headless machines as well as those > > with browsers.) > I agree with the statement that Device code is the best first choice > if we absolutely have to pick one. > Though I don't think we have to. > > While device flow can be used for all kinds of user-facing > applications, it's specifically designed for input-constrained > scenarios. As clearly stated in the Abstract here - > https://www.rfc-editor.org/rfc/rfc8628 > The authorization code with pkce flow is recommended by the RFSc and > major providers for cases when it's feasible. > The long term goal is to provide both, though I don't see why the > backbone protocol implementation first wouldn't add value. > > Another point is user authentication is one side of the whole story > and the other critical one is system-to-system authentication. Where > we have Client Credentials and Certificates. > With the latter it is much harder to get generically implemented, as > provider-specific tokens need to be signed. > > Adding the other reasoning, I think libpq support for specific flows > can get in the further iterations, after the protocol support. > > > in that case I'd rather spend cycles on generic SASL. > I see 2 approaches to generic SASL: > (a). Generic SASL is a framework used in the protocol, with the > mechanisms implemented on top and exposed to the DBAs as auth types to > configure in hba. > This is the direction we're going here, which is well aligned with the > existing hba-based auth configuration. > (b). Generic SASL exposed to developers on the server- and client- > side to extend on. It seems to be a much longer shot. > The specific points of large ambiguity are libpq distribution model > (which you pointed to) and potential pluggability of insecure > mechanisms. > > I do see (a) as a sweet spot with a lot of value for various > participants with much less ambiguity. > > > Additionally, the "issuer" field added here is not part of the RFC. I've > > written my thoughts about unofficial extensions upthread but haven't > > received a response, so I'm going to start being more strident: Please, > > for the sake of reviewers, call out changes you've made to the spec, and > > why they're justified. > Thanks for your feedback on this. We had this discussion as well, and > added that as a convenience for the client to identify the provider. > I don't see a reason why an issuer would be absolutely necessary, so > we will get your point that sticking to RFCs is a safer choice. > > > The patches seem to be out of order now (and the documentation in the > > commit messages has been removed). > Feedback taken. Work in progress. > > On Tue, Jan 17, 2023 at 2:44 PM Jacob Champion <[email protected]> wrote: > > > > On Sun, Jan 15, 2023 at 12:03 PM Andrey Chudnovsky > > <[email protected]> wrote: > > > 2. Removed Device Code implementation in libpq. Several reasons: > > > - Reduce scope and focus on the protocol first. > > > - Device code implementation uses iddawc dependency. Taking this > > > dependency is a controversial step which requires broader discussion. > > > - Device code implementation without iddaws would significantly > > > increase the scope of the patch, as libpq needs to poll the token > > > endpoint, setup different API calls, e.t.c. > > > - That flow should canonically only be used for clients which can't > > > invoke browsers. If it is the only flow to be implemented, it can be > > > used in the context when it's not expected by the OAUTH protocol. > > > > I'm not understanding the concern in the final point -- providers > > generally require you to opt into device authorization, at least as far > > as I can tell. So if you decide that it's not appropriate for your use > > case... don't enable it. (And I haven't seen any claims that opting into > > device authorization weakens the other flows in any way. So if we're > > going to implement a flow in libpq, I still think device authorization > > is the best choice, since it works on headless machines as well as those > > with browsers.) > > > > All of this points at a bigger question to the community: if we choose > > not to provide a flow implementation in libpq, is adding OAUTHBEARER > > worth the additional maintenance cost? > > > > My personal vote would be "no". I think the hook-only approach proposed > > here would ensure that only larger providers would implement it in > > practice, and in that case I'd rather spend cycles on generic SASL. > > > > > 3. Temporarily removed test suite. We are actively working on aligning > > > the tests with the latest changes. Will add a patch with tests soon. > > > > Okay. Case in point, the following change to the patch appears to be > > invalid JSON: > > > > > + appendStringInfo(&buf, > > > + "{ " > > > + "\"status\": \"invalid_token\", " > > > + "\"openid-configuration\": \"%s\"," > > > + "\"scope\": \"%s\" ", > > > + "\"issuer\": \"%s\" ", > > > + "}", > > > > Additionally, the "issuer" field added here is not part of the RFC. I've > > written my thoughts about unofficial extensions upthread but haven't > > received a response, so I'm going to start being more strident: Please, > > for the sake of reviewers, call out changes you've made to the spec, and > > why they're justified. > > > > The patches seem to be out of order now (and the documentation in the > > commit messages has been removed). > > > > Thanks, > > --Jacob Attachments: [application/x-gzip] v6-0004-common-jsonapi-support-FRONTEND-clients.patch.gz (6.4K, ../../CABkiuWo4fJQ7dhqgYLtJh41kpCkT6iXOO8Eym3Rdh5tx2RJCJw@mail.gmail.com/2-v6-0004-common-jsonapi-support-FRONTEND-clients.patch.gz) download [application/x-gzip] v6-0002-backend-add-OAUTHBEARER-SASL-mechanishm.patch.gz (9.5K, ../../CABkiuWo4fJQ7dhqgYLtJh41kpCkT6iXOO8Eym3Rdh5tx2RJCJw@mail.gmail.com/3-v6-0002-backend-add-OAUTHBEARER-SASL-mechanishm.patch.gz) download [application/x-gzip] v6-0003-Add-a-very-simple-oauth_provider-extension.patch.gz (2.0K, ../../CABkiuWo4fJQ7dhqgYLtJh41kpCkT6iXOO8Eym3Rdh5tx2RJCJw@mail.gmail.com/4-v6-0003-Add-a-very-simple-oauth_provider-extension.patch.gz) download [application/x-gzip] v6-0001-libpq-add-OAUTHBEARER-SASL-mechanism-and-call-back-hooks.patch.gz (9.1K, ../../CABkiuWo4fJQ7dhqgYLtJh41kpCkT6iXOO8Eym3Rdh5tx2RJCJw@mail.gmail.com/5-v6-0001-libpq-add-OAUTHBEARER-SASL-mechanism-and-call-back-hooks.patch.gz) download ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2023-01-25 04:46 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-11 23:36 [PATCH v9 4/9] wal_compression_method: default to zlib.. Justin Pryzby <[email protected]> 2023-01-12 19:08 Re: [PoC] Federated Authn/z with OAUTHBEARER mahendrakar s <[email protected]> 2023-01-15 20:03 ` Re: [PoC] Federated Authn/z with OAUTHBEARER Andrey Chudnovsky <[email protected]> 2023-01-17 22:43 ` Re: [PoC] Federated Authn/z with OAUTHBEARER Jacob Champion <[email protected]> 2023-01-18 01:53 ` Re: [PoC] Federated Authn/z with OAUTHBEARER Andrey Chudnovsky <[email protected]> 2023-01-25 04:46 ` Re: [PoC] Federated Authn/z with OAUTHBEARER mahendrakar s <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox