codeant-ai-for-open-source[bot] commented on code in PR #38092:
URL: https://github.com/apache/superset/pull/38092#discussion_r2873336008


##########
docs/admin_docs/installation/kubernetes.mdx:
##########
@@ -308,6 +308,161 @@ configOverrides:
     AUTH_USER_REGISTRATION_ROLE = "Admin"
 ```
 
+:::note
+
+Here another example with groups mapping, logout from Keycloak and PKCE flow.
+
+You need to create a Secret that contains CLIENT_ID, CLIENT_SECRET and 
OIDC_ISSUER.
+
+Your oauth provider must be configured to support PKCE flow.
+
+For keycloak for example, you need to set 'Proof Key for Code Exchange Code 
Challenge Method' to value S256 under:
+`Clients -> your-client-id -> Advanced -> Advanced settings`
+
+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.
+
+:::
+
+```yaml
+extraSecretEnv:
+  CLIENT_ID: my_superset_clientid
+  CLIENT_SECRET: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+  OIDC_ISSUER: https://myoauthprovider.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):
+            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:** The `logout` method in `CustomOAuthView` calls 
`session.clear()` and redirects to the OIDC provider, but it fails to call the 
parent class's `logout()` method (e.g., `super().logout()`). The parent method 
in `AuthOAuthView` handles critical cleanup such as calling Flask-Login's 
`logout_user()` to clear the authentication session and CSRF tokens. Without 
this, the user remains logged in to the Flask application even after the OIDC 
logout, creating a security vulnerability where the local session persists. 
[security]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Security vulnerability: Local Flask session persists after logout.
   - ❌ Users retain authenticated access to Superset after clicking logout.
   - ❌ Missing `logout_user()` call leaves remember-me cookies active.
   ```
   </details>
   
   ```suggestion
           @expose('/logout/', methods=['GET', 'POST'])
           def logout(self):
               session.clear()
               redirect_url = super().logout()
               return redirect(
                   
f'{OIDC_ISSUER}/protocol/openid-connect/logout?post_logout_redirect_uri={request.url_root.strip("/")}&client_id={CLIENT_ID}'
               )
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. User copies the `CustomOAuthView` class from `kubernetes.mdx:393` into 
their
   `superset_config.py` to implement Keycloak logout.
   
   2. User logs into Superset via OAuth (Keycloak) at the `/login/` endpoint, 
establishing a
   Flask-Login session with remember-me tokens.
   
   3. User clicks logout button which routes to `/logout/` handled by
   `CustomOAuthView.logout()` at line 396.
   
   4. The method executes `session.clear()` (line 397) clearing Flask session 
data but
   **fails to call `logout_user()`** from Flask-Login (normally handled by 
parent
   `AuthOAuthView.logout()`).
   
   5. The user is redirected to Keycloak logout (line 398-399), but upon 
returning to
   Superset, the Flask-Login session remains active (authentication 
cookies/tokens persist),
   allowing access to protected resources without re-authentication.
   
   6. **Security Impact**: User appears logged out from OIDC provider but 
retains active
   local session in Superset, creating a session fixation vulnerability where 
local
   application access persists after "logout".
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** docs/admin_docs/installation/kubernetes.mdx
   **Line:** 395:400
   **Comment:**
        *Security: The `logout` method in `CustomOAuthView` calls 
`session.clear()` and redirects to the OIDC provider, but it fails to call the 
parent class's `logout()` method (e.g., `super().logout()`). The parent method 
in `AuthOAuthView` handles critical cleanup such as calling Flask-Login's 
`logout_user()` to clear the authentication session and CSRF tokens. Without 
this, the user remains logged in to the Flask application even after the OIDC 
logout, creating a security vulnerability where the local session persists.
   
   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.
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38092&comment_hash=d766a88b08622e95c5dba3181c0a4c948f23e438a8defe4be86e42142bfedb90&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38092&comment_hash=d766a88b08622e95c5dba3181c0a4c948f23e438a8defe4be86e42142bfedb90&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]

Reply via email to