From ce13a9c7ca81f204da787750d736bef2a2cb63ff Mon Sep 17 00:00:00 2001 From: bmccandless Date: Mon, 3 Aug 2020 10:45:21 -0700 Subject: [PATCH] oauth support, add the token in a configuration specified cookie (#1702) * oauth support, add the token in a configuration specified cookie Previously, the id token was stored in the session token. Now, it can be placed in a different cookie with different properties. --- server/auth/auth_oauth.py | 71 +++++++++++++++++++++++++++------ server/common/app_config.py | 9 +++++ server/common/default_config.py | 9 +++++ 3 files changed, 77 insertions(+), 12 deletions(-) diff --git a/server/auth/auth_oauth.py b/server/auth/auth_oauth.py index 88669f88..ca2adf95 100644 --- a/server/auth/auth_oauth.py +++ b/server/auth/auth_oauth.py @@ -1,4 +1,4 @@ -from flask import session, request, redirect, current_app, has_request_context +from flask import session, request, redirect, current_app, after_this_request, has_request_context, g from server.auth.auth import AuthTypeClientBase, AuthTypeFactory from server.common.errors import AuthenticationError, ConfigurationError from urllib.parse import urlencode @@ -24,15 +24,20 @@ class AuthTypeOAuth(AuthTypeClientBase): CXG_ID_TOKEN = "id_token" - def __init__(self, app_config): + def __init__(self, server_config): super().__init__() if missingimport: raise ConfigurationError(f"oauth requires these modules: {', '.join(missingimport)}") self.algorithms = ["RS256"] - self.api_base_url = app_config.authentication__params_oauth__api_base_url - self.client_id = app_config.authentication__params_oauth__client_id - self.client_secret = app_config.authentication__params_oauth__client_secret - self.callback_base_url = app_config.authentication__params_oauth__callback_base_url + self.api_base_url = server_config.authentication__params_oauth__api_base_url + self.client_id = server_config.authentication__params_oauth__client_id + self.client_secret = server_config.authentication__params_oauth__client_secret + self.callback_base_url = server_config.authentication__params_oauth__callback_base_url + self.session_cookie = server_config.authentication__params_oauth__session_cookie + self.cookie_params = server_config.authentication__params_oauth__cookie + self._validate_cookie_params() + + # set the audience self.audience = self.client_id # load the jwks (JSON Web Key Set). @@ -45,6 +50,21 @@ class AuthTypeOAuth(AuthTypeClientBase): except Exception: raise ConfigurationError(f"error in oauth, api_url_base: {self.api_base_url}, cannot access {jwksloc}") + def _validate_cookie_params(self): + """check the cookie_params, and raise a ConfigurationError if there is something wrong""" + if self.session_cookie: + return + + if not isinstance(self.cookie_params, dict): + raise ConfigurationError("either session_cookie or cookie must be set") + valid_keys = {"key", "max_age", "expires", "path", "domain", "secure", "httponly", "samesite"} + keys = set(self.cookie_params.keys()) + unknown = keys - valid_keys + if unknown: + raise ConfigurationError(f"unexpected key in cookie params: {', '.join(unknown)}") + if "key" not in keys: + raise ConfigurationError("must have a key (name) in the cookie params") + def is_valid_authentication_type(self): return True @@ -111,8 +131,15 @@ class AuthTypeOAuth(AuthTypeClientBase): return self.client.authorize_redirect(redirect_uri=callbackurl) def logout(self): - if self.CXG_ID_TOKEN in session: - del session[self.CXG_ID_TOKEN] + if self.session_cookie: + if self.CXG_ID_TOKEN in session: + del session[self.CXG_ID_TOKEN] + else: + @after_this_request + def remove_cookie(response): + response.set_cookie(self.cookie_params["key"], "", expires=0) + return response + return_path = request.args.get("dataset", "") return_to = f"{self.callback_base_url}/{return_path}" params = {'returnTo' : return_to, 'client_id' : self.client_id} @@ -121,10 +148,23 @@ class AuthTypeOAuth(AuthTypeClientBase): def callback(self): token = self.client.authorize_access_token() id_token = token.get("id_token") - session[self.CXG_ID_TOKEN] = id_token - del session["oauth_callback_redirect"] - oauth_callback_redirect = session.get("oauth_callback_redirect", "/") + oauth_callback_redirect = session.pop("oauth_callback_redirect", "/") resp = redirect(oauth_callback_redirect) + + if self.session_cookie: + session[self.CXG_ID_TOKEN] = id_token + else: + args = self.cookie_params.copy() + del args["key"] + try: + resp.set_cookie( + self.cookie_params["key"], + id_token, + **args) + g.token = id_token + except Exception as e: + raise AuthenticationError(f"unable to set_cookie {self.cookie_params}") from e + return resp def get_login_url(self, data_adaptor): @@ -143,7 +183,14 @@ class AuthTypeOAuth(AuthTypeClientBase): def get_token(self): """Function to return the token""" - return session.get(self.CXG_ID_TOKEN) + if "token" in g: + return g.token + if self.session_cookie: + g.token = session.get(self.CXG_ID_TOKEN) + else: + g.token = request.cookies.get(self.cookie_params["key"]) + + return g.token def get_jwt_payload(self): if not has_request_context(): diff --git a/server/common/app_config.py b/server/common/app_config.py index d54c86b0..aef6af98 100644 --- a/server/common/app_config.py +++ b/server/common/app_config.py @@ -393,6 +393,7 @@ class ServerConfig(BaseConfig): def __init__(self, app_config, default_config): dictval_cases = [ ("app", "csp_directives"), + ("authentication", "params_oauth", "cookie"), ("adaptor", "cxg_adaptor", "tiledb_ctx"), ("multi_dataset", "dataroot"), ] @@ -417,6 +418,8 @@ class ServerConfig(BaseConfig): self.authentication__params_oauth__client_secret = dc["authentication"]["params_oauth"]["client_secret"] self.authentication__params_oauth__callback_base_url = \ dc["authentication"]["params_oauth"]["callback_base_url"] + self.authentication__params_oauth__session_cookie = dc["authentication"]["params_oauth"]["session_cookie"] + self.authentication__params_oauth__cookie = dc["authentication"]["params_oauth"]["cookie"] self.multi_dataset__dataroot = dc["multi_dataset"]["dataroot"] self.multi_dataset__index = dc["multi_dataset"]["index"] @@ -531,6 +534,12 @@ class ServerConfig(BaseConfig): self.check_attr("authentication__params_oauth__client_id", ptypes) self.check_attr("authentication__params_oauth__client_secret", ptypes) self.check_attr("authentication__params_oauth__callback_base_url", (type(None), str)) + self.check_attr("authentication__params_oauth__session_cookie", bool) + + if self.authentication__params_oauth__session_cookie: + self.check_attr("authentication__params_oauth__cookie", (type(None), dict)) + else: + self.check_attr("authentication__params_oauth__cookie", dict) # secret key: first, from CXG_OAUTH_CLIENT_SECRET environment variable # second, from config file self.authentication__params__oauth__client_secret = os.environ.get( diff --git a/server/common/default_config.py b/server/common/default_config.py index 0fd79620..f77559bc 100644 --- a/server/common/default_config.py +++ b/server/common/default_config.py @@ -33,6 +33,15 @@ server: # A value of None, indicates the client and server are on the localhost. http://localhost: will be used. callback_base_url: null + # if true, the jwt containing the id_token is stored in a session cookie + session_cookie: true + + # if session_cookie is false, then a regular cookie will be used. In that case + # the cookie will be defined by a dictionary of parameters. + # The keys of the dictionary match the parameters of the flask set_cookie api + # (https://flask.palletsprojects.com/en/1.1.x/api/), and with the same meaning. + # legal keys: key, max_age, expires, path, domain, secure, httponly, and samesite. + cookie: null multi_dataset: # If dataroot is set, then cellxgene may serve multiple datasets. This parameter is not