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

Combine the SSO Redirect Servlets #9015

Merged
merged 7 commits into from
Jan 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/9015.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for multiple SSO Identity Providers.
35 changes: 26 additions & 9 deletions synapse/handlers/cas_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ def __init__(self, hs: "HomeServer"):
self._http_client = hs.get_proxied_http_client()

# identifier for the external_ids table
self._auth_provider_id = "cas"
self.idp_id = "cas"

self._sso_handler = hs.get_sso_handler()

self._sso_handler.register_identity_provider(self)

def _build_service_param(self, args: Dict[str, str]) -> str:
"""
Generates a value to use as the "service" parameter when redirecting or
Expand All @@ -105,7 +107,7 @@ async def _validate_ticket(
Args:
ticket: The CAS ticket from the client.
service_args: Additional arguments to include in the service URL.
Should be the same as those passed to `get_redirect_url`.
Should be the same as those passed to `handle_redirect_request`.

Raises:
CasError: If there's an error parsing the CAS response.
Expand Down Expand Up @@ -184,16 +186,31 @@ def _parse_cas_response(self, cas_response_body: bytes) -> CasResponse:

return CasResponse(user, attributes)

def get_redirect_url(self, service_args: Dict[str, str]) -> str:
"""
Generates a URL for the CAS server where the client should be redirected.
async def handle_redirect_request(
self,
request: SynapseRequest,
client_redirect_url: Optional[bytes],
ui_auth_session_id: Optional[str] = None,
) -> str:
"""Generates a URL for the CAS server where the client should be redirected.

Args:
service_args: Additional arguments to include in the final redirect URL.
request: the incoming HTTP request
client_redirect_url: the URL that we should redirect the
client to after login (or None for UI Auth).
ui_auth_session_id: The session ID of the ongoing UI Auth (or
None if this is a login).

Returns:
The URL to redirect the client to.
URL to redirect to
"""

if ui_auth_session_id:
service_args = {"session": ui_auth_session_id}
else:
assert client_redirect_url
service_args = {"redirectUrl": client_redirect_url.decode("utf8")}

