From 77eef5b1ad0863232a19545451d8fe24bc578936 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 21 Jul 2022 12:46:13 -0400 Subject: [PATCH] types: Add OIDCIdPConfigDict. The presence of `auto_signup` in idp_settings_dict in the test case test_social_auth_registration_auto_signup is incompatible with the previous type annotation of SOCIAL_AUTH_OIDC_ENABLED_IDPS, where `bool` is not allowed. Signed-off-by: Zixuan James Li --- zerver/lib/types.py | 9 +++++++++ zproject/backends.py | 12 +++++------- zproject/default_settings.py | 4 ++-- zproject/prod_settings_template.py | 2 +- zproject/test_extra_settings.py | 4 ++-- 5 files changed, 19 insertions(+), 12 deletions(-) diff --git a/zerver/lib/types.py b/zerver/lib/types.py index 52cb43cf5e..4cef9ddd5c 100644 --- a/zerver/lib/types.py +++ b/zerver/lib/types.py @@ -82,6 +82,15 @@ class SAMLIdPConfigDict(TypedDict, total=False): x509cert_path: str +class OIDCIdPConfigDict(TypedDict, total=False): + oidc_url: str + display_name: str + display_icon: Optional[str] + client_id: str + secret: Optional[str] + auto_signup: bool + + class UnspecifiedValue: """In most API endpoints, we use a default value of `None"` to encode parameters that the client did not pass, which is nicely Pythonic. diff --git a/zproject/backends.py b/zproject/backends.py index 890d08a552..38abf64b3e 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -87,7 +87,7 @@ from zerver.lib.redis_utils import get_dict_from_redis, get_redis_client, put_di from zerver.lib.request import RequestNotes from zerver.lib.sessions import delete_user_sessions from zerver.lib.subdomains import get_subdomain -from zerver.lib.types import ProfileDataElementUpdateDict +from zerver.lib.types import OIDCIdPConfigDict, ProfileDataElementUpdateDict from zerver.lib.url_encoding import append_url_query_string from zerver.lib.users import check_full_name, validate_user_custom_profile_field from zerver.models import ( @@ -2675,14 +2675,12 @@ class GenericOpenIdConnectBackend(SocialAuthMixin, OpenIdConnectAuth): # Hack: We don't yet support multiple IdPs, but we want this # module to import if nothing has been configured yet. - settings_dict: Dict[str, Union[Optional[str], bool]] = list( - settings.SOCIAL_AUTH_OIDC_ENABLED_IDPS.values() or [{}] + settings_dict: OIDCIdPConfigDict = list( + settings.SOCIAL_AUTH_OIDC_ENABLED_IDPS.values() or [OIDCIdPConfigDict()] )[0] - display_icon: Optional[str] = cast(Optional[str], settings_dict.get("display_icon", None)) - assert isinstance(display_icon, (str, type(None))) - display_name: str = cast(str, settings_dict.get("display_name", "OIDC")) - assert isinstance(display_name, str) + display_icon: Optional[str] = settings_dict.get("display_icon", None) + display_name: str = settings_dict.get("display_name", "OIDC") full_name_validated = getattr(settings, "SOCIAL_AUTH_OIDC_FULL_NAME_VALIDATED", False) diff --git a/zproject/default_settings.py b/zproject/default_settings.py index 66e2ee5f17..89e33428b5 100644 --- a/zproject/default_settings.py +++ b/zproject/default_settings.py @@ -9,7 +9,7 @@ from .config import DEVELOPMENT, PRODUCTION, get_secret if TYPE_CHECKING: from django_auth_ldap.config import LDAPSearch - from zerver.lib.types import SAMLIdPConfigDict + from zerver.lib.types import OIDCIdPConfigDict, SAMLIdPConfigDict if PRODUCTION: from .prod_settings import EXTERNAL_HOST, ZULIP_ADMINISTRATOR @@ -102,7 +102,7 @@ SOCIAL_AUTH_APPLE_SCOPE = ["name", "email"] SOCIAL_AUTH_APPLE_EMAIL_AS_USERNAME = True # Generic OpenID Connect: -SOCIAL_AUTH_OIDC_ENABLED_IDPS: Dict[str, Dict[str, Optional[str]]] = {} +SOCIAL_AUTH_OIDC_ENABLED_IDPS: Dict[str, "OIDCIdPConfigDict"] = {} SOCIAL_AUTH_OIDC_FULL_NAME_VALIDATED = False SOCIAL_AUTH_SYNC_CUSTOM_ATTRS_DICT: Dict[str, Dict[str, Dict[str, str]]] = {} diff --git a/zproject/prod_settings_template.py b/zproject/prod_settings_template.py index 023f7fed13..29d62a755d 100644 --- a/zproject/prod_settings_template.py +++ b/zproject/prod_settings_template.py @@ -352,7 +352,7 @@ AUTH_LDAP_USER_ATTR_MAP = { ## https://zulip.readthedocs.io/en/latest/production/authentication-methods.html#openid-connect ## -SOCIAL_AUTH_OIDC_ENABLED_IDPS = { +SOCIAL_AUTH_OIDC_ENABLED_IDPS: Dict[str, Any] = { ## This field (example: "idp_name") may appear in URLs during ## authentication, but is otherwise not user-visible. "idp_name": { diff --git a/zproject/test_extra_settings.py b/zproject/test_extra_settings.py index 175a86c408..f570cd3af8 100644 --- a/zproject/test_extra_settings.py +++ b/zproject/test_extra_settings.py @@ -5,7 +5,7 @@ import ldap from django_auth_ldap.config import LDAPSearch from zerver.lib.db import TimeTrackingConnection, TimeTrackingCursor -from zerver.lib.types import SAMLIdPConfigDict, SCIMConfigDict +from zerver.lib.types import OIDCIdPConfigDict, SAMLIdPConfigDict, SCIMConfigDict from .config import DEPLOY_ROOT, get_from_file_if_exists from .settings import ( @@ -194,7 +194,7 @@ APPLE_ID_TOKEN_GENERATION_KEY = get_from_file_if_exists( "zerver/tests/fixtures/apple/token_gen_private_key" ) -SOCIAL_AUTH_OIDC_ENABLED_IDPS = { +SOCIAL_AUTH_OIDC_ENABLED_IDPS: Dict[str, OIDCIdPConfigDict] = { "testoidc": { "display_name": "Test OIDC", "oidc_url": "https://example.com/api/openid",