-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add OAuth2 support #7059
Add OAuth2 support #7059
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
It is now possible to register and login in using the OAuth2 Authorization Code Flow. Contributed by Max Klenk. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
# -*- coding: utf-8 -*- | ||
# Copyright 2015, 2016 OpenMarket Ltd | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. /me checks calendar. Also please don't assign copyright for new code to OpenMarket unless you work for them? You can either put your own name here or omit the copyright line altogether. |
||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# 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. | ||
|
||
from ._base import Config | ||
|
||
|
||
class OAuth2Config(Config): | ||
"""OAuth2 Configuration | ||
|
||
oauth_server_url: URL of OAuth2 server | ||
""" | ||
|
||
section = "oauth2" | ||
|
||
def read_config(self, config, **kwargs): | ||
oauth2_config = config.get("oauth2_config", None) | ||
if oauth2_config: | ||
self.oauth2_enabled = oauth2_config.get("enabled", True) | ||
self.oauth2_server_authorization_url = oauth2_config[ | ||
"server_authorization_url" | ||
] | ||
self.oauth2_server_token_url = oauth2_config["server_token_url"] | ||
self.oauth2_server_userinfo_url = oauth2_config["server_userinfo_url"] | ||
self.oauth2_client_id = oauth2_config["client_id"] | ||
self.oauth2_client_secret = oauth2_config["client_secret"] | ||
else: | ||
self.oauth2_enabled = False | ||
self.oauth2_server_authorization_url = None | ||
self.oauth2_server_token_url = None | ||
self.oauth2_server_userinfo_url = None | ||
self.oauth2_client_id = None | ||
self.oauth2_client_secret = None | ||
|
||
def generate_config_section(self, config_dir_path, server_name, **kwargs): | ||
return """ | ||
# Enable OAuth2 for registration and login. | ||
# | ||
#oauth2_config: | ||
# enabled: true | ||
# server_authorization_url: "https://oauth.server.com/oauth2/authorize" | ||
# server_token_url: "https://oauth.server.com/oauth2/token" | ||
# server_userinfo_url: "https://oauth.server.com/oauth2/userinfo" | ||
# client_id: "FORM_OAUTH_SERVER" | ||
# client_secret: "FORM_OAUTH_SERVER" | ||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,154 @@ | ||
# -*- coding: utf-8 -*- | ||
# Copyright 2019 The Matrix.org Foundation C.I.C. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. likewise, it's not really sensible to put Matrix.org here unless you work for them. |
||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# 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 logging | ||
|
||
from six.moves import urllib | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't need to support python 2 any more; please use |
||
|
||
from twisted.web.client import PartialDownloadError | ||
|
||
from synapse.api.errors import Codes, LoginError | ||
from synapse.http.servlet import parse_string | ||
from synapse.rest.client.v1.login import SSOAuthHandler | ||
from synapse.util.caches.expiringcache import ExpiringCache | ||
from synapse.util.stringutils import random_string | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class OAuth2Handler: | ||
def __init__(self, hs): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be great if you could add typing annotations (see PEP-484) for the params and return types of all the new methods. |
||
# config | ||
self.public_baseurl = hs.config.public_baseurl.encode("ascii") | ||
self.oauth2_server_authorization_url = hs.config.oauth2_server_authorization_url.encode( | ||
"ascii" | ||
) | ||
self.oauth2_server_token_url = hs.config.oauth2_server_token_url | ||
self.oauth2_server_userinfo_url = hs.config.oauth2_server_userinfo_url | ||
self.oauth2_client_id = hs.config.oauth2_client_id | ||
self.oauth2_client_secret = hs.config.oauth2_client_secret | ||
self.oauth2_scope = "openid" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are these object-level fields rather than file-level constants? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The response_type and response_mode should be hardcoded as the code flow is the only one implemented yet. The scope on the other hand should be configurable. |
||
self.oauth2_response_type = "code" | ||
self.oauth2_response_mode = "query" | ||
|
||
# state | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please can you document the structure of this. what does it map from and to? |
||
self.nonces = ExpiringCache( | ||
cache_name="oauth_nonces", | ||
clock=hs.get_clock(), | ||
expiry_ms=5 * 60 * 1000, # 5 minutes | ||
reset_expiry_on_get=False, | ||
) | ||
|
||
# tools | ||
self._sso_auth_handler = SSOAuthHandler(hs) | ||
self._http_client = hs.get_proxied_http_client() | ||
|
||
def handle_redirect_request(self, client_redirect_url): | ||
"""Handle an incoming request to /login/sso/redirect | ||
|
||
Args: | ||
client_redirect_url (bytes): the URL that we should redirect the | ||
client to when everything is done | ||
|
||
Returns: | ||
bytes: URL to redirect to | ||
""" | ||
|
||
oauth2_nonce = random_string(12) | ||
self.nonces[oauth2_nonce] = {"redirectUrl": client_redirect_url} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the |
||
|
||
service_param = urllib.parse.urlencode( | ||
{ | ||
b"redirect_uri": self.get_server_redirect_url(), | ||
b"client_id": self.oauth2_client_id, | ||
b"scope": self.oauth2_scope, | ||
b"response_type": self.oauth2_response_type, | ||
b"response_mode": self.oauth2_response_mode, | ||
b"state": oauth2_nonce, | ||
} | ||
).encode("ascii") | ||
return b"%s?%s" % (self.oauth2_server_authorization_url, service_param) | ||
|
||
async def handle_oauth2_response(self, request): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also handle oauth2 errors |
||
"""Handle an incoming request to /_matrix/oauth2/response | ||
|
||
Args: | ||
request (SynapseRequest): the incoming request from the browser. We'll | ||
respond to it with a redirect. | ||
|
||
Returns: | ||
Deferred[none]: Completes once we have handled the request. | ||
""" | ||
oauth2_code = parse_string(request, "code", required=True) | ||
oauth2_state = parse_string(request, "state", required=False) | ||
|
||
# validate state | ||
if oauth2_state not in self.nonces: | ||
raise LoginError( | ||
400, "Invalid or expire state passed", errcode=Codes.UNAUTHORIZED | ||
) | ||
|
||
client_redirect_url = self.nonces[oauth2_state].pop("redirectUrl").decode() | ||
logging.warning(client_redirect_url) | ||
|
||
access_token = await self.get_access_token(oauth2_code) | ||
userinfo = await self.get_userinfo(access_token) | ||
|
||
user = "sso_" + userinfo.get("sub") | ||
displayname = userinfo.get("preferred_username") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That should not be hard-coded. Also, only the |
||
|
||
result = await self._sso_auth_handler.on_successful_auth( | ||
user, request, client_redirect_url, displayname | ||
) | ||
return result | ||
|
||
async def get_access_token(self, oauth2_code): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for internal methods like this, please could you prefix them with (alternatively, if they form part of the public API of the class, they need docstrings.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (tbh it could do with a docstring anyway: in this case it should clarify that this method is fetching the OAuth2 access token rather than a matrix access token.) |
||
args = { | ||
"client_id": self.oauth2_client_id, | ||
"client_secret": self.oauth2_client_secret, | ||
"code": oauth2_code, | ||
"grant_type": "authorization_code", | ||
"redirect_uri": self.get_server_redirect_url(), | ||
} | ||
|
||
try: | ||
body = await self._http_client.post_urlencoded_get_json( | ||
self.oauth2_server_token_url, args | ||
) | ||
except PartialDownloadError as pde: | ||
# Twisted raises this error if the connection is closed, | ||
# even if that's being used old-http style to signal end-of-data | ||
body = pde.response | ||
|
||
access_token = body.get("access_token") | ||
return access_token | ||
|
||
async def get_userinfo(self, access_token): | ||
headers = { | ||
"Authorization": ["Bearer " + access_token], | ||
} | ||
|
||
try: | ||
userinfo = await self._http_client.get_json( | ||
self.oauth2_server_userinfo_url, {}, headers | ||
) | ||
except PartialDownloadError as pde: | ||
# Twisted raises this error if the connection is closed, | ||
# even if that's being used old-http style to signal end-of-data | ||
userinfo = pde.response | ||
|
||
return userinfo | ||
|
||
def get_server_redirect_url(self): | ||
return self.public_baseurl + b"_matrix/client/r0/login/oauth/response" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we not store this in a field in the constructor, rather than having to call a method? |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,6 +87,7 @@ def __init__(self, hs): | |
self.jwt_secret = hs.config.jwt_secret | ||
self.jwt_algorithm = hs.config.jwt_algorithm | ||
self.saml2_enabled = hs.config.saml2_enabled | ||
self.oauth2_enabled = hs.config.oauth2_enabled | ||
self.cas_enabled = hs.config.cas_enabled | ||
self.auth_handler = self.hs.get_auth_handler() | ||
self.registration_handler = hs.get_registration_handler() | ||
|
@@ -104,6 +105,9 @@ def on_GET(self, request): | |
if self.saml2_enabled: | ||
flows.append({"type": LoginRestServlet.SSO_TYPE}) | ||
flows.append({"type": LoginRestServlet.TOKEN_TYPE}) | ||
if self.oauth2_enabled: | ||
flows.append({"type": LoginRestServlet.SSO_TYPE}) | ||
flows.append({"type": LoginRestServlet.TOKEN_TYPE}) | ||
if self.cas_enabled: | ||
flows.append({"type": LoginRestServlet.SSO_TYPE}) | ||
|
||
|
@@ -425,6 +429,26 @@ def get_sso_url(self, client_redirect_url): | |
raise NotImplementedError() | ||
|
||
|
||
class OAuth2RedirectServlet(BaseSSORedirectServlet): | ||
PATTERNS = client_patterns("/login/sso/redirect", v1=True) | ||
|
||
def __init__(self, hs): | ||
self._oauth2_handler = hs.get_oauth2_handler() | ||
|
||
def get_sso_url(self, client_redirect_url): | ||
return self._oauth2_handler.handle_redirect_request(client_redirect_url) | ||
|
||
|
||
class OAuth2ResponseServlet(RestServlet): | ||
PATTERNS = client_patterns("/login/oauth/response", v1=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. once again, the CAS code sets a poor example here. This doesn't form part of the matrix client-server API, so it should be on a different path. Have a look at the way the |
||
|
||
def __init__(self, hs): | ||
self._oauth2_handler = hs.get_oauth2_handler() | ||
|
||
def on_GET(self, request): | ||
return self._oauth2_handler.handle_oauth2_response(request) | ||
|
||
|
||
class CasRedirectServlet(BaseSSORedirectServlet): | ||
def __init__(self, hs): | ||
super(CasRedirectServlet, self).__init__() | ||
|
@@ -600,5 +624,8 @@ def register_servlets(hs, http_server): | |
if hs.config.cas_enabled: | ||
CasRedirectServlet(hs).register(http_server) | ||
CasTicketServlet(hs).register(http_server) | ||
elif hs.config.oauth2_enabled: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we probably need to add a check somewhere that will make synapse refuse to start if you try to enable more than one of CAS/OAuth/SAML2. |
||
OAuth2RedirectServlet(hs).register(http_server) | ||
OAuth2ResponseServlet(hs).register(http_server) | ||
elif hs.config.saml2_enabled: | ||
SAMLRedirectServlet(hs).register(http_server) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please don't follow the poor example of the
CAS
config block: there are conventions defined for the config file at https://github.com/matrix-org/synapse/blob/master/docs/code_style.md#configuration-file-format.