From 2f2318d58e2275e96a1599e53fc7971f7d91052a Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Tue, 14 Dec 2021 17:50:23 +0100 Subject: [PATCH] Use configuration classes from oidcmsg. Need to have oidcmsg 1.5.4 . utc_time_sans_frac instead of time.time() since the later is unspecified when it comes to timezone. --- setup.py | 2 +- src/oidcop/configure.py | 261 ++++----------------------------- src/oidcop/cookie_handler.py | 45 +++--- tests/test_00_configure.py | 8 +- tests/test_04_token_handler.py | 8 +- 5 files changed, 63 insertions(+), 261 deletions(-) diff --git a/setup.py b/setup.py index 47b7b04b..a0af09fc 100644 --- a/setup.py +++ b/setup.py @@ -72,7 +72,7 @@ def run_tests(self): "Programming Language :: Python :: 3.9", "Topic :: Software Development :: Libraries :: Python Modules"], install_requires=[ - "oidcmsg==1.5.3", + "oidcmsg==1.5.4", "pyyaml", "jinja2>=2.11.3", "responses>=0.13.0" diff --git a/src/oidcop/configure.py b/src/oidcop/configure.py index 2b9cc6d9..f286d80f 100755 --- a/src/oidcop/configure.py +++ b/src/oidcop/configure.py @@ -1,32 +1,17 @@ -"""Configuration management for IDP""" +"""Configuration management for OP""" import copy -import importlib -import json import logging import os from typing import Dict from typing import List from typing import Optional -from typing import Union -from oidcop.logging import configure_logging +from oidcmsg.configure import Base + from oidcop.scopes import SCOPE2CLAIMS -from oidcop.utils import load_yaml_config logger = logging.getLogger(__name__) - -DEFAULT_FILE_ATTRIBUTE_NAMES = [ - "server_key", - "server_cert", - "filename", - "template_dir", - "private_path", - "public_path", - "db_file", - "jwks_file", -] - OP_DEFAULT_CONFIG = { "capabilities": { "subject_types_supported": ["public", "pairwise"], @@ -105,109 +90,6 @@ } -def add_base_path(conf: Union[dict, str], base_path: str, file_attributes: List[str]): - if isinstance(conf, str): - if conf.startswith("/"): - pass - elif conf == "": - conf = "./" + conf - else: - conf = os.path.join(base_path, conf) - elif isinstance(conf, dict): - for key, val in conf.items(): - if key in file_attributes: - if val.startswith("/"): - continue - elif val == "": - conf[key] = "./" + val - else: - conf[key] = os.path.join(base_path, val) - if isinstance(val, dict): - conf[key] = add_base_path(val, base_path, file_attributes) - - return conf - - -def set_domain_and_port(conf: dict, uris: List[str], domain: str, port: int): - for key, val in conf.items(): - if key in uris: - if isinstance(val, list): - _new = [v.format(domain=domain, port=port) for v in val] - else: - _new = val.format(domain=domain, port=port) - conf[key] = _new - elif isinstance(val, dict): - conf[key] = set_domain_and_port(val, uris, domain, port) - return conf - - -def create_from_config_file( - cls, - filename: str, - base_path: str = "", - entity_conf: Optional[List[dict]] = None, - file_attributes: Optional[List[str]] = None, - domain: Optional[str] = "", - port: Optional[int] = 0, -): - if filename.endswith(".yaml"): - """Load configuration as YAML""" - _conf = load_yaml_config(filename) - elif filename.endswith(".json"): - _str = open(filename).read() - _conf = json.loads(_str) - elif filename.endswith(".py"): - head, tail = os.path.split(filename) - tail = tail[:-3] - module = importlib.import_module(tail) - _conf = getattr(module, "OIDCOP_CONFIG") - else: - raise ValueError("Unknown file type") - - return cls( - _conf, - entity_conf=entity_conf, - base_path=base_path, - file_attributes=file_attributes, - domain=domain, - port=port, - ) - - -class Base(dict): - """ Configuration base class """ - - parameter = {} - - def __init__( - self, - conf: Dict, - base_path: str = "", - file_attributes: Optional[List[str]] = None, - ): - dict.__init__(self) - - if file_attributes is None: - file_attributes = DEFAULT_FILE_ATTRIBUTE_NAMES - - if base_path and file_attributes: - # this adds a base path to all paths in the configuration - add_base_path(conf, base_path, file_attributes) - - def __getattr__(self, item): - return self[item] - - def __setattr__(self, key, value): - if key in self: - raise KeyError("{} has already been set".format(key)) - super(Base, self).__setitem__(key, value) - - def __setitem__(self, key, value): - if key in self: - raise KeyError("{} has already been set".format(key)) - super(Base, self).__setitem__(key, value) - - class EntityConfiguration(Base): default_config = AS_DEFAULT_CONFIG uris = ["issuer", "base_url"] @@ -231,26 +113,18 @@ class EntityConfiguration(Base): } def __init__( - self, - conf: Dict, - base_path: Optional[str] = "", - entity_conf: Optional[List[dict]] = None, - domain: Optional[str] = "", - port: Optional[int] = 0, - file_attributes: Optional[List[str]] = None, + self, + conf: Dict, + base_path: Optional[str] = "", + entity_conf: Optional[List[dict]] = None, + domain: Optional[str] = "", + port: Optional[int] = 0, + file_attributes: Optional[List[str]] = None, + dir_attributes: Optional[List[str]] = None, ): conf = copy.deepcopy(conf) - Base.__init__(self, conf, base_path, file_attributes) - - if file_attributes is None: - file_attributes = DEFAULT_FILE_ATTRIBUTE_NAMES - - if not domain: - domain = conf.get("domain", "127.0.0.1") - - if not port: - port = conf.get("port", 80) + Base.__init__(self, conf, base_path, file_attributes, dir_attributes=dir_attributes) for key in self.parameter.keys(): _val = conf.get(key) @@ -263,6 +137,7 @@ def __init__( file_attributes=file_attributes, domain=domain, port=port, + dir_attributes=dir_attributes ) else: continue @@ -277,29 +152,6 @@ def __init__( setattr(self, key, _val) - # try: - # _dir = self.template_dir - # except AttributeError: - # self.template_dir = os.path.abspath("templates") - # else: - # self.template_dir = - - def format(self, conf, base_path, file_attributes, domain, port): - """ - Formats parts of the configuration. That includes replacing the strings {domain} and {port} - with the used domain and port and making references to files and directories absolute - rather then relative. The formatting is done in place. - - :param conf: The configuration part - :param base_path: The base path used to make file/directory refrences absolute - :param file_attributes: Attribute names that refer to files or directories. - :param domain: The domain name - :param port: The port used - """ - add_base_path(conf, base_path, file_attributes) - if isinstance(conf, dict): - set_domain_and_port(conf, self.uris, domain=domain, port=port) - class OPConfiguration(EntityConfiguration): "Provider configuration" @@ -316,13 +168,14 @@ class OPConfiguration(EntityConfiguration): ) def __init__( - self, - conf: Dict, - base_path: Optional[str] = "", - entity_conf: Optional[List[dict]] = None, - domain: Optional[str] = "", - port: Optional[int] = 0, - file_attributes: Optional[List[str]] = None, + self, + conf: Dict, + base_path: Optional[str] = "", + entity_conf: Optional[List[dict]] = None, + domain: Optional[str] = "", + port: Optional[int] = 0, + file_attributes: Optional[List[str]] = None, + dir_attributes: Optional[List[str]] = None, ): super().__init__( conf=conf, @@ -331,21 +184,22 @@ def __init__( domain=domain, port=port, file_attributes=file_attributes, + dir_attributes=dir_attributes ) - self.scopes_to_claims class ASConfiguration(EntityConfiguration): "Authorization server configuration" def __init__( - self, - conf: Dict, - base_path: Optional[str] = "", - entity_conf: Optional[List[dict]] = None, - domain: Optional[str] = "", - port: Optional[int] = 0, - file_attributes: Optional[List[str]] = None, + self, + conf: Dict, + base_path: Optional[str] = "", + entity_conf: Optional[List[dict]] = None, + domain: Optional[str] = "", + port: Optional[int] = 0, + file_attributes: Optional[List[str]] = None, + dir_attributes: Optional[List[str]] = None, ): EntityConfiguration.__init__( self, @@ -355,63 +209,10 @@ def __init__( domain=domain, port=port, file_attributes=file_attributes, + dir_attributes=dir_attributes ) -class Configuration(Base): - """Server Configuration""" - - uris = ["issuer", "base_url"] - - def __init__( - self, - conf: Dict, - entity_conf: Optional[List[dict]] = None, - base_path: str = "", - file_attributes: Optional[List[str]] = None, - domain: Optional[str] = "", - port: Optional[int] = 0, - ): - Base.__init__(self, conf, base_path, file_attributes) - - log_conf = conf.get("logging") - if log_conf: - self.logger = configure_logging(config=log_conf).getChild(__name__) - else: - self.logger = logging.getLogger("oidcop") - - self.webserver = conf.get("webserver", {}) - - if not domain: - domain = conf.get("domain", "127.0.0.1") - - if not port: - port = conf.get("port", 80) - - set_domain_and_port(conf, self.uris, domain=domain, port=port) - - if entity_conf: - for econf in entity_conf: - _path = econf.get("path") - _cnf = conf - if _path: - for step in _path: - _cnf = _cnf[step] - _attr = econf["attr"] - _cls = econf["class"] - setattr( - self, - _attr, - _cls( - _cnf, - base_path=base_path, - file_attributes=file_attributes, - domain=domain, - port=port, - ), - ) - - DEFAULT_EXTENDED_CONF = { "add_on": { "pkce": { diff --git a/src/oidcop/cookie_handler.py b/src/oidcop/cookie_handler.py index ea2549b5..cb8130e7 100755 --- a/src/oidcop/cookie_handler.py +++ b/src/oidcop/cookie_handler.py @@ -14,6 +14,7 @@ from cryptojwt.jwe.utils import split_ctx_and_tag from cryptojwt.jwk.hmac import SYMKey from cryptojwt.jws.hmac import HMACSigner +from cryptojwt.jwt import utc_time_sans_frac from cryptojwt.key_jar import init_key_jar from oidcmsg.time_util import epoch_in_a_while @@ -31,13 +32,13 @@ class CookieHandler: def __init__( - self, - sign_key: Optional[SYMKey] = None, - enc_key: Optional[SYMKey] = None, - keys: Optional[dict] = None, - sign_alg: [str] = "SHA256", - name: Optional[dict] = None, - **kwargs, + self, + sign_key: Optional[SYMKey] = None, + enc_key: Optional[SYMKey] = None, + keys: Optional[dict] = None, + sign_alg: [str] = "SHA256", + name: Optional[dict] = None, + **kwargs, ): if keys: @@ -101,7 +102,7 @@ def _sign_enc_payload(self, payload: str, timestamp: Optional[Union[int, str]] = if timestamp: timestamp = str(timestamp) else: - timestamp = str(int(time.time())) + timestamp = str(int(utc_time_sans_frac())) bytes_load = payload.encode("utf-8") bytes_timestamp = timestamp.encode("utf-8") @@ -153,9 +154,9 @@ def _ver_dec_content(self, parts): mac = base64.b64decode(b64_mac) verifier = HMACSigner(algorithm=self.sign_alg) if verifier.verify( - payload.encode("utf-8") + timestamp.encode("utf-8"), - mac, - self.sign_key.key, + payload.encode("utf-8") + timestamp.encode("utf-8"), + mac, + self.sign_key.key, ): return payload, timestamp else: @@ -178,9 +179,9 @@ def _ver_dec_content(self, parts): if len(p) == 3: verifier = HMACSigner(algorithm=self.sign_alg) if verifier.verify( - payload.encode("utf-8") + timestamp.encode("utf-8"), - base64.b64decode(p[2]), - self.sign_key.key, + payload.encode("utf-8") + timestamp.encode("utf-8"), + base64.b64decode(p[2]), + self.sign_key.key, ): return payload, timestamp else: @@ -190,13 +191,13 @@ def _ver_dec_content(self, parts): return None def make_cookie_content( - self, - name: str, - value: str, - typ: Optional[str] = "", - timestamp: Optional[Union[int, str]] = "", - max_age: Optional[int] = 0, - **kwargs, + self, + name: str, + value: str, + typ: Optional[str] = "", + timestamp: Optional[Union[int, str]] = "", + max_age: Optional[int] = 0, + **kwargs, ) -> dict: """ Create and return information to put in a cookie @@ -210,7 +211,7 @@ def make_cookie_content( """ if not timestamp: - timestamp = str(int(time.time())) + timestamp = str(int(utc_time_sans_frac())) # create cookie payload if not value and not typ: diff --git a/tests/test_00_configure.py b/tests/test_00_configure.py index b6d78103..c0c9a815 100644 --- a/tests/test_00_configure.py +++ b/tests/test_00_configure.py @@ -1,11 +1,11 @@ import json import os +from oidcmsg.configure import Configuration +from oidcmsg.configure import create_from_config_file import pytest -from oidcop.configure import Configuration from oidcop.configure import OPConfiguration -from oidcop.configure import create_from_config_file from oidcop.logging import configure_logging BASEDIR = os.path.abspath(os.path.dirname(__file__)) @@ -76,7 +76,7 @@ def test_op_configure_default(): id_token_conf = configuration.get("id_token", {}) assert set(id_token_conf.keys()) == {"kwargs", "class"} assert id_token_conf["kwargs"] == { - "base_claims": {"email": {"essential": True}, "email_verified": {"essential": True},} + "base_claims": {"email": {"essential": True}, "email_verified": {"essential": True}, } } @@ -95,7 +95,7 @@ def test_op_configure_default_from_file(): id_token_conf = configuration.get("id_token", {}) assert set(id_token_conf.keys()) == {"kwargs", "class"} assert id_token_conf["kwargs"] == { - "base_claims": {"email": {"essential": True}, "email_verified": {"essential": True},} + "base_claims": {"email": {"essential": True}, "email_verified": {"essential": True}, } } diff --git a/tests/test_04_token_handler.py b/tests/test_04_token_handler.py index bdd9c937..2f3ea077 100644 --- a/tests/test_04_token_handler.py +++ b/tests/test_04_token_handler.py @@ -3,9 +3,8 @@ import hmac import os import secrets -import time -from cryptojwt.jwt import utc_time_sans_frac +from oidcmsg.time_util import utc_time_sans_frac import pytest from oidcop.configure import OPConfiguration @@ -105,8 +104,9 @@ def test_is_expired(self): _token = self.th("another_id") assert self.th.is_expired(_token) is False - when = time.time() + 900 - assert self.th.is_expired(_token, int(when)) + when = utc_time_sans_frac() + # has it expired 24 hours from now ? + assert self.th.is_expired(_token, int(when) + 86400) class TestTokenHandler(object):