codeant-ai-for-open-source[bot] commented on code in PR #38092: URL: https://github.com/apache/superset/pull/38092#discussion_r3453876127
########## docs/admin_docs/security/oauth.mdx: ########## @@ -0,0 +1,182 @@ +--- +title: Oauth examples +sidebar_position: 4 +--- + +# Keycloak with PKCE, clean logout and groups mapping + +Here a full example using Keycloak as an OIDC provider with PKCE flow, clean logout and groups mapping to Superset roles. + +You need to create a Secret that contains CLIENT_ID, CLIENT_SECRET and OIDC_ISSUER. + +Keycloak must be configured to support PKCE flow, so you need to set 'Proof Key for Code Exchange Code Challenge Method' to value S256 under: +`Clients -> your-client-id -> Advanced -> Advanced settings` + +Logout is first handled by Superset to clear the session and then the user is redirected to Keycloak logout endpoint to complete the logout process on the OIDC provider side. +It is important to clear the session in Superset first to avoid any issues with the OIDC provider. + +This code correctly handles the case where a user successfully authenticates with Keycloak but does not belong to any of the mapped groups. +In that situation, the user is simply redirected back to the login page. + +## The Secret +You can create the Secret with a manifest like this: +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: superset-oauth-providers + namespace: my-beloved-superset-namespace +stringData: + CLIENT_ID: my_superset_clientid + CLIENT_SECRET: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + OIDC_ISSUER: https://myoauthprovider.youpi.fr/auth/realms/MYREALM +type: Opaque +``` + +and you will need to reference it in your Helm values like this: +```yaml +envFromSecrets: + - superset-oauth-providers +``` + +Or you can directly set the environment variables in the values file like this: +```yaml +extraSecretEnv: + CLIENT_ID: my_superset_clientid + CLIENT_SECRET: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + OIDC_ISSUER: https://myoauthprovider.youpi.fr/auth/realms/MYREALM +``` + +## The config overrides +```yaml +configOverrides: + enable_oauth: | + from flask import redirect, request, session + from flask_appbuilder import expose + from flask_appbuilder.security.manager import AUTH_OAUTH + from flask_appbuilder.security.views import AuthOAuthView + from flask_login import login_user + from superset.security import SupersetSecurityManager + import logging + import os + + log = logging.getLogger(__name__) + + AUTH_TYPE = AUTH_OAUTH + AUTH_USER_REGISTRATION = True + AUTH_ROLES_SYNC_AT_LOGIN = True + AUTH_USER_REGISTRATION_ROLE = None + + PROVIDER_NAME = 'keycloak' + CLIENT_ID = os.getenv('CLIENT_ID') + CLIENT_SECRET = os.getenv('CLIENT_SECRET') + OIDC_ISSUER = os.getenv('OIDC_ISSUER') + OIDC_BASE_URL = f'{OIDC_ISSUER}/protocol/openid-connect' + OIDC_AUTH_URL = f'{OIDC_BASE_URL}/auth' + OIDC_METADATA_URL = f'{OIDC_ISSUER}/.well-known/openid-configuration' + OIDC_TOKEN_URL = f'{OIDC_BASE_URL}/token' + OIDC_USERINFO_URL = f'{OIDC_BASE_URL}/userinfo' + OAUTH_PROVIDERS = [ + { + 'name': PROVIDER_NAME, + 'token_key': 'access_token', + 'icon': 'fa-circle-o', + 'remote_app': { + 'api_base_url': OIDC_BASE_URL, + 'access_token_url': OIDC_TOKEN_URL, + 'authorize_url': OIDC_AUTH_URL, + 'request_token_url': None, + 'server_metadata_url': OIDC_METADATA_URL, + 'client_id': CLIENT_ID, + 'client_secret': CLIENT_SECRET, + 'client_kwargs': { + 'scope': 'openid email profile', + 'code_challenge_method': 'S256', + 'response_type': 'code', + }, + }, + } + ] + + # Make sure you create these groups on Keycloak + AUTH_ROLES_MAPPING = { + f'{CLIENT_ID}_admin': ['Admin'], + f'{CLIENT_ID}_alpha': ['Alpha'], + f'{CLIENT_ID}_gamma': ['Gamma'], + f'{CLIENT_ID}_public': ['Public'], + } + + + class CustomOAuthView(AuthOAuthView): + # Override the logout method to also log out from the OIDC provider + @expose('/logout/', methods=['GET', 'POST']) + def logout(self): + super().logout() + session.clear() + return redirect( + f'{OIDC_ISSUER}/protocol/openid-connect/logout?post_logout_redirect_uri={request.url_root.strip("/")}&client_id={CLIENT_ID}' Review Comment: **Suggestion:** Building `post_logout_redirect_uri` from `request.url_root` makes logout depend on the incoming Host/proxy headers, which often resolves to an internal URL behind ingress and causes Keycloak to reject logout with an invalid redirect URI. Use a fixed externally configured base URL (and URL-encode it) instead of deriving it from each request. [security] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Keycloak logout fails due to invalid redirect URI. - ⚠️ Logout brittle behind load balancer or proxy setups. - ⚠️ Documentation promotes host-derived logout redirect pattern. ``` </details> <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Copy the documented `configOverrides.enable_oauth` snippet from `docs/admin_docs/security/oauth.mdx:50-182` into your Helm values so it is rendered into the running Superset container as configuration (per the Helm `configOverrides` mechanism described in this doc). 2. Deploy Superset with this configuration so that initialization in `superset/initialization/__init__.py:940-949` reads `CUSTOM_SECURITY_MANAGER = CustomSsoSecurityManager` from config (as set at `docs/admin_docs/security/oauth.mdx:180-181`) and assigns `CustomSsoSecurityManager` as `appbuilder.security_manager_class`, which in turn uses `CustomOAuthView` defined at `docs/admin_docs/security/oauth.mdx:110-118` as the OAuth auth view. 3. Log in to Superset via Keycloak, then trigger logout by requesting `/logout/` in the browser; Flask-AppBuilder routes this to `CustomOAuthView.logout()` at `docs/admin_docs/security/oauth.mdx:112-118`, which calls `redirect(f'{OIDC_ISSUER}/protocol/openid-connect/logout?post_logout_redirect_uri={request.url_root.strip("/")}&client_id={CLIENT_ID}')`. 4. Run Superset behind a reverse proxy/ingress where the external URL registered in Keycloak is `https://superset.example.com/` but the WSGI request `url_root` resolves to an internal host like `http://superset.superset.svc.cluster.local/` (a common Kubernetes pattern). On logout, observe Keycloak rejecting the request with an "invalid redirect URI" or similar error because `post_logout_redirect_uri` is built from `request.url_root` (internal host) instead of a fixed, registered external URL, causing the provider logout to fail. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7e7c39686c334b23bf9b0a80db1f04f2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7e7c39686c334b23bf9b0a80db1f04f2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** docs/admin_docs/security/oauth.mdx **Line:** 117:117 **Comment:** *Security: Building `post_logout_redirect_uri` from `request.url_root` makes logout depend on the incoming Host/proxy headers, which often resolves to an internal URL behind ingress and causes Keycloak to reject logout with an invalid redirect URI. Use a fixed externally configured base URL (and URL-encode it) instead of deriving it from each request. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38092&comment_hash=7f7a3c9fb27f1b9644b7f184884a5a2ac50d03d5b5a9ed02ba89150d1776361e&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38092&comment_hash=7f7a3c9fb27f1b9644b7f184884a5a2ac50d03d5b5a9ed02ba89150d1776361e&reaction=dislike'>👎</a> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
