projects / postgresql.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 45363fc)
oauth: Disallow OAuth connections via postgres_fdw/dblink
authorJacob Champion <jchampion@postgresql.org>
Tue, 29 Apr 2025 20:08:24 +0000 (13:08 -0700)
committerJacob Champion <jchampion@postgresql.org>
Tue, 29 Apr 2025 20:08:24 +0000 (13:08 -0700)
A subsequent commit will reclassify oauth_client_secret from dispchar=""
to dispchar="*", so that UIs will treat it like a secret. For our FDWs,
this change will move that option from SERVER to USER MAPPING, which we
need to avoid.

But upon further discussion, we don't really want our FDWs to use our
builtin Device Authorization flow at all, for several reasons:

- the URL and code would be printed to the server logs, not sent over
  the client connection
- tokens are not cached/refreshed, so every single connection has to be
  manually authorized by a user with a browser
- oauth_client_secret needs to belong to the foreign server, but options
  on SERVER are publicly accessible
- all non-superusers would need password_required=false, which is
  dangerous

Future OAuth work can use FDWs as a motivating use case. But for now,
disallow all oauth_* connection options for these two extensions.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20250415191435.55.nmisch%40google.com

contrib/dblink/dblink.c patch | blob | blame | history
contrib/dblink/expected/dblink.out patch | blob | blame | history
contrib/dblink/sql/dblink.sql patch | blob | blame | history
contrib/postgres_fdw/expected/postgres_fdw.out patch | blob | blame | history
contrib/postgres_fdw/option.c patch | blob | blame | history
contrib/postgres_fdw/sql/postgres_fdw.sql patch | blob | blame | history

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 092f0753ff585b1cefbefd4613f5aba6b49d6f90..1b2d72c6def34a78a84c19edd5e543e16e3130b7 100644 (file)
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -3094,6 +3094,13 @@ is_valid_dblink_option(const PQconninfoOption *options, const char *option,
    if (strcmp(opt->keyword, "client_encoding") == 0)
        return false;
 
+   /*
+    * Disallow OAuth options for now, since the builtin flow communicates on
+    * stderr by default and can't cache tokens yet.
+    */
+   if (strncmp(opt->keyword, "oauth_", strlen("oauth_")) == 0)
+       return false;
+
    /*
     * If the option is "user" or marked secure, it should be specified only
     * in USER MAPPING.  Others should be specified only in SERVER.
diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out
index 7809f58d96b5f45b4e365e1fee2a831671d1712e..c70c79574fd1d17e1fb161b0865fe9480719cc4d 100644 (file)
--- a/contrib/dblink/expected/dblink.out
+++ b/contrib/dblink/expected/dblink.out
@@ -898,6 +898,17 @@ CREATE USER MAPPING FOR public SERVER fdtest
   OPTIONS (server 'localhost');  -- fail, can't specify server here
 ERROR:  invalid option "server"
 CREATE USER MAPPING FOR public SERVER fdtest OPTIONS (user :'USER');
+-- OAuth options are not allowed in either context
+ALTER SERVER fdtest OPTIONS (ADD oauth_issuer 'https://example.com');
+ERROR:  invalid option "oauth_issuer"
+ALTER SERVER fdtest OPTIONS (ADD oauth_client_id 'myID');
+ERROR:  invalid option "oauth_client_id"
+ALTER USER MAPPING FOR public SERVER fdtest
+   OPTIONS (ADD oauth_issuer 'https://example.com');
+ERROR:  invalid option "oauth_issuer"
+ALTER USER MAPPING FOR public SERVER fdtest
+   OPTIONS (ADD oauth_client_id 'myID');
+ERROR:  invalid option "oauth_client_id"
 GRANT USAGE ON FOREIGN SERVER fdtest TO regress_dblink_user;
 GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO regress_dblink_user;
 SET SESSION AUTHORIZATION regress_dblink_user;
diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql
index 7870ce5d5a43dc0a5b2c6925dc1c7d4b55b6f50f..365b21036e85420b662854ac6c948f83b403e511 100644 (file)
--- a/contrib/dblink/sql/dblink.sql
+++ b/contrib/dblink/sql/dblink.sql
@@ -469,6 +469,14 @@ CREATE USER MAPPING FOR public SERVER fdtest
   OPTIONS (server 'localhost');  -- fail, can't specify server here
 CREATE USER MAPPING FOR public SERVER fdtest OPTIONS (user :'USER');
 
+-- OAuth options are not allowed in either context
+ALTER SERVER fdtest OPTIONS (ADD oauth_issuer 'https://example.com');
+ALTER SERVER fdtest OPTIONS (ADD oauth_client_id 'myID');
+ALTER USER MAPPING FOR public SERVER fdtest
+   OPTIONS (ADD oauth_issuer 'https://example.com');
+ALTER USER MAPPING FOR public SERVER fdtest
+   OPTIONS (ADD oauth_client_id 'myID');
+
 GRANT USAGE ON FOREIGN SERVER fdtest TO regress_dblink_user;
 GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO regress_dblink_user;
 
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index d1acee5a5fa516d03fcfae5d99f565ebbfe621ea..24ff5f70cceabbc4aadad12b3dab7ca78baed9c3 100644 (file)
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -196,6 +196,17 @@ ALTER USER MAPPING FOR public SERVER testserver1
 -- permitted to check validation.
 ALTER USER MAPPING FOR public SERVER testserver1
    OPTIONS (ADD sslkey 'value', ADD sslcert 'value');
+-- OAuth options are not allowed in either context
+ALTER SERVER testserver1 OPTIONS (ADD oauth_issuer 'https://example.com');
+ERROR:  invalid option "oauth_issuer"
+ALTER SERVER testserver1 OPTIONS (ADD oauth_client_id 'myID');
+ERROR:  invalid option "oauth_client_id"
+ALTER USER MAPPING FOR public SERVER testserver1
+   OPTIONS (ADD oauth_issuer 'https://example.com');
+ERROR:  invalid option "oauth_issuer"
+ALTER USER MAPPING FOR public SERVER testserver1
+   OPTIONS (ADD oauth_client_id 'myID');
+ERROR:  invalid option "oauth_client_id"
 ALTER FOREIGN TABLE ft1 OPTIONS (schema_name 'S 1', table_name 'T 1');
 ALTER FOREIGN TABLE ft2 OPTIONS (schema_name 'S 1', table_name 'T 1');
 ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 OPTIONS (column_name 'C 1');
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index d0766f007d2f9ab2f565ae59202b3b98321fc2ed..c2f936640bca865ab4054b148775cdb032abb266 100644 (file)
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -348,6 +348,13 @@ InitPgFdwOptions(void)
            strcmp(lopt->keyword, "client_encoding") == 0)
            continue;
 
+       /*
+        * Disallow OAuth options for now, since the builtin flow communicates
+        * on stderr by default and can't cache tokens yet.
+        */
+       if (strncmp(lopt->keyword, "oauth_", strlen("oauth_")) == 0)
+           continue;
+
        /* We don't have to copy keyword string, as described above. */
        popt->keyword = lopt->keyword;
 
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index ea6287b03fd93d97324dd50402513d2febb2d36b..1f27260bafe1709aba24c64c7b08203818457fb9 100644 (file)
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -213,6 +213,14 @@ ALTER USER MAPPING FOR public SERVER testserver1
 ALTER USER MAPPING FOR public SERVER testserver1
    OPTIONS (ADD sslkey 'value', ADD sslcert 'value');
 
+-- OAuth options are not allowed in either context
+ALTER SERVER testserver1 OPTIONS (ADD oauth_issuer 'https://example.com');
+ALTER SERVER testserver1 OPTIONS (ADD oauth_client_id 'myID');
+ALTER USER MAPPING FOR public SERVER testserver1
+   OPTIONS (ADD oauth_issuer 'https://example.com');
+ALTER USER MAPPING FOR public SERVER testserver1
+   OPTIONS (ADD oauth_client_id 'myID');
+
 ALTER FOREIGN TABLE ft1 OPTIONS (schema_name 'S 1', table_name 'T 1');
 ALTER FOREIGN TABLE ft2 OPTIONS (schema_name 'S 1', table_name 'T 1');
 ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 OPTIONS (column_name 'C 1');
This is the main PostgreSQL git repository.