Fully adopt the upstream OIDC module and serve the SSO sandbox over TLS (#1153)

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>

Author: jviotti

DEPENDENCIES

@@ -1,6 +1,6 @@
 vendorpull https://github.com/sourcemeta/vendorpull 1dcbac42809cf87cb5b045106b863e17ad84ba02
 uwebsockets https://github.com/uNetworking/uWebSockets v20.79.0
-core https://github.com/sourcemeta/core 42d50f3c1e2d63c456fbdf37d18854edab39b5a0
+core https://github.com/sourcemeta/core 99b6a5e8df44d5cdd78a364eac4a059315884f3d
 blaze https://github.com/sourcemeta/blaze ca1949507ea5f4215f9a55ca796cd074602ff705
 jsonbinpack https://github.com/sourcemeta/jsonbinpack f775b2df5fa89d5a70acb940a5c938173811fea7
 jsonschema https://github.com/sourcemeta/jsonschema v16.3.0

docs/configuration.md

@@ -474,7 +474,7 @@ same response as any other unauthenticated request.
 | `/issuer`       | String  | :red_circle: **Yes** | N/A | The token issuer to trust, matched against the `iss` claim |
 | `/audience`     | String  | :red_circle: **Yes** | N/A | The audience this instance identifies as. A token is accepted when its `aud` claim includes this value, so a token minted for several audiences at once is accepted as long as this one is among them |
 | `/algorithms`   | Array   | :red_circle: **Yes** | N/A | The JSON Web Signature algorithms the policy accepts. One or more of `RS256`, `RS384`, `RS512`, `PS256`, `PS384`, `PS512`, `ES256`, `ES384`, `ES512`, and `EdDSA` |
-| `/jwksUri`      | String  | No | Discovered from the issuer | The URL of the issuer's JSON Web Key Set. When omitted, it is discovered from the issuer's OpenID Connect metadata at `{issuer}/.well-known/openid-configuration` |
+| `/jwksUri`      | String  | No | Discovered from the issuer | The URL of the issuer's JSON Web Key Set. When omitted, it is discovered from the issuer's OpenID Connect metadata at `{issuer}/.well-known/openid-configuration`, which requires the issuer to be an `https` URL that publishes a valid OpenID Provider metadata document. Set it explicitly for an issuer that does not meet that bar |
 
 For example, the following instance keeps `/docs` public, gates `/partners`
 behind an API key, and protects `/internal` with a JWT policy that trusts a
@@ -535,7 +535,7 @@ follows is signed with a secret of the instance's own, unrelated to the provider
 | Property        | Type | Required | Default | Description |
 |-----------------|------|----------|---------|-------------|
 | `/title`        | String  | No | The policy name | A human readable version of the policy name |
-| `/issuer`       | String  | :red_circle: **Yes** | N/A | The OpenID Connect issuer to trust, matched against the identity token's `iss` claim and used to discover the provider's metadata, including the signing key set that verifies tokens |
+| `/issuer`       | String  | :red_circle: **Yes** | N/A | The OpenID Connect issuer to trust, matched against the identity token's `iss` claim and used to discover the provider's metadata, including the signing key set that verifies tokens. It must be an `https` URL, as OpenID Connect Discovery requires. Front a provider that only speaks plain HTTP with TLS termination and trust its certificate authority |
 | `/clientId`     | String  | :red_circle: **Yes** | N/A | The client identifier registered with the provider for this instance |
 | `/clientSecret` | Object  | :red_circle: **Yes** | N/A | The client secret shared with the provider, read from an environment variable so that it never lives in the configuration file |
 | `/clientSecret/environmentVariable` | String | :red_circle: **Yes** | N/A | The name of the environment variable that holds the client secret |

enterprise/Dockerfile

@@ -128,14 +128,16 @@ RUN test -f /usr/lib/*/ossl-modules/fips.so
 # Remove packages not needed at runtime to reduce the image attack surface.
 # We keep debconf and perl-base because ca-certificates depends on debconf,
 # which in turn requires perl-base. Purging either removes ca-certificates and
-# wipes the CA bundle that cURL needs to verify TLS certificates
+# wipes the CA bundle that cURL needs to verify TLS certificates. We also keep
+# sed because update-ca-certificates needs it, so images that extend this one
+# can install custom certificate authorities
 RUN cp /bin/mount /bin/umount /tmp/ \
   && apt-get --yes update \
   && apt-get --yes purge --allow-remove-essential \
     adduser \
     e2fsprogs gzip hostname init-system-helpers \
     login logsave mount ncurses-base ncurses-bin passwd \
-    sed sysvinit-utils tzdata util-linux util-linux-extra \
+    sysvinit-utils tzdata util-linux util-linux-extra \
   && apt-get --yes autoremove --allow-remove-essential \
   && rm -rf /var/lib/apt/lists/* \
   && mv /tmp/mount /tmp/umount /bin/

enterprise/authentication/authentication.cc

@@ -5,9 +5,9 @@
 #include <sourcemeta/core/io.h>
 #include <sourcemeta/core/jose.h>
 #include <sourcemeta/core/json.h>
+#include <sourcemeta/core/oidc.h>
 
 #include "authentication_format.h"
-#include "authentication_oidc.h"
 
 #include <algorithm>     // std::ranges::all_of
 #include <bit>           // std::countr_zero
@@ -794,18 +794,28 @@ struct Authentication::Impl {
 
     std::string location;
     if (jwks_uri.empty()) {
-      const auto metadata{this->fetcher_(oidc_discovery_url(issuer))};
+      const auto url{sourcemeta::core::oidc_discovery_url(issuer)};
+      if (!url.has_value()) {
+        return nullptr;
+      }
+
+      const auto metadata{this->fetcher_(url.value())};
       if (!metadata.has_value()) {
         return nullptr;
       }
 
-      auto document{
-          oidc_parse_provider_metadata(metadata.value().body, issuer)};
-      if (!document.has_value() || !document.value().jwks_uri.has_value()) {
+      auto parsed{sourcemeta::core::try_parse_json(metadata.value().body)};
+      if (!parsed.has_value()) {
+        return nullptr;
+      }
+
+      const auto document{sourcemeta::core::OIDCProviderMetadata::from(
+          std::move(parsed).value(), issuer)};
+      if (!document.has_value()) {
         return nullptr;
       }
 
-      location = std::move(document.value().jwks_uri).value();
+      location = document.value().jwks_uri();
     } else {
       location = jwks_uri;
     }

enterprise/authentication/authentication_oidc.h

@@ -1,4 +1,6 @@
 FROM one
+COPY tls/ca.crt /usr/local/share/ca-certificates/sandbox-authority.crt
+RUN update-ca-certificates
 COPY one.json .
 COPY schemas schemas
 RUN sourcemeta one.json --profile

enterprise/e2e/auth-closed/Dockerfile

@@ -1 +1,6 @@
 include ../../../test/e2e/common.mk
+
+# The identity provider serves real TLS under this sandbox's committed
+# authority, so the client trusts that authority and dials the certificate's
+# name through the locally mapped port
+HURL_FLAGS = --cacert tls/ca.crt --connect-to keycloak:8443:localhost:8443

enterprise/e2e/auth-closed/Makefile

@@ -4,9 +4,10 @@ services:
     command: start-dev --import-realm
     environment:
       - KC_HEALTH_ENABLED=true
-      - KC_HOSTNAME=http://keycloak:8080
+      - KC_HOSTNAME=https://keycloak:8443
+      - KC_PROXY_HEADERS=xforwarded
     ports:
-      - "8080:8080"
+      - "8443:8443"
     volumes:
       - ./realm.json:/opt/keycloak/data/import/realm.json:ro
     healthcheck:
@@ -15,6 +16,18 @@ services:
       timeout: 5s
       retries: 60
 
+  # TLS termination sharing the identity provider's network identity, so the
+  # provider answers over https under the very name its issuer advertises
+  keycloak-nginx:
+    image: nginx:1.27-alpine
+    network_mode: service:keycloak
+    volumes:
+      - ./nginx.conf:/etc/nginx/nginx.conf:ro
+      - ./tls:/etc/nginx/tls:ro
+    depends_on:
+      keycloak:
+        condition: service_started
+
   sandbox:
     build:
       context: .
@@ -28,5 +41,7 @@ services:
     depends_on:
       keycloak:
         condition: service_healthy
+      keycloak-nginx:
+        condition: service_started
     ports:
       - "${PORT}:8001"

enterprise/e2e/auth-closed/compose.yml

@@ -45,7 +45,7 @@ GET {{base}}/self/v1/auth/login/keycloak
 HTTP 303
 Cache-Control: no-store
 [Asserts]
-header "Location" startsWith "http://keycloak:8080/realms/main/protocol/openid-connect/auth?"
+header "Location" startsWith "https://keycloak:8443/realms/main/protocol/openid-connect/auth?"
 header "Location" contains "client_id=registry"
 header "Set-Cookie" startsWith "sourcemeta_one_transaction_keycloak="
 header "WWW-Authenticate" not exists

enterprise/e2e/auth-closed/hurl/exempt.all.hurl

@@ -1,5 +1,5 @@
 # A token opens the closed registry just as the root key does
-POST http://localhost:8080/realms/main/protocol/openid-connect/token
+POST https://keycloak:8443/realms/main/protocol/openid-connect/token
 [FormParams]
 grant_type: client_credentials
 client_id: ci-service
@@ -73,7 +73,7 @@ HTTP 200
 jsonpath "$.valid" == true
 
 # A token minted for a different audience opens nothing
-POST http://localhost:8080/realms/main/protocol/openid-connect/token
+POST https://keycloak:8443/realms/main/protocol/openid-connect/token
 [FormParams]
 grant_type: client_credentials
 client_id: ci-wrong