address comments and rework migration

fence/__init__.py

@@ -364,7 +364,6 @@ def app_config(
     _setup_audit_service_client(app)
     _setup_data_endpoint_and_boto(app)
     _load_keys(app, root_dir)
-    _set_authlib_cfgs(app)
 
     app.prometheus_counters = {}
     if config["ENABLE_PROMETHEUS_METRICS"]:
@@ -406,24 +405,6 @@ def _load_keys(app, root_dir):
     }
 
 
-def _set_authlib_cfgs(app):
-    # authlib OIDC settings
-    # key will need to be added
-    settings = {"OAUTH2_JWT_KEY": keys.default_private_key(app)}
-    app.config.update(settings)
-    config.update(settings)
-
-    # only add the following if not already provided
-    config.setdefault("OAUTH2_JWT_ENABLED", True)
-    config.setdefault("OAUTH2_JWT_ALG", "RS256")
-    config.setdefault("OAUTH2_JWT_ISS", app.config["BASE_URL"])
-    config.setdefault("OAUTH2_PROVIDER_ERROR_URI", "/api/oauth2/errors")
-    app.config.setdefault("OAUTH2_JWT_ENABLED", True)
-    app.config.setdefault("OAUTH2_JWT_ALG", "RS256")
-    app.config.setdefault("OAUTH2_JWT_ISS", app.config["BASE_URL"])
-    app.config.setdefault("OAUTH2_PROVIDER_ERROR_URI", "/api/oauth2/errors")
-
-
 def _setup_oidc_clients(app):
     configured_idps = config.get("OPENID_CONNECT", {})
 

fence/blueprints/login/__init__.py

@@ -368,8 +368,6 @@ def get_all_shib_idps():
     Returns:
         list: list of {"idp": "", "name": ""} dictionaries
     """
-    # TODO Config is only returning {'client_id': '', 'client_secret': '', 'redirect_url': 'http://localhost/user/login/fence/login'}
-    # Why does it only have 3 attributes left when there are so many in the config, including shibboleth_discovery_url
     url = config["OPENID_CONNECT"].get("fence", {}).get("shibboleth_discovery_url")
     if not url:
         raise InternalError(

fence/blueprints/login/fence_login.py

@@ -91,11 +91,6 @@ def get(self):
                 " login page for the original application to continue."
             )
         # Get the token response and log in the user.
-        # TODO: fence_client <authlib.integrations.flask_client.OAuth> no longer has _get_session(), need to get redirect_uri from elsewhere
-        # This has to do with how authutils implement OAuth Client
-        # Look at https://github.com/uc-cdis/authutils/tree/feat/authlib
-
-        # redirect_uri = flask.current_app.fence_client._get_session().redirect_uri
         client_name = config["OPENID_CONNECT"]["fence"].get("name", "fence")
         client = flask.current_app.fence_client._clients[client_name]
         oauth2_redirect_uri = client.client_kwargs.get("redirect_uri")

fence/config-default.yaml

@@ -386,16 +386,9 @@ ENABLED_IDENTITY_PROVIDERS: {}
 
 
 # //////////////////////////////////////////////////////////////////////////////////////
-# LIBRARY CONFIGURATION (authlib & flask)
+# LIBRARY CONFIGURATION (flask)
 #   - Already contains reasonable defaults
 # //////////////////////////////////////////////////////////////////////////////////////
-# authlib-specific configs for OIDC flow and JWTs
-# NOTE: the OAUTH2_JWT_KEY cfg gets set automatically by fence if keys are setup
-#       correctly
-OAUTH2_JWT_ALG: 'RS256'
-OAUTH2_JWT_ENABLED: true
-OAUTH2_JWT_ISS: '{{BASE_URL}}'
-OAUTH2_PROVIDER_ERROR_URI: '/api/oauth2/errors'
 
 # used for flask, "path mounted under by the application / web server"
 # since we deploy as microservices, fence is typically under {{base}}/user

fence/models.py

@@ -60,7 +60,6 @@
 from fence import logger
 from fence.config import config
 from fence.errors import UserError
-import jwt
 
 
 def query_for_user(session, username):
@@ -227,7 +226,10 @@ class Client(Base, OAuth2ClientMixin):
     def __init__(self, client_id, expires_in=0, **kwargs):
 
         # New Json object for Authlib Oauth client
-        client_metadata = {}
+        if "_client_metadata" in kwargs:
+            client_metadata = json.loads(kwargs.pop("_client_metadata"))
+        else:
+            client_metadata = {}
 
         if "allowed_scopes" in kwargs:
             allowed_scopes = kwargs.pop("allowed_scopes")
@@ -289,11 +291,13 @@ def __init__(self, client_id, expires_in=0, **kwargs):
                 "token_endpoint_auth_method"
             )
 
-        expires_at = get_client_expires_at(
-            expires_in=expires_in, grant_types=client_metadata["grant_types"]
-        )
-        if expires_at:
-            kwargs["expires_at"] = expires_at
+        # Do this if expires_in is specified or expires_at is not supplied
+        if expires_in != 0 or ("expires_at" not in kwargs):
+            expires_at = get_client_expires_at(
+                expires_in=expires_in, grant_types=client_metadata["grant_types"]
+            )
+            if expires_at:
+                kwargs["expires_at"] = expires_at
 
         if "client_id_issued_at" not in kwargs or kwargs["client_id_issued_at"] is None:
             kwargs["client_id_issued_at"] = int(time.time())
@@ -347,7 +351,7 @@ def check_requested_scopes(self, scopes):
             return False
         return set(self.allowed_scopes).issuperset(scopes)
 
-    # Replaces Authlib method
+    # Replaces Authlib method. Our logic does not actually look at token_auth_endpoint value
     def check_endpoint_auth_method(self, method, endpoint):
         """
         Only basic auth is supported. If anything else gets added, change this
