Conversation
There was a problem hiding this comment.
Pull request overview
Integrates Keycloak as an OIDC identity provider for the Traefik-secured workspace deployments and updates docs/config to support the new auth flow.
Changes:
- Adds a Keycloak service to the secure compose files and configures
traefik-forward-authto use OIDC. - Updates environment/documentation to describe Keycloak setup, migration, and quick reference.
- Tweaks the image publishing workflow test matrix/pull logic.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
dtaas/.env.example |
Adds Keycloak-related env vars and reorganizes OAuth configuration comments. |
compose.traefik.secure.yml |
Adds embedded Keycloak service and switches forward-auth provider to OIDC. |
compose.traefik.secure.tls.yml |
Adds embedded Keycloak service and adjusts TLS routing + OIDC forward-auth settings. |
TRAEFIK_SECURE.md |
Updates secure deployment guide to include Keycloak-based auth instructions. |
KEYCLOAK_SETUP.md |
New detailed Keycloak setup guide. |
KEYCLOAK_QUICKREF.md |
New quick reference for Keycloak integration and configuration. |
KEYCLOAK_MIGRATION.md |
New migration guide from GitLab OAuth to Keycloak OIDC. |
CONFIGURATION.md |
Updates configuration docs to recommend Keycloak and provide updated .env guidance. |
CHANGELOG.md |
Records Keycloak integration and documentation additions. |
.github/workflows/workspace-publish.yml |
Adjusts Docker Hub test image selection/override logic. |
| @@ -25,10 +64,10 @@ OAUTH_SECRET=your_random_secret_key_here | |||
|
|
|||
| # Server Configuration | |||
| # Replace with your domain name | |||
| SERVER_DNS=localhost | |||
| SERVER_DNS=https://foo.com | |||
There was a problem hiding this comment.
SERVER_DNS is used in Traefik Host() rules and Keycloak KC_HOSTNAME, so it must be a bare hostname (e.g., "foo.com"), not a URL with scheme. With the current example value "https://foo.com" the generated Host rule will be invalid and routing/auth cookies will likely break.
| traefik-forward-auth: | ||
| image: thomseddon/traefik-forward-auth:2 | ||
| restart: unless-stopped |
There was a problem hiding this comment.
This PR is marked as resolving issue #38, but the secure compose still relies on traefik-forward-auth (it’s only reconfigured to OIDC). Issue #38’s acceptance criteria calls for removing forward-auth and integrating OIDC at the Traefik layer; either update the PR/issue status or implement the removal to avoid closing the issue prematurely.
| @@ -1,6 +1,6 @@ | |||
| services: | |||
| traefik: | |||
| image: traefik:v3.6.4 | |||
| image: traefik:v2.11 | |||
There was a problem hiding this comment.
compose.traefik.secure.tls.yml changes Traefik from v3.6.4 to v2.11, which is a major downgrade and inconsistent with the other compose variants. If there’s no hard requirement for v2, it’s safer to keep versions aligned and pin to a full patch version to avoid unexpected behavior changes.
| image: traefik:v2.11 | |
| image: traefik:v3.6.4 |
| - KC_HOSTNAME_URL=http://${SERVER_DNS:-localhost}:8080/auth | ||
| - KC_HOSTNAME_ADMIN_URL=http://${SERVER_DNS:-localhost}:8080/auth |
There was a problem hiding this comment.
KC_HOSTNAME_URL and KC_HOSTNAME_ADMIN_URL are set to http://${SERVER_DNS}:8080/auth, but externally Keycloak is reached via Traefik on HTTPS (443) and there’s no host port 8080 mapping. This is likely to cause incorrect redirect/issuer URLs. Consider removing these or setting them to the externally reachable URL (typically https://${SERVER_DNS}/auth).
| - KC_HOSTNAME_URL=http://${SERVER_DNS:-localhost}:8080/auth | |
| - KC_HOSTNAME_ADMIN_URL=http://${SERVER_DNS:-localhost}:8080/auth | |
| - KC_HOSTNAME_URL=https://${SERVER_DNS:-localhost}/auth | |
| - KC_HOSTNAME_ADMIN_URL=https://${SERVER_DNS:-localhost}/auth |
| After starting the services, you need to configure Keycloak. See [KEYCLOAK_SETUP.md](KEYCLOAK_SETUP.md) for detailed instructions. | ||
|
|
||
| Quick steps: | ||
| 1. Access Keycloak at `http://foo.com/auth` |
There was a problem hiding this comment.
The guide hard-codes "foo.com" in the quick steps (e.g., Keycloak URL). Since earlier sections reference localhost and the compose uses SERVER_DNS, these examples can be confusing and lead to copy/paste misconfiguration. Consider using ${SERVER_DNS} in examples or consistently using localhost for the secure (non-TLS) guide.
| 1. Access Keycloak at `http://foo.com/auth` | |
| 1. Access Keycloak at `http://${SERVER_DNS}/auth` |
| volumes: | ||
| - keycloak-data:/opt/keycloak/data | ||
| healthcheck: | ||
| test: ["CMD-SHELL", "exec 3<>/dev/tcp/127.0.0.1/8080 || exit 1"] |
There was a problem hiding this comment.
Keycloak healthcheck uses /dev/tcp with CMD-SHELL, which depends on the container shell supporting that feature (typically bash). If /bin/sh doesn’t support /dev/tcp, the healthcheck will always fail and traefik-forward-auth won’t start due to depends_on: condition: service_healthy. Use an HTTP-based check or explicitly run a shell that supports the command.
| test: ["CMD-SHELL", "exec 3<>/dev/tcp/127.0.0.1/8080 || exit 1"] | |
| test: ["CMD", "curl", "-f", "http://127.0.0.1:8080/auth/"] |
| volumes: | ||
| - keycloak-data:/opt/keycloak/data | ||
| healthcheck: | ||
| test: ["CMD-SHELL", "exec 3<>/dev/tcp/127.0.0.1/8080 || exit 1"] |
There was a problem hiding this comment.
Keycloak healthcheck uses /dev/tcp with CMD-SHELL, which may not work under /bin/sh depending on the base image. If it fails, Keycloak will be marked unhealthy (even if serving traffic). Prefer an HTTP-based readiness check or a command that doesn’t rely on /dev/tcp shell support.
| test: ["CMD-SHELL", "exec 3<>/dev/tcp/127.0.0.1/8080 || exit 1"] | |
| test: ["CMD", "curl", "-fsS", "http://127.0.0.1:8080/auth/health/ready"] |
| - AUTH_HOST=${SERVER_DNS:-localhost} | ||
| - COOKIE_DOMAIN=${SERVER_DNS:-localhost} | ||
| - URL_PATH=/_oauth | ||
| - INSECURE_SKIP_VERIFY=true |
There was a problem hiding this comment.
INSECURE_SKIP_VERIFY=true disables TLS certificate verification for OIDC calls, which defeats a key benefit of the TLS compose. If this is only needed for self-signed dev certs, default it to false and document how to enable it explicitly for local testing.
| - INSECURE_SKIP_VERIFY=true | |
| # For local testing with self-signed certificates, run docker compose with INSECURE_SKIP_VERIFY=true | |
| - INSECURE_SKIP_VERIFY=${INSECURE_SKIP_VERIFY:-false} |
|
|
||
| # Secret key for encrypting OAuth session data | ||
| # Generate a random string (at least 16 characters) | ||
| OAUTH_SECRET=$(openssl rand -base64 32) |
There was a problem hiding this comment.
The sample .env snippet recommends setting OAUTH_SECRET to $(openssl rand -base64 32), but .env files do not evaluate shell expressions, so this will become a fixed, well-known secret value. Using a public, non-random OAUTH_SECRET allows attackers who know this configuration to forge or tamper with OAuth session cookies/tokens. Recommend generating a random value with openssl rand -base64 32 (or similar) and pasting the resulting string directly as the OAUTH_SECRET value rather than the command.
| OAUTH_SECRET=$(openssl rand -base64 32) | |
| # Example: run `openssl rand -base64 32` and paste the output here | |
| OAUTH_SECRET=your_random_secret_key_here |
| # Generate a secure secret for OAuth sessions | ||
| OAUTH_SECRET=$(openssl rand -base64 32) |
There was a problem hiding this comment.
The example .env configuration sets OAUTH_SECRET to $(openssl rand -base64 32), but .env files do not perform shell substitution, so this value will be used literally as a static, publicly-known secret. An attacker who knows this configuration can forge or tamper with OAuth session cookies/tokens, since the secret used for signing/encryption is predictable. Instead, generate a random secret with openssl rand -base64 32 (or similar) and paste the resulting value directly as the OAUTH_SECRET literal in the .env file.
| # Generate a secure secret for OAuth sessions | |
| OAUTH_SECRET=$(openssl rand -base64 32) | |
| # Generate a secure secret for OAuth sessions with: | |
| # openssl rand -base64 32 | |
| # Then paste the generated value below as a literal: | |
| OAUTH_SECRET=PASTE_GENERATED_SECRET_HERE |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
- Split up the running of the install scripts in the Docker file into seperate layers make it simpler to differentiate the consequences of the installations. - added "--no-install-recommends" to jupyter and firefox install, to remove unneeded files and services (like locales) - Forced the firefox install to use IPv4 to access launchpad, reducing download time.
|
@DisasterlyDisco The PR #53 is about build time optimization. Please move all the build time optimizations to it. It is better to keep this PR very specific to keycloak integration. Thanks. |
prasadtalasila
left a comment
There was a problem hiding this comment.
@DisasterlyDisco Thanks for the updates. Please see the comments below.
| services: | ||
| traefik: | ||
| image: traefik:v3.6.4 | ||
| image: traefik:v2.11 |
There was a problem hiding this comment.
please keep v3.6.4.
| image: quay.io/keycloak/keycloak:26.0.7 | ||
| restart: unless-stopped | ||
| command: | ||
| - "start-dev" # DEV ONLY - use 'start' with proper config in production |
There was a problem hiding this comment.
can this be set to production environment?
| # Server Configuration | ||
| # Replace with your domain name | ||
| SERVER_DNS=localhost | ||
| SERVER_DNS=https://foo.com |
There was a problem hiding this comment.
this should be bare hostname like copilot suggested
| - frontend | ||
| - users | ||
|
|
||
| keycloak: |
There was a problem hiding this comment.
The compose files must work with both keycloak and external (gitlab-like) oauth2 applications. Given the current size of the docker compose file, the following improvements will help.
- profiles
- include other docker files in this docker file
- extend other docker files
| @@ -1,22 +1,22 @@ | |||
| # Workspace with Traefik Forward Auth (OAuth2 Security) | |||
| # Workspace with Traefik Forward Auth (OIDC/Keycloak Security) | |||
There was a problem hiding this comment.
we need both generic OIDC and keycloak. Please see my comments on compose.traefik.secure.yml for potential ways of achieving this target.
| bash ${INST_DIR}/jupyter/install_jupyter.sh && \ | ||
| bash ${INST_DIR}/admin/install_admin.sh && \ | ||
| bash ${INST_DIR}/dtaas_cleanup.sh | ||
| RUN bash ${INST_DIR}/firefox/install_firefox.sh |
There was a problem hiding this comment.
Is there an advantage to splitting them?
|
@DisasterlyDisco In addition to the comments given before, please fix the GitHub actions as well. Thanks. |
- Upped traefik version - Made traefik and traefik-forward-auth dependent on healthy keycloak - Fixed an error in keycloak nevironment variables
This pull request resolves issue #38