args = urllib.parse.urlencode(
{"service": self._build_service_param(service_args)}
)
Expand Down Expand Up @@ -275,7 +292,7 @@ async def _handle_cas_response(
# first check if we're doing a UIA
if session:
return await self._sso_handler.complete_sso_ui_auth_request(
self._auth_provider_id, cas_response.username, session, request,
self.idp_id, cas_response.username, session, request,
)

# otherwise, we're handling a login request.
Expand Down Expand Up @@ -375,7 +392,7 @@ async def grandfather_existing_users() -> Optional[str]:
return None

await self._sso_handler.complete_sso_login_request(
self._auth_provider_id,
self.idp_id,
cas_response.username,
request,
client_redirect_url,
Expand Down
15 changes: 10 additions & 5 deletions synapse/handlers/oidc_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,12 @@ def __init__(self, hs: "HomeServer"):
self._macaroon_secret_key = hs.config.macaroon_secret_key

# identifier for the external_ids table
self._auth_provider_id = "oidc"
self.idp_id = "oidc"

self._sso_handler = hs.get_sso_handler()

self._sso_handler.register_identity_provider(self)

def _validate_metadata(self):
"""Verifies the provider metadata.

Expand Down Expand Up @@ -475,7 +477,7 @@ async def _parse_id_token(self, token: Token, nonce: str) -> UserInfo:
async def handle_redirect_request(
self,
request: SynapseRequest,
client_redirect_url: bytes,
client_redirect_url: Optional[bytes],
ui_auth_session_id: Optional[str] = None,
) -> str:
"""Handle an incoming request to /login/sso/redirect
Expand All @@ -499,7 +501,7 @@ async def handle_redirect_request(
request: the incoming request from the browser.
We'll respond to it with a redirect and a cookie.
client_redirect_url: the URL that we should redirect the client to
when everything is done
when everything is done (or None for UI Auth)
ui_auth_session_id: The session ID of the ongoing UI Auth (or
None if this is a login).

Expand All @@ -511,6 +513,9 @@ async def handle_redirect_request(
state = generate_token()
nonce = generate_token()

if not client_redirect_url:
client_redirect_url = b""

cookie = self._generate_oidc_session_token(
state=state,
nonce=nonce,
Expand Down Expand Up @@ -682,7 +687,7 @@ async def handle_oidc_callback(self, request: SynapseRequest) -> None:
return

return await self._sso_handler.complete_sso_ui_auth_request(
self._auth_provider_id, remote_user_id, ui_auth_session_id, request
self.idp_id, remote_user_id, ui_auth_session_id, request
)

# otherwise, it's a login
Expand Down Expand Up @@ -923,7 +928,7 @@ async def grandfather_existing_users() -> Optional[str]:
extra_attributes = await get_extra_attributes(userinfo, token)

await self._sso_handler.complete_sso_login_request(
self._auth_provider_id,
self.idp_id,
remote_user_id,
request,
client_redirect_url,
Expand Down
25 changes: 18 additions & 7 deletions synapse/handlers/saml_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,27 +73,38 @@ def __init__(self, hs: "HomeServer"):
)

# identifier for the external_ids table
self._auth_provider_id = "saml"
self.idp_id = "saml"

# a map from saml session id to Saml2SessionData object
self._outstanding_requests_dict = {} # type: Dict[str, Saml2SessionData]

self._sso_handler = hs.get_sso_handler()
self._sso_handler.register_identity_provider(self)

def handle_redirect_request(
self, client_redirect_url: bytes, ui_auth_session_id: Optional[str] = None
) -> bytes:
async def handle_redirect_request(
self,
request: SynapseRequest,
client_redirect_url: Optional[bytes],
ui_auth_session_id: Optional[str] = None,
) -> str:
Copy link
Member Author

Choose a reason for hiding this comment

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

it always returned a str, but the declaration was incorrect. Again, it worked because you can request.redirect to a bytes or a str.

"""Handle an incoming request to /login/sso/redirect

Args:
request: the incoming HTTP request
client_redirect_url: the URL that we should redirect the
client to when everything is done
client to after login (or None for UI Auth).
ui_auth_session_id: The session ID of the ongoing UI Auth (or
None if this is a login).

Returns:
URL to redirect to
"""
if not client_redirect_url:
# Some SAML identity providers (e.g. Google) require a
# RelayState parameter on requests, so pass in a dummy redirect URL
# (which will never get used).
client_redirect_url = b"unused"

reqid, info = self._saml_client.prepare_for_authenticate(
entityid=self._saml_idp_entityid, relay_state=client_redirect_url
)
Expand Down Expand Up @@ -210,7 +221,7 @@ async def _handle_authn_response(
return

return await self._sso_handler.complete_sso_ui_auth_request(
self._auth_provider_id,
self.idp_id,
remote_user_id,
current_session.ui_auth_session_id,
request,
Expand Down Expand Up @@ -306,7 +317,7 @@ async def grandfather_existing_users() -> Optional[str]:
return None

await self._sso_handler.complete_sso_login_request(
self._auth_provider_id,
self.idp_id,
remote_user_id,
request,
client_redirect_url,
Expand Down
86 changes: 84 additions & 2 deletions synapse/handlers/sso.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import abc
import logging
from typing import TYPE_CHECKING, Awaitable, Callable, Dict, List, Optional

import attr
from typing_extensions import NoReturn
from typing_extensions import NoReturn, Protocol

from twisted.web.http import Request

from synapse.api.errors import RedirectException, SynapseError
from synapse.api.errors import Codes, RedirectException, SynapseError
from synapse.http.server import respond_with_html
from synapse.http.site import SynapseRequest
from synapse.types import JsonDict, UserID, contains_invalid_mxid_characters
Expand All @@ -40,6 +41,53 @@ class MappingException(Exception):
"""


class SsoIdentityProvider(Protocol):
"""Abstract base class to be implemented by SSO Identity Providers

An Identity Provider, or IdP, is an external HTTP service which authenticates a user
to say whether they should be allowed to log in, or perform a given action.

Synapse supports various implementations of IdPs, including OpenID Connect, SAML,
and CAS.

The main entry point is `handle_redirect_request`, which should return a URI to
redirect the user's browser to the IdP's authentication page.

Each IdP should be registered with the SsoHandler via
`hs.get_sso_handler().register_identity_provider()`, so that requests to
`/_matrix/client/r0/login/sso/redirect` can be correctly dispatched.
"""

@property
@abc.abstractmethod
def idp_id(self) -> str:
"""A unique identifier for this SSO provider

Eg, "saml", "cas", "github"
"""

@abc.abstractmethod
async def handle_redirect_request(
self,
request: SynapseRequest,
client_redirect_url: Optional[bytes],
ui_auth_session_id: Optional[str] = None,
) -> str:
"""Handle an incoming request to /login/sso/redirect

Args:
request: the incoming HTTP request
client_redirect_url: the URL that we should redirect the
client to after login (or None for UI Auth).
ui_auth_session_id: The session ID of the ongoing UI Auth (or
None if this is a login).

Returns:
URL to redirect to
"""
raise NotImplementedError()


@attr.s
class UserAttributes:
# the localpart of the mxid that the mapper has assigned to the user.
Expand Down Expand Up @@ -100,6 +148,14 @@ def __init__(self, hs: "HomeServer"):
# a map from session id to session data
self._username_mapping_sessions = {} # type: Dict[str, UsernameMappingSession]

# map from idp_id to SsoIdentityProvider
self._identity_providers = {} # type: Dict[str, SsoIdentityProvider]

def register_identity_provider(self, p: SsoIdentityProvider):
p_id = p.idp_id
assert p_id not in self._identity_providers
self._identity_providers[p_id] = p

def render_error(
self,
request: Request,
Expand All @@ -124,6 +180,32 @@ def render_error(
)
respond_with_html(request, code, html)

async def handle_redirect_request(
self, request: SynapseRequest, client_redirect_url: bytes,
) -> str:
"""Handle a request to /login/sso/redirect

Args:
request: incoming HTTP request
client_redirect_url: the URL that we should redirect the
client to after login.

Returns:
the URI to redirect to
"""
if not self._identity_providers:
raise SynapseError(
400, "Homeserver not configured for SSO.", errcode=Codes.UNRECOGNIZED
)

# if we only have one auth provider, redirect to it directly
if len(self._identity_providers) == 1:
ap = next(iter(self._identity_providers.values()))
return await ap.handle_redirect_request(request, client_redirect_url)

# otherwise, we have a configuration error
raise Exception("Multiple SSO identity providers have been configured!")
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a slight behavior change, previously if you had multiple SSO providers enabled than we would prioritize CAS, then SAML, then OIDC. I think this was broken / kind of unsupported though so I don't mind making it more explicit. Should we raise this error during register_identity_provider instead of waiting for a request?

I assume there's a master plan here to support pivoting here eventually when there's multiple SSO provider support?

Copy link
Member Author

Choose a reason for hiding this comment

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

yup, a PR is about to arrive! I literally just threw in this exception so that there's something sensible between PRs. I think it's fine for now.


async def get_sso_user_by_remote_user_id(
self, auth_provider_id: str, remote_user_id: str
) -> Optional[str]:
Expand Down
Loading