@@ -360,10 +364,6 @@ def check_endpoint_auth_method(self, method, endpoint):
 
         return True
 
-    def validate_scopes(self, scopes):
-        scopes = scopes[0].split(",")
-        return all(scope in self.scope for scope in scopes)
-
     def check_response_type(self, response_type):
         allowed_response_types = []
         if "authorization_code" in self.grant_types:
@@ -826,14 +826,3 @@ def populate_iss_sub_pair_to_user_table(target, connection, **kw):
         else:
             transaction.commit()
             logger.info("Population was successful")
-
-
-class JWTToken(object):
-    def __init__(self, token):
-        self.encoded_string = token
-        self.client_id = jwt.decode(
-            token, algorithms=["RS256"], options={"verify_signature": False}
-        ).get("azp")
-
-    def check_client(self, client):
-        return self.client_id == client.client_id

fence/oidc/endpoints.py

@@ -2,13 +2,12 @@
 import authlib.oauth2.rfc7009
 import bcrypt
 import flask
-from fence.models import JWTToken
 
 from cdislogging import get_logger
 
 from fence.errors import BlacklistingError
 import fence.jwt.blacklist
-
+import jwt
 
 logger = get_logger(__name__)
 
@@ -110,3 +109,14 @@ def create_revocation_response(self):
         finally:
             body = {"error": message} if message != "" else {}
         return (status, body, headers)
+
+
+class JWTToken(object):
+    def __init__(self, token):
+        self.encoded_string = token
+        self.client_id = jwt.decode(
+            token, algorithms=["RS256"], options={"verify_signature": False}
+        ).get("azp")
+
+    def check_client(self, client):
+        return self.client_id == client.client_id

fence/oidc/grants/__init__.py

@@ -1,4 +1,4 @@
 from fence.oidc.grants.implicit_grant import ImplicitGrant
-from fence.oidc.grants.oidc_code_grant import OpenIDCodeGrant
+from fence.oidc.grants.oidc_code_grant import AuthorizationCodeGrant
 from fence.oidc.grants.refresh_token_grant import RefreshTokenGrant
 from fence.oidc.grants.client_credentials_grant import ClientCredentialsGrant

fence/oidc/grants/oidc_code_grant.py

@@ -12,36 +12,17 @@
 from fence.models import AuthorizationCode, ClientAuthType, User
 
 
-class OpenIDCodeGrant(grants.AuthorizationCodeGrant):
+class AuthorizationCodeGrant(grants.AuthorizationCodeGrant):
 
     TOKEN_ENDPOINT_AUTH_METHODS = [auth_type.value for auth_type in ClientAuthType]
 
     def __init__(self, *args, **kwargs):
-        super(OpenIDCodeGrant, self).__init__(*args, **kwargs)
+        super(AuthorizationCodeGrant, self).__init__(*args, **kwargs)
         # Override authlib validate_request_prompt with our own, to fix login prompt behavior
-        """self._hooks["after_validate_consent_request"].discard(
-            grants.util.validate_request_prompt
-        )"""
         self.register_hook(
             "after_validate_consent_request", self.validate_request_prompt
         )
 
