Review and refactor the OpenIDConnect code

hiboo/sso/oidc.py

+""" The OIDC SSO providers implements OpenID Connect
+
+Supported grants are authorization code, OpenID implicit and hybrid.
+It relies heavily on authlib for the OAuth/OIDC implementation.
+"""
+
 from werkzeug.security import gen_salt
 from authlib.integrations import flask_oauth2, sqla_oauth2
 from authlib.oauth2 import rfc6749 as oauth2
@@ -12,18 +18,20 @@ import time
 
 class Config(object):
     """ Handles service configuration and forms.
+
+    Settings are:
+     - token_endpoint_auth_method: the method for authenticating clients
+     - redirect_url: the (single) supported redirect uri for the client
+     - grant_types: supported grant types
+     - response_types: supported response types (the order matters)
     """
 
     @classmethod
     def derive_form(cls, form):
-        """ Add required fields to a form.
-        """
-        return type('NewForm', (forms.OIDCForm, form), {})
+        return type('DerivedOIDCForm', (forms.OIDCForm, form), {})
 
     @classmethod
     def populate_service(cls, form, service):
-        """ Populate a service from a form
-        """
         service.config.update({
             "token_endpoint_auth_method": form.token_endpoint_auth_method.data,
             "redirect_uris": [form.redirect_uri.data],
@@ -34,8 +42,6 @@ class Config(object):
 
     @classmethod
     def populate_form(cls, service, form):
-        """ Populate a form from a service
-        """
         form.process(
             obj=service,
             token_endpoint_auth_method=service.config.get("token_endpoint_auth_method"),
@@ -46,6 +52,8 @@ class Config(object):
 
     @classmethod
     def update_client(cls, service):
+        """ If necessary, prepare the client with cryptographic material.
+        """
         if "client_id" not in service.config:
             service.config.update(
                 client_id=gen_salt(24),
@@ -55,67 +63,15 @@ class Config(object):
             )
 
 
-class Client(sqla_oauth2.OAuth2ClientMixin):
-    """ OIDC client that only supports authorization code, implicit and
-    hybrid flows.
+class AuthorizationCodeMixin(object):
+    """ Mixin for defining oauth grants
     """
 
-    scope = "openid"
-
-    def __init__(self, service):
-        self.service = service
-        self.client_id = service.config["client_id"]
-        self.client_secret = service.config["client_secret"]
-        self.client_metadata = service.config
-        self.authorization = flask_oauth2.AuthorizationServer(
-            query_client=self.query_client,
-            save_token=self.save_token,
-            app=flask.current_app
-        )
-        self.authorization.register_grant(
-            AuthorizationCodeGrant, [OpenIDCode(require_nonce=False)]
-        )
-        self.authorization.register_grant(ImplicitGrant)
-        self.authorization.register_grant(HybridGrant)
-
-    def query_client(self, client_id):
-        return self if client_id == self.client_id else None
+    # Authorization code object for redis storage
+    AuthorizationCode = type("AuthorizationCode", (utils.SerializableObj, sqla_oauth2.OAuth2AuthorizationCodeMixin), {})
 
-    def save_token(self, token, request):
-        pass
-
-    def get_jwt_config(self):
-        return {
-            'key': self.service.config["jwt_key"],
-            'alg': self.service.config["jwt_alg"],
-            'iss': flask.url_for("sso.oidc_token", service_uuid=self.service.uuid, _external=True),
-            'exp': 3600,
-        }
-
-    @classmethod
-    def generate_user_info(cls, user, scope):
-        # The login attribute is not standard as per OIDC spec, but it is used
-        # by many RP.
-        return oidc.UserInfo(
-            sub=user.uuid,
-            name=user.username,
-            prefered_username=user.username,
-            login=user.username,
-            email=user.email
-        )
-
-    @classmethod
-    def exists_nonce(cls, nonce, request):
-        return bool(utils.redis.get("nonce:{}".format(nonce)))
-
-
-class AuthorizationCode(utils.SerializableObj, sqla_oauth2.OAuth2AuthorizationCodeMixin):
-    """ Authorization code object for storage
-    """
-
-    @classmethod
-    def create(cls, client, grant_user, request):
-        obj = cls(
+    def create_authorization_code(self, client, grant_user, request):
+        obj = AuthorizationCodeMixin.AuthorizationCode(
             code=gen_salt(48), nonce=request.data.get("nonce") or "",
             client_id=client.client_id, redirect_uri=request.redirect_uri,
             scope=request.scope, user_id=grant_user.uuid,
@@ -126,72 +82,85 @@ class AuthorizationCode(utils.SerializableObj, sqla_oauth2.OAuth2AuthorizationCo
             utils.redis.set("nonce:{}".format(obj.nonce), obj.code)
         return obj.code
 
-    @classmethod
-    def get(cls, code, client):
-        obj = cls.unserialize(utils.redis.hgetall("code:{}".format(code)))
-        if obj and obj.client_id == client.client_id:
-            return obj
-
-    @classmethod
-    def delete(cls, authorization_code):
-        utils.redis.delete("code:{}".format(authorization_code))
-
-
-class AuthorizationCodeGrant(oauth2.grants.AuthorizationCodeGrant):
-    """ Authorization code grant
-    """
-
-    def create_authorization_code(self, client, grant_user, request):
-        return AuthorizationCode.create(client, grant_user, request)
-
     def parse_authorization_code(self, code, client):
-        return AuthorizationCode.get(code, client)
+        obj = AuthorizationCodeMixin.AuthorizationCode.unserialize(
+            utils.redis.hgetall("code:{}".format(code))
+        )
+        return if obj and obj.client_id == client.client_id else None
 
     def delete_authorization_code(self, authorization_code):
-        return AuthorizationCode.delete(authorization_code)
+        utils.redis.delete("code:{}".format(authorization_code))
 
     def authenticate_user(self, authorization_code):
-        profile = models.Profile.query.get(authorization_code.user_id)
-        return profile
+        return models.Profile.query.get(authorization_code.user_id)
+
 
+class OpenIDMixin(object):
+    """ Mixin for defining OpenID grants
+    """
 
-class OpenIDCode(oidc.grants.OpenIDCode):
     def exists_nonce(self, nonce, request):
-        return Client.exists_nonce(nonce, request)
+        return bool(utils.redis.get("nonce:{}".format(nonce)))
 
     def get_jwt_config(self, grant):
-        return grant.client.get_jwt_config()
+        service = grant.client.service
+        return {
+            'key': service.config["jwt_key"], 'alg': service.config["jwt_alg"],
+            'iss': flask.url_for("sso.oidc_token", service_uuid=service.uuid, _external=True),
+            'exp': 3600,
+        }
 
     def generate_user_info(self, user, scope):
-        return Client.generate_user_info(user, scope)
-
-
-class ImplicitGrant(oidc.grants.OpenIDImplicitGrant):
-    def exists_nonce(self, nonce, request):
-        return Client.exists_nonce(nonce, request)
+        return oidc.UserInfo(
+            sub=user.uuid,
+            name=user.username,
+            prefered_username=user.username,
+            login=user.username,
+            email=user.email
+        )
 
-    def get_jwt_config(self):
-        return self.request.client.get_jwt_config()
 
-    def generate_user_info(self, user, scope):
-        return Client.generate_user_info(user, scope)
+class Client(sqla_oauth2.OAuth2ClientMixin):
+    """ OIDC client that supports authorization code, implicit and
+    hybrid flows.
+    """
 
+    scope = "openid"
 
-class HybridGrant(oidc.grants.OpenIDHybridGrant):
-    def create_authorization_code(self, client, grant_user, request):
-        return AuthorizationCode.create(client, grant_user, request)
+    # Declare grant types using the above base classes
+    AuthorizationCodeGrant = type("AuthorizationCodeGrant", (AuthorizationCodeMixin, oauth2.grants.AuthorizationCodeGrant), {})
+    OpenIDCode = type("OpenIDCode", (OpenIDMixin, oidc.grants.OpenIDCode), {})
+    ImplicitGrant = type("ImplicitGrant", (OpenIDMixin, oidc.grants.OpenIDImplicitGrant), {})
+    HybridGrant = type("HybridGrant", (AuthorizationCodeMixin, OpenIDMixin, oidc.grants.OpenIDHybridGrant), {})
 
-    def exists_nonce(self, nonce, request):
-        return Client.exists_nonce(nonce, request)
+    def __init__(self, service):
+        self.service = service
+        self.client_id = service.config["client_id"]
+        self.client_secret = service.config["client_secret"]
+        # Configuration is stored in a format compatible with authlib metadata
+        # so it only needs to be passed to the authorization server object
+        self.client_metadata = service.config
+        self.authorization = flask_oauth2.AuthorizationServer(
+            query_client=self.query_client,
+            save_token=self.save_token,
+            app=flask.current_app
+        )
+        self.authorization.register_grant(
+            Client.AuthorizationCodeGrant, [Client.OpenIDCode(require_nonce=False)]
+        )
+        self.authorization.register_grant(Client.ImplicitGrant)
+        self.authorization.register_grant(Client.HybridGrant)
 
-    def get_jwt_config(self):
-        return self.request.client.get_jwt_config()
+    def query_client(self, client_id):
+        return self if client_id == self.client_id else None
 
-    def generate_user_info(self, user, scope):
-        return Client.generate_user_info(user, scope)
+    def save_token(self, token, request):
+        # Tokens are not saved since Hiboo supports user authentication, note
+        # long term app authentication.
+        pass
 
 
-@blueprint.route("/authorize/<service_uuid>", methods=["GET", "POST"])
+@blueprint.route("/oidc/authorize/<service_uuid>", methods=["GET", "POST"])
 def oidc_authorize(service_uuid):
     # Get the profile from user input (implies redirects)
     service = models.Service.query.get(service_uuid) or flask.abort(404)
@@ -202,12 +171,11 @@ def oidc_authorize(service_uuid):
     return client.authorization.create_authorization_response(grant_user=picked)
 
 
-@blueprint.route("/token/<service_uuid>", methods=["POST"])
+@blueprint.route("/oidc/token/<service_uuid>", methods=["POST"])
 def oidc_token(service_uuid):
     # Get the profile from user input (implies redirects)
     service = models.Service.query.get(service_uuid) or flask.abort(404)
     service.protocol == "oidc" or flask.abort(404)
     # Generate and return the response
     client = Client(service)
-    result = client.authorization.create_token_response()
-    return result
+    return client.authorization.create_token_response()

hiboo/sso/templates/protocol_oidc.html

@@ -3,10 +3,10 @@
 
 {% macro describe(service) %}
 <dt>{% trans %}Authorization endpoint{% endtrans %}</dt>
-<dd>{{ url_for("sso.oidc_authorize", service_uuid=service.uuid, _external=True) }}</dd>
+<dd><pre>{{ url_for("sso.oidc_authorize", service_uuid=service.uuid, _external=True) }}</pre></dd>
 
 <dt>{% trans %}Token endpoint{% endtrans %}</dt>
-<dd>{{ url_for("sso.oidc_token", service_uuid=service.uuid, _external=True) }}</dd>
+<dd><pre>{{ url_for("sso.oidc_token", service_uuid=service.uuid, _external=True) }}</pre></dd>
 
 <dt>{% trans %}Client ID{% endtrans %}</dt>
 <dd><pre>{{ service.config["client_id"] }}</pre></dd>