rusackas commented on code in PR #41635:
URL: https://github.com/apache/superset/pull/41635#discussion_r3964669683
##########
tests/unit_tests/security/manager_test.py:
##########
@@ -49,6 +53,94 @@ def test_security_manager(app_context: None) -> None:
assert sm
+def test_superset_oauth_view_oauth_oauth_single_provider(
Review Comment:
good catch, fixed the duplicated `oauth` in the test name (`b60bc73`).
##########
tests/unit_tests/security/manager_test.py:
##########
@@ -49,6 +53,94 @@ def test_security_manager(app_context: None) -> None:
assert sm
+def test_superset_oauth_view_oauth_oauth_single_provider(
+ mocker: MockerFixture, app: Flask, app_context: None
+) -> None:
+ """
+ Test that SupersetOAuthView auto-selects the provider and delegates
+ to the parent login method when only a single OAuth provider is configured.
+ """
+ mock_provider = MagicMock()
+ mock_remotes = {"google": mock_provider}
+
+ mocker.patch.object(app.appbuilder.sm, "oauth_remotes", mock_remotes)
+
+ mock_super_login = mocker.patch(
+ "superset.views.auth.AuthOAuthView.login",
return_value="mocked_response"
+ )
+
+ oauth_view = SupersetOAuthView()
+
+ oauth_view.appbuilder = app.appbuilder
+
+ oauth_view.login()
+
+ mock_super_login.assert_called_once_with("google")
+
+
+def test_superset_oauth_view_oauth_multiple_providers(
+ mocker: MockerFixture, app: Flask, app_context: None
+) -> None:
+ """
+ Test that SupersetOAuthView does NOT auto-select when multiple OAuth
+ providers are configured, and instead calls parent login without provider.
+ """
+ mock_provider1 = MagicMock()
+ mock_provider2 = MagicMock()
+ mock_remotes = {"google": mock_provider1, "github": mock_provider2}
+
+ mocker.patch.object(app.appbuilder.sm, "oauth_remotes", mock_remotes)
+
+ mock_super_login = mocker.patch(
+ "superset.views.auth.AuthOAuthView.login",
return_value="mocked_response"
+ )
+
+ oauth_view = SupersetOAuthView()
+
+ oauth_view.appbuilder = app.appbuilder
+
+ oauth_view.login()
+
+ mock_super_login.assert_called_once_with(None)
+
+
+def test_security_manager_with_oauth_auth_type(
+ mocker: MockerFixture, app: Flask, app_context: None
+) -> None:
+ """
+ Test that SupersetSecurityManager login works correctly when
+ AUTH_TYPE is set to AUTH_OAUTH with a single provider.
+ """
+ from flask_appbuilder.security.manager import AUTH_OAUTH
+
+ app.config["AUTH_TYPE"] = AUTH_OAUTH
+ app.config["OAUTH_PROVIDERS"] = [
+ {
+ "name": "google",
+ "icon": "fa-google",
+ "token_key": "access_token",
+ "remote_app": {
+ "client_id": "test_client_id",
+ "client_secret": "test_client_secret",
+ "api_base_url": "https://accounts.google.com/o/oauth2/v2/auth";,
+ "client_kwargs": {"scope": "email profile"},
+ },
+ }
+ ]
+
+ mock_provider = mocker.Mock()
+ mock_remotes = {"google": mock_provider}
+ mocker.patch.object(app.appbuilder.sm, "oauth_remotes", mock_remotes)
Review Comment:
you're right, this one was misleading. `app` is a module-scoped fixture, so
the security manager is already built before this test sets
`AUTH_TYPE`/`OAUTH_PROVIDERS` on `app.config` -- those lines never do anything,
and the test then patches `oauth_remotes` directly like the single-provider
test above it already does. Removed it as a dupe rather than trying to make it
exercise real config-driven init, since that'd mean re-initializing the whole
appbuilder stack for marginal extra coverage (`b60bc73`).
--
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]