-    """
-    def get_jwt_config(self, grant):
-        return {
-            'key': flask.current_app.config["OAUTH2_JWT_KEY"],
-            'alg': flask.current_app.config["OAUTH2_JWT_ALG"],
-            'iss': flask.current_app.config["OAUTH2_JWT_ISS"],
-            'exp': flask.current_app.config["OAUTH2_TOKEN_EXPIRES_IN"]["authorization_code"]
-        }
-
-    def generate_user_info(self, user, scope):
-            user_info = UserInfo(sub=user.id, name=user.name)
-            if 'email' in scope:
-                user_info['email'] = user.email
-            return user_info
-    """
-
     @staticmethod
     def create_authorization_code(client, grant_user, request):
         """
@@ -76,7 +57,6 @@ def create_authorization_code(client, grant_user, request):
 
         return code.code
 
-    # In Case we need to use AuthorizationCodeGrant
     def save_authorization_code(self, code, request):
         # requested lifetime (in seconds) for the refresh token
         refresh_token_expires_in = get_valid_expiration_from_request(

fence/oidc/server.py

@@ -9,7 +9,7 @@
 from fence.oidc.client import authenticate_public_client, query_client
 from fence.oidc.endpoints import RevocationEndpoint
 from fence.oidc.grants import (
-    OpenIDCodeGrant,
+    AuthorizationCodeGrant,
     ImplicitGrant,
     RefreshTokenGrant,
     ClientCredentialsGrant,
@@ -18,7 +18,7 @@
 
 
 server = OIDCServer(query_client=query_client, save_token=lambda *_: None)
-server.register_grant(OpenIDCodeGrant)
+server.register_grant(AuthorizationCodeGrant)
 server.register_grant(ImplicitGrant)
 server.register_grant(RefreshTokenGrant)
 server.register_grant(ClientCredentialsGrant)

fence/scripting/fence_create.py

@@ -216,7 +216,7 @@ def delete_expired_clients_action(DB, slack_webhook=None, warning_days=None):
         # to delete
         pass
 
-    def split_uris(uris):
+    def format_uris(uris):
         if not uris:
             return uris
         return " ".join(uris)
@@ -235,7 +235,7 @@ def split_uris(uris):
 
         for client in clients:
             expired_messages.append(
-                f"Client '{client.name}' (ID '{client.client_id}') expired at {datetime.fromtimestamp(client.expires_at)} UTC. Redirect URIs: {split_uris(client.redirect_uris)})"
+                f"Client '{client.name}' (ID '{client.client_id}') expired at {datetime.fromtimestamp(client.expires_at)} UTC. Redirect URIs: {format_uris(client.redirect_uris)})"
             )
             _remove_client_service_accounts(current_session, client)
             current_session.delete(client)
@@ -257,7 +257,7 @@ def split_uris(uris):
     expiring_messages = ["Some OIDC clients are expiring soon!"]
     expiring_messages.extend(
         [
-            f"Client '{client.name}' (ID '{client.client_id}') expires at {datetime.fromtimestamp(client.expires_at)} UTC. Redirect URIs: {split_uris(client.redirect_uris)}"
+            f"Client '{client.name}' (ID '{client.client_id}') expires at {datetime.fromtimestamp(client.expires_at)} UTC. Redirect URIs: {format_uris(client.redirect_uris)}"
             for client in expiring_clients
         ]
     )

fence/utils.py

@@ -441,8 +441,15 @@ def validate_scopes(request_scopes, client):
         raise Exception("Client object is None")
 
     if request_scopes:
-        request_scopes = scope_to_list(request_scopes)
-        if not client.check_requested_scopes(set(request_scopes)):
+        scopes = scope_to_list(request_scopes)
+        # can we get some debug logs here that log the client, what scopes they have, and what scopes were requested
+        if not client.check_requested_scopes(set(scopes)):
+            logger.debug(
+                "Request Scope are "
+                + " ".join(scopes)
+                + "but client supported scopes are "
+                + client.scope
+            )
             raise InvalidScopeError("Failed to Authorize due to unsupported scope")
 
     return True

migrations/models/migration_client.py

@@ -8,7 +8,7 @@
 
 # This needs to be in a different file
 # Otherwise SqlAlchemy would import this multiple times and then complain about metadata conflict
-class MigrationClient(Base, OAuth2ClientMixin):
+class MigrationClient(Base):
 
     __tablename__ = "migration_client"
 
@@ -23,20 +23,16 @@ class MigrationClient(Base, OAuth2ClientMixin):
     description = Column(String(400))
 
     # required if you need to support client credential
-    user_id = Column(Integer, ForeignKey(User.id, ondelete="CASCADE"))
-    user = relationship(
-        "User",
-        backref=backref(
-            "migration_clients", cascade="all, delete-orphan", passive_deletes=True
-        ),
-    )
+    user_id = Column(Integer)
 
     # this is for internal microservices to skip user grant
     auto_approve = Column(Boolean, default=False)
 
     # public or confidential
     is_confidential = Column(Boolean, default=True)
 
+    issued_at = Column(Integer, nullable=False, default=lambda: int(time.time()))
+
     expires_at = Column(Integer, nullable=False, default=0)
 
     _redirect_uris = Column(Text)