Copilot commented on code in PR #40024:
URL: https://github.com/apache/superset/pull/40024#discussion_r3236479532
##########
superset/security/api.py:
##########
@@ -134,6 +135,146 @@ def csrf_token(self) -> Response:
"""
return self.response(200, result=generate_csrf())
+ @expose("/auth_providers/", methods=("GET",))
+ @event_logger.log_this
+ @safe
+ @statsd_metrics
+ def auth_providers(self) -> Response:
+ """Get the available SSO auth providers.
+ ---
+ get:
+ summary: Get the SSO auth providers
+ description: >-
+ Returns a list of configured SSO auth providers (OAuth or SAML)
+ with their names, icons, and login URLs. This endpoint does not
+ require authentication and is intended for client applications
+ that need to discover login options before authenticating.
+ responses:
+ 200:
+ description: Result contains the configured auth providers
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ auth_type:
+ type: integer
+ description: >-
+ The FAB auth type constant (1=DB, 2=LDAP,
+ 3=OAUTH, etc.)
+ login_page:
+ type: string
+ description: >-
+ URL of the login page. For SSO flows, open this
+ URL in a browser or web view.
+ browser_login:
+ type: boolean
+ description: >-
+ Whether authentication requires a browser flow
+ (OAuth/SAML). If false, use the
+ /api/v1/security/login endpoint with
+ username/password instead.
+ providers:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ icon:
+ type: string
+ login_url:
+ type: string
+ description: >-
+ Direct login URL for this provider.
+ Open in a browser or web view to start
+ the SSO flow.
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ auth_type = current_app.config.get("AUTH_TYPE")
+ providers: list[dict[str, str]] = []
+
+ # The login page URL is the SPA entry point that handles all auth
flows.
+ # For OAuth/SAML, the React frontend handles the redirect to the IdP.
+ login_page = self.appbuilder.get_url_for_login
+
+ login_base = login_page.rstrip("/")
+
+ if auth_type == AUTH_OAUTH:
+ for provider in self.appbuilder.sm.oauth_providers:
+ name = provider["name"]
+ providers.append(
+ {
+ "name": name,
+ "icon": provider.get("icon", "fa-sign-in"),
+ "login_url": f"{login_base}/{name}",
+ }
+ )
+ elif auth_type == AUTH_SAML:
+ for provider in self.appbuilder.sm.saml_providers:
+ name = provider["name"]
+ providers.append(
+ {
+ "name": name,
+ "icon": provider.get("icon", "fa-sign-in"),
+ "login_url": f"{login_base}/{name}",
+ }
+ )
Review Comment:
The OAUTH and SAML branches are identical except for the source attribute.
Consider extracting the provider source (`self.appbuilder.sm.oauth_providers` /
`saml_providers`) into a single variable and using one loop to build the list.
This removes duplication and reduces the surface area for future
divergence/bugs.
##########
superset/security/api.py:
##########
@@ -134,6 +135,146 @@ def csrf_token(self) -> Response:
"""
return self.response(200, result=generate_csrf())
+ @expose("/auth_providers/", methods=("GET",))
+ @event_logger.log_this
+ @safe
+ @statsd_metrics
+ def auth_providers(self) -> Response:
+ """Get the available SSO auth providers.
+ ---
+ get:
+ summary: Get the SSO auth providers
+ description: >-
+ Returns a list of configured SSO auth providers (OAuth or SAML)
+ with their names, icons, and login URLs. This endpoint does not
+ require authentication and is intended for client applications
+ that need to discover login options before authenticating.
+ responses:
+ 200:
+ description: Result contains the configured auth providers
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ auth_type:
+ type: integer
+ description: >-
+ The FAB auth type constant (1=DB, 2=LDAP,
+ 3=OAUTH, etc.)
+ login_page:
+ type: string
+ description: >-
+ URL of the login page. For SSO flows, open this
+ URL in a browser or web view.
+ browser_login:
+ type: boolean
+ description: >-
+ Whether authentication requires a browser flow
+ (OAuth/SAML). If false, use the
+ /api/v1/security/login endpoint with
+ username/password instead.
+ providers:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ icon:
+ type: string
+ login_url:
+ type: string
+ description: >-
+ Direct login URL for this provider.
+ Open in a browser or web view to start
+ the SSO flow.
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ auth_type = current_app.config.get("AUTH_TYPE")
+ providers: list[dict[str, str]] = []
+
+ # The login page URL is the SPA entry point that handles all auth
flows.
+ # For OAuth/SAML, the React frontend handles the redirect to the IdP.
+ login_page = self.appbuilder.get_url_for_login
+
+ login_base = login_page.rstrip("/")
+
+ if auth_type == AUTH_OAUTH:
+ for provider in self.appbuilder.sm.oauth_providers:
+ name = provider["name"]
+ providers.append(
+ {
+ "name": name,
+ "icon": provider.get("icon", "fa-sign-in"),
+ "login_url": f"{login_base}/{name}",
+ }
+ )
+ elif auth_type == AUTH_SAML:
+ for provider in self.appbuilder.sm.saml_providers:
+ name = provider["name"]
+ providers.append(
+ {
+ "name": name,
+ "icon": provider.get("icon", "fa-sign-in"),
+ "login_url": f"{login_base}/{name}",
+ }
+ )
+
+ # browser_login indicates whether authentication requires a browser
+ # flow (OAuth/SAML SSO) or can be done via the /api/v1/security/login
+ # endpoint with username/password (DB/LDAP).
+ browser_login = auth_type in (AUTH_OAUTH, AUTH_SAML)
+
+ return self.response(
+ 200,
+ auth_type=auth_type,
+ login_page=login_page,
+ browser_login=browser_login,
+ providers=providers,
+ )
+
+ @expose("/session_token/", methods=("GET",))
+ @event_logger.log_this
+ @protect()
+ @safe
+ @statsd_metrics
+ @permission_name("read")
+ def session_token(self) -> Response:
+ """Get a JWT access token from the active web session.
+ ---
+ get:
+ summary: Get a JWT session token
+ description: >-
+ Converts an active browser session (cookie-based) into a JWT
+ access token. Intended for mobile or external applications that
+ complete SSO login via a web view and need a bearer token for
+ subsequent API calls.
+ responses:
+ 200:
+ description: Result contains the JWT access token
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ access_token:
+ type: string
+ 401:
+ $ref: '#/components/responses/401'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ from flask_jwt_extended import create_access_token
Review Comment:
The `create_access_token` import is performed inside the request handler on
every call. Unless there is a known circular import to avoid, prefer moving
this import to the module-level imports at the top of the file for consistency
with the rest of the codebase and to avoid per-request import overhead.
##########
superset/security/api.py:
##########
@@ -134,6 +135,146 @@ def csrf_token(self) -> Response:
"""
return self.response(200, result=generate_csrf())
+ @expose("/auth_providers/", methods=("GET",))
+ @event_logger.log_this
+ @safe
+ @statsd_metrics
+ def auth_providers(self) -> Response:
+ """Get the available SSO auth providers.
+ ---
+ get:
+ summary: Get the SSO auth providers
+ description: >-
+ Returns a list of configured SSO auth providers (OAuth or SAML)
+ with their names, icons, and login URLs. This endpoint does not
+ require authentication and is intended for client applications
+ that need to discover login options before authenticating.
+ responses:
+ 200:
+ description: Result contains the configured auth providers
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ auth_type:
+ type: integer
+ description: >-
+ The FAB auth type constant (1=DB, 2=LDAP,
+ 3=OAUTH, etc.)
+ login_page:
+ type: string
+ description: >-
+ URL of the login page. For SSO flows, open this
+ URL in a browser or web view.
+ browser_login:
+ type: boolean
+ description: >-
+ Whether authentication requires a browser flow
+ (OAuth/SAML). If false, use the
+ /api/v1/security/login endpoint with
+ username/password instead.
+ providers:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ icon:
+ type: string
+ login_url:
+ type: string
+ description: >-
+ Direct login URL for this provider.
+ Open in a browser or web view to start
+ the SSO flow.
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ auth_type = current_app.config.get("AUTH_TYPE")
Review Comment:
`auth_type` is returned to unauthenticated clients as a raw FAB integer
constant (1=DB, 2=LDAP, …), which couples external API consumers to FAB
internals. Consider returning a stable string identifier (e.g., `"db"`,
`"oauth"`, `"saml"`) alongside or instead of the integer so the public contract
is not tied to an upstream library's enum values.
##########
superset/security/api.py:
##########
@@ -134,6 +135,146 @@ def csrf_token(self) -> Response:
"""
return self.response(200, result=generate_csrf())
+ @expose("/auth_providers/", methods=("GET",))
+ @event_logger.log_this
+ @safe
+ @statsd_metrics
+ def auth_providers(self) -> Response:
+ """Get the available SSO auth providers.
+ ---
+ get:
+ summary: Get the SSO auth providers
+ description: >-
+ Returns a list of configured SSO auth providers (OAuth or SAML)
+ with their names, icons, and login URLs. This endpoint does not
+ require authentication and is intended for client applications
+ that need to discover login options before authenticating.
+ responses:
+ 200:
+ description: Result contains the configured auth providers
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ auth_type:
+ type: integer
+ description: >-
+ The FAB auth type constant (1=DB, 2=LDAP,
+ 3=OAUTH, etc.)
+ login_page:
+ type: string
+ description: >-
+ URL of the login page. For SSO flows, open this
+ URL in a browser or web view.
+ browser_login:
+ type: boolean
+ description: >-
+ Whether authentication requires a browser flow
+ (OAuth/SAML). If false, use the
+ /api/v1/security/login endpoint with
+ username/password instead.
+ providers:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ icon:
+ type: string
+ login_url:
+ type: string
+ description: >-
+ Direct login URL for this provider.
+ Open in a browser or web view to start
+ the SSO flow.
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ auth_type = current_app.config.get("AUTH_TYPE")
+ providers: list[dict[str, str]] = []
+
+ # The login page URL is the SPA entry point that handles all auth
flows.
+ # For OAuth/SAML, the React frontend handles the redirect to the IdP.
+ login_page = self.appbuilder.get_url_for_login
+
+ login_base = login_page.rstrip("/")
+
+ if auth_type == AUTH_OAUTH:
+ for provider in self.appbuilder.sm.oauth_providers:
+ name = provider["name"]
+ providers.append(
+ {
+ "name": name,
+ "icon": provider.get("icon", "fa-sign-in"),
+ "login_url": f"{login_base}/{name}",
+ }
+ )
+ elif auth_type == AUTH_SAML:
+ for provider in self.appbuilder.sm.saml_providers:
+ name = provider["name"]
+ providers.append(
+ {
+ "name": name,
+ "icon": provider.get("icon", "fa-sign-in"),
+ "login_url": f"{login_base}/{name}",
+ }
+ )
+
Review Comment:
The `AUTH_SAML` branch is not exercised by any of the new integration tests
(only DB and OAuth are tested). Consider adding a SAML-equivalent test (mocking
`AUTH_TYPE` and `saml_providers`) to ensure this branch continues to work and
to guard against regressions when refactoring the duplicated OAuth/SAML logic.
##########
superset/security/api.py:
##########
@@ -134,6 +135,146 @@ def csrf_token(self) -> Response:
"""
return self.response(200, result=generate_csrf())
+ @expose("/auth_providers/", methods=("GET",))
+ @event_logger.log_this
+ @safe
+ @statsd_metrics
+ def auth_providers(self) -> Response:
+ """Get the available SSO auth providers.
+ ---
+ get:
+ summary: Get the SSO auth providers
+ description: >-
+ Returns a list of configured SSO auth providers (OAuth or SAML)
+ with their names, icons, and login URLs. This endpoint does not
+ require authentication and is intended for client applications
+ that need to discover login options before authenticating.
+ responses:
+ 200:
+ description: Result contains the configured auth providers
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ auth_type:
+ type: integer
+ description: >-
+ The FAB auth type constant (1=DB, 2=LDAP,
+ 3=OAUTH, etc.)
+ login_page:
+ type: string
+ description: >-
+ URL of the login page. For SSO flows, open this
+ URL in a browser or web view.
+ browser_login:
+ type: boolean
+ description: >-
+ Whether authentication requires a browser flow
+ (OAuth/SAML). If false, use the
+ /api/v1/security/login endpoint with
+ username/password instead.
+ providers:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ icon:
+ type: string
+ login_url:
+ type: string
+ description: >-
+ Direct login URL for this provider.
+ Open in a browser or web view to start
+ the SSO flow.
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ auth_type = current_app.config.get("AUTH_TYPE")
+ providers: list[dict[str, str]] = []
+
+ # The login page URL is the SPA entry point that handles all auth
flows.
+ # For OAuth/SAML, the React frontend handles the redirect to the IdP.
+ login_page = self.appbuilder.get_url_for_login
+
+ login_base = login_page.rstrip("/")
+
+ if auth_type == AUTH_OAUTH:
+ for provider in self.appbuilder.sm.oauth_providers:
+ name = provider["name"]
+ providers.append(
+ {
+ "name": name,
+ "icon": provider.get("icon", "fa-sign-in"),
+ "login_url": f"{login_base}/{name}",
+ }
+ )
+ elif auth_type == AUTH_SAML:
+ for provider in self.appbuilder.sm.saml_providers:
+ name = provider["name"]
+ providers.append(
+ {
+ "name": name,
+ "icon": provider.get("icon", "fa-sign-in"),
+ "login_url": f"{login_base}/{name}",
+ }
+ )
+
+ # browser_login indicates whether authentication requires a browser
+ # flow (OAuth/SAML SSO) or can be done via the /api/v1/security/login
+ # endpoint with username/password (DB/LDAP).
+ browser_login = auth_type in (AUTH_OAUTH, AUTH_SAML)
+
+ return self.response(
+ 200,
+ auth_type=auth_type,
+ login_page=login_page,
+ browser_login=browser_login,
+ providers=providers,
+ )
+
+ @expose("/session_token/", methods=("GET",))
+ @event_logger.log_this
+ @protect()
+ @safe
+ @statsd_metrics
+ @permission_name("read")
+ def session_token(self) -> Response:
+ """Get a JWT access token from the active web session.
+ ---
+ get:
+ summary: Get a JWT session token
+ description: >-
+ Converts an active browser session (cookie-based) into a JWT
+ access token. Intended for mobile or external applications that
+ complete SSO login via a web view and need a bearer token for
+ subsequent API calls.
+ responses:
+ 200:
+ description: Result contains the JWT access token
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ access_token:
+ type: string
+ 401:
+ $ref: '#/components/responses/401'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ from flask_jwt_extended import create_access_token
+
+ user = g.user
+ if not user or user.is_anonymous:
+ return self.response_401()
+
Review Comment:
This endpoint is already decorated with `@protect()`, which rejects
unauthenticated requests before the handler runs, so this guard is effectively
unreachable. If you intend to keep it as defense-in-depth, a brief comment
explaining why would help; otherwise it can be removed to reduce noise.
--
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]
