Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add additional type hints to SSO and registration code #8784

Merged
merged 8 commits into from Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8784.misc
@@ -0,0 +1 @@
Add additional type hints in the registration and SSO handlers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a bugfix now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! I'll re-word this to talk about the bug being fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this although I don't love my wording...

1 change: 1 addition & 0 deletions mypy.ini
Expand Up @@ -37,6 +37,7 @@ files =
synapse/handlers/presence.py,
synapse/handlers/profile.py,
synapse/handlers/read_marker.py,
synapse/handlers/register.py,
synapse/handlers/room.py,
synapse/handlers/room_member.py,
synapse/handlers/room_member_worker.py,
Expand Down
18 changes: 13 additions & 5 deletions synapse/handlers/cas_handler.py
Expand Up @@ -14,7 +14,7 @@
# limitations under the License.
import logging
import urllib
from typing import Dict, Optional, Tuple
from typing import TYPE_CHECKING, Dict, Optional, Tuple
from xml.etree import ElementTree as ET

from twisted.web.client import PartialDownloadError
Expand All @@ -23,6 +23,9 @@
from synapse.http.site import SynapseRequest
from synapse.types import UserID, map_username_to_mxid_localpart

if TYPE_CHECKING:
from synapse.app.homeserver import HomeServer

logger = logging.getLogger(__name__)


Expand All @@ -31,10 +34,10 @@ class CasHandler:
Utility class for to handle the response from a CAS SSO service.

Args:
hs (synapse.server.HomeServer)
hs
"""

def __init__(self, hs):
def __init__(self, hs: "HomeServer"):
self.hs = hs
self._hostname = hs.hostname
self._auth_handler = hs.get_auth_handler()
Expand Down Expand Up @@ -205,11 +208,16 @@ async def handle_ticket(
registered_user_id = await self._auth_handler.check_user_exists(user_id)

if session:
# If there's a session then the user must already exist.
assert registered_user_id

await self._auth_handler.complete_sso_ui_auth(
registered_user_id, session, request,
)

else:
# If this not a UI auth request than there must be a redirect URL.
assert client_redirect_url

if not registered_user_id:
# Pull out the user-agent and IP from the request.
user_agent = request.get_user_agent("")
Expand All @@ -218,7 +226,7 @@ async def handle_ticket(
registered_user_id = await self._registration_handler.register_user(
localpart=localpart,
default_display_name=user_display_name,
user_agent_ips=(user_agent, ip_address),
user_agent_ips=[(user_agent, ip_address)],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix is made to the CAS, OIDC, and SAML handlers and is a bug from #8034.

)

await self._auth_handler.complete_sso_login(
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/oidc_handler.py
Expand Up @@ -925,7 +925,7 @@ async def _map_userinfo_to_user(
registered_user_id = await self._registration_handler.register_user(
localpart=localpart,
default_display_name=attributes["display_name"],
user_agent_ips=(user_agent, ip_address),
user_agent_ips=[(user_agent, ip_address)],
)

await self.store.record_user_external_id(
Expand Down