bito-code-review[bot] commented on code in PR #38092:
URL: https://github.com/apache/superset/pull/38092#discussion_r3455967092
##########
docs/admin_docs/security/oauth.mdx:
##########
@@ -0,0 +1,183 @@
+---
+title: OAuth Configuration
+sidebar_position: 4
+---
+
+# Keycloak
+
+## Complete example: Keycloak with PKCE and group mapping
+
+This example demonstrates a complete Keycloak integration with Superset,
including group-to-role mapping, single logout with Keycloak, and PKCE support.
+
+First, create a Secret containing the following values:
+
+* `CLIENT_ID`
+* `CLIENT_SECRET`
+* `OIDC_ISSUER`
+
+The Keycloak client must be configured to support the PKCE flow.
+
+To enable PKCE, set **Proof Key for Code Exchange Code Challenge Method** to
**S256** under:
+
+`Clients -> my_superset_clientid -> Advanced -> Advanced settings`
+
+This implementation correctly handles the case where a user successfully
authenticates with Keycloak but does not belong to any of the configured groups.
+
+The following snippet defines the mapping between Keycloak groups and Superset
roles:
+
+```python
+AUTH_ROLES_MAPPING = {
+ f'{CLIENT_ID}_admin': ['Admin'],
+ f'{CLIENT_ID}_alpha': ['Alpha'],
+ f'{CLIENT_ID}_gamma': ['Gamma'],
+ f'{CLIENT_ID}_public': ['Public'],
+}
+```
+
+If a user does not belong to any of the mapped groups, they will be redirected
back to the login page.
+
+Make sure the following groups exist in Keycloak and that users are assigned
to at least one of them:
+
+* `<my_superset_clientid>_admin`
+* `<my_superset_clientid>_alpha`
+* `<my_superset_clientid>_gamma`
+* `<my_superset_clientid>_public`
+
+## Helm chart configuration
+```yaml
+extraSecretEnv:
+ CLIENT_ID: my_superset_clientid
+ CLIENT_SECRET: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ OIDC_ISSUER: https://mykeycloak.youpi.fr/auth/realms/MYREALM
+
+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:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-20: Unencoded logout redirect URI</b></div>
<div id="fix">
The `post_logout_redirect_uri` parameter value is not URL-encoded.
`request.url_root.strip('/')` may contain special characters (e.g., port, path)
that require encoding per OIDC spec. Use `quote(request.url_root.strip('/'),
safe='')` or construct query params via urlencode. (See also:
[CWE-20](https://cwe.mitre.org/data/definitions/20.html))
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- docs/admin_docs/security/oauth.mdx
+++ docs/admin_docs/security/oauth.mdx
@@ -114,7 +114,8 @@
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}'
+ from urllib.parse import quote
+ return redirect(
+
f"{OIDC_ISSUER}/protocol/openid-connect/logout?post_logout_redirect_uri={quote(request.url_root.strip('/'),
safe='')}&client_id={CLIENT_ID}"
)
```
</div>
</details>
</div>
<small><i>Code Review Run #cffcdd</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
