Code rewrites and alembic migration

.secrets.baseline

@@ -198,7 +198,7 @@
         "filename": "fence/utils.py",
         "hashed_secret": "8318df9ecda039deac9868adf1944a29a95c7114",
         "is_verified": false,
-        "line_number": 128
+        "line_number": 129
       }
     ],
     "migrations/versions/a04a70296688_non_unique_client_name.py": [
@@ -241,14 +241,14 @@
         "filename": "tests/conftest.py",
         "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9",
         "is_verified": false,
-        "line_number": 1556
+        "line_number": 1557
       },
       {
         "type": "Base64 High Entropy String",
         "filename": "tests/conftest.py",
         "hashed_secret": "227dea087477346785aefd575f91dd13ab86c108",
         "is_verified": false,
-        "line_number": 1579
+        "line_number": 1580
       }
     ],
     "tests/credentials/google/test_credentials.py": [
@@ -312,7 +312,7 @@
         "filename": "tests/login/test_fence_login.py",
         "hashed_secret": "d300421e208bfd0d432294de15169fd9b8975def",
         "is_verified": false,
-        "line_number": 48
+        "line_number": 49
       }
     ],
     "tests/migrations/test_a04a70296688.py": [
@@ -368,5 +368,5 @@
       }
     ]
   },
-  "generated_at": "2023-05-15T16:30:09Z"
+  "generated_at": "2023-09-01T18:49:42Z"
 }

bin/fence_create.py

@@ -329,6 +329,9 @@ def parse_arguments():
         help='scopes to include in the token (e.g. "user" or "data")',
     )
     token_create.add_argument("--exp", help="time in seconds until token expiration")
+    token_create.add_argument(
+        "--client_id", help="Client Id, required to generate refresh token"
+    )
 
     force_link_google = subparsers.add_parser("force-link-google")
     force_link_google.add_argument(
@@ -581,6 +584,7 @@ def main():
             username=args.username,
             scopes=args.scopes,
             expires_in=args.exp,
+            client_id=args.client_id,
         )
         token_type = str(args.type).strip().lower()
         if token_type == "access_token" or token_type == "access":

fence/blueprints/login/fence_login.py

@@ -30,19 +30,22 @@ def __init__(self):
 
     def get(self):
         """Handle ``GET /login/fence``."""
-        oauth2_redirect_uri = flask.current_app.fence_client.client_kwargs.get(
-            "redirect_uri"
-        )
+
+        # OAuth class can have mutliple clients
+        client = flask.current_app.fence_client._clients[
+            flask.current_app.config["OPENID_CONNECT"]["fence"]["name"]
+        ]
+
+        oauth2_redirect_uri = client.client_kwargs.get("redirect_uri")
+
         redirect_url = flask.request.args.get("redirect")
         if redirect_url:
             validate_redirect(redirect_url)
             flask.session["redirect"] = redirect_url
-        (
-            authorization_url,
-            state,
-        ) = flask.current_app.fence_client.generate_authorize_redirect(
-            oauth2_redirect_uri, prompt="login"
-        )
+
+        rv = client.create_authorization_url(oauth2_redirect_uri, prompt="login")
+
+        authorization_url = rv["url"]
 
         # add idp parameter to the authorization URL
         if "idp" in flask.request.args:
@@ -57,7 +60,7 @@ def get(self):
                 flask.session["shib_idp"] = shib_idp
             authorization_url = add_params_to_uri(authorization_url, params)
 
-        flask.session["state"] = state
+        flask.session["state"] = rv["state"]
         return flask.redirect(authorization_url)
 
 
@@ -91,16 +94,21 @@ def get(self):
         # 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
-        tokens = flask.current_app.fence_client.fetch_access_token(
-            redirect_uri, **flask.request.args.to_dict()
+
+        # 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")
+
+        tokens = client.fetch_access_token(
+            oauth2_redirect_uri, **flask.request.args.to_dict()
         )
 
         try:
             # For multi-Fence setup with two Fences >=5.0.0
             id_token_claims = validate_jwt(
                 tokens["id_token"],
-                aud=self.client.client_id,
+                aud=client.client_id,
                 scope={"openid"},
                 purpose="id",
                 attempt_refresh=True,

fence/blueprints/login/utils.py

@@ -21,7 +21,10 @@ def allowed_login_redirects():
     with flask.current_app.db.session as session:
         clients = session.query(Client).all()
         for client in clients:
-            allowed.extend(client.redirect_uris)
+            if isinstance(client.redirect_uris, list):
+                allowed.extend(client.redirect_uris)
+            elif isinstance(client.redirect_uris, str):
+                allowed.append(client.redirect_uris)
     return {domain(url) for url in allowed}
 
 

fence/blueprints/oauth2.py

@@ -32,7 +32,10 @@
 from fence.utils import clear_cookies
 from fence.user import get_current_user
 from fence.config import config
-
+from authlib.oauth2.rfc6749.errors import (
+    InvalidScopeError,
+)
+from fence.utils import validate_scopes
 
 blueprint = flask.Blueprint("oauth2", __name__)
 
@@ -114,14 +117,21 @@ def authorize(*args, **kwargs):
         return flask.redirect(login_url)
 
     try:
-        grant = server.validate_consent_request(end_user=user)
+        grant = server.get_consent_grant(end_user=user)
     except OAuth2Error as e:
         raise Unauthorized("Failed to authorize: {}".format(str(e)))
 
     client_id = grant.client.client_id
     with flask.current_app.db.session as session:
         client = session.query(Client).filter_by(client_id=client_id).first()
 
+    # Need to do scope check here now due to our design of putting allowed_scope on client
+    # Authlib now put allowed scope on OIDC server side which doesn't work with our design without modification to the lib
+    # Doing the scope check here because both client and grant is available here
+    # Either Get or Post request
+    request_scopes = flask.request.args.get("scope") or flask.request.form.get("scope")
+    validate_scopes(request_scopes, client)
+
     # TODO: any way to get from grant?
     confirm = flask.request.form.get("confirm") or flask.request.args.get("confirm")
     if client.auto_approve:

fence/models.py

@@ -60,6 +60,7 @@
 from fence import logger
 from fence.config import config
 from fence.errors import UserError
+import jwt
 
 
 def query_for_user(session, username):
@@ -156,7 +157,7 @@ def get_client_expires_at(expires_in, grant_types):
             # `timestamp()` already converts to UTC
             expires_at = (datetime.now() + timedelta(days=expires_in)).timestamp()
 
-    if "client_credentials" in grant_types.split("\n"):
+    if "client_credentials" in grant_types:
         if not expires_in or expires_in <= 0 or expires_in > 366:
             logger.warning(
                 "Credentials with the 'client_credentials' grant which will be used externally are required to expire within 12 months. Use the `--expires-in` parameter to add an expiration."
@@ -194,9 +195,9 @@ class Client(Base, OAuth2ClientMixin):
 
     __tablename__ = "client"
 
-    client_id = Column(String(40), primary_key=True)
+    client_id = Column(String(48), primary_key=True, index=True)
     # this is hashed secret
-    client_secret = Column(String(60), unique=True, index=True, nullable=True)
+    client_secret = Column(String(120), unique=True, index=True, nullable=True)
 
     # human readable name
     name = Column(String(40), nullable=False)
@@ -217,19 +218,12 @@ class Client(Base, OAuth2ClientMixin):
     # public or confidential
     is_confidential = Column(Boolean, default=True)
 
-    # NOTE: DEPRECATED
-    # Client now uses `redirect_uri` column, from authlib client model
-    _redirect_uris = Column(Text)
-
-    _allowed_scopes = Column(Text, nullable=False, default="")
+    expires_at = Column(Integer, nullable=False, default=0)
 
+    # Deprecated, keeping these around in case it is needed later
     _default_scopes = Column(Text)
     _scopes = ["compute", "storage", "user"]
 
-    expires_at = Column(Integer, nullable=False, default=0)
-
-    # note that authlib adds a response_type column which is not used here
-
     def __init__(self, client_id, expires_in=0, **kwargs):
         """
         NOTE that for authlib, the client must have an attribute ``redirect_uri`` which
@@ -241,19 +235,18 @@ def __init__(self, client_id, expires_in=0, **kwargs):
         if "allowed_scopes" in kwargs:
             allowed_scopes = kwargs.pop("allowed_scopes")
             if isinstance(allowed_scopes, list):
-                kwargs["_allowed_scopes"] = " ".join(allowed_scopes)
+                client_metadata["scope"] = " ".join(allowed_scopes)
             else:
-                kwargs["_allowed_scopes"] = allowed_scopes
-            client_metadata["scope"] = kwargs["_allowed_scopes"]
+                client_metadata["scope"] = allowed_scopes
 
         # redirect uri is now part of authlibs client_metadata
         if "redirect_uris" in kwargs:
             redirect_uris = kwargs.pop("redirect_uris")
             if isinstance(redirect_uris, list):
-                # redirect_uris is now part of the json object
-                client_metadata["redirect_uris"] = "\n".join(redirect_uris)
-            else:
+                # redirect_uris is now part of the metadata json object
                 client_metadata["redirect_uris"] = redirect_uris
+            else:
+                client_metadata["redirect_uris"] = [redirect_uris]
 
         # default grant types to allow for auth code flow and resfreshing
         grant_types = kwargs.pop("grant_types", None) or [
@@ -262,10 +255,10 @@ def __init__(self, client_id, expires_in=0, **kwargs):
         ]
         # grant types is now part of authlibs client_metadata
         if isinstance(grant_types, list):
-            client_metadata["grant_types"] = "\n".join(grant_types)
-        else:
-            # assume it's already in correct format
             client_metadata["grant_types"] = grant_types
+        else:
+            # assume it's already in correct format and make it a list
+            client_metadata["grant_types"] = [grant_types]
 
         supported_grant_types = [
             "authorization_code",
@@ -275,10 +268,10 @@ def __init__(self, client_id, expires_in=0, **kwargs):
         ]
         assert all(
             grant_type in supported_grant_types
-            for grant_type in client_metadata["grant_types"].split("\n")
+            for grant_type in client_metadata["grant_types"]
         ), f"Grant types '{client_metadata['grant_types']}' are not in supported types {supported_grant_types}"
 
-        if "authorization_code" in client_metadata["grant_types"].split("\n"):
+        if "authorization_code" in client_metadata["grant_types"]:
             assert kwargs.get("user") or kwargs.get(
                 "user_id"
             ), "A username is required for the 'authorization_code' grant"
@@ -294,13 +287,10 @@ def __init__(self, client_id, expires_in=0, **kwargs):
             # assume it's already in correct format
             client_metadata["response_types"] = response_types
 
-        # scope is now part of authlib's client_metadata
-        scope = kwargs.pop("scope", None)
-        if isinstance(response_types, list):
-            client_metadata["scope"] = "\n".join(scope)
-        else:
-            # assume it's already in correct format
-            client_metadata["scope"] = scope
+        if "token_endpoint_auth_method" in kwargs:
+            client_metadata["token_endpoint_auth_method"] = kwargs.pop(
+                "token_endpoint_auth_method"
+            )
 
         expires_at = get_client_expires_at(
             expires_in=expires_in, grant_types=client_metadata["grant_types"]
@@ -331,10 +321,7 @@ def client_type(self):
             return "public"
         return "confidential"
 
-    @property
-    def default_redirect_uri(self):
-        return self.redirect_uris[0]
-
+    ##Deprecated, Not called anywhere currently
     @property
     def default_scopes(self):
         if self._default_scopes:
@@ -363,14 +350,18 @@ def check_requested_scopes(self, scopes):
             return False
         return set(self.allowed_scopes).issuperset(scopes)
 
-    def check_token_endpoint_auth_method(self, method):
+    # Replaces Authlib method
+    def check_endpoint_auth_method(self, method, endpoint):
         """
         Only basic auth is supported. If anything else gets added, change this
         """
-        protected_types = [ClientAuthType.basic.value, ClientAuthType.post.value]
-        return (self.is_confidential and method in protected_types) or (
-            not self.is_confidential and method == ClientAuthType.none.value
-        )
+        if endpoint == "token":
+            protected_types = [ClientAuthType.basic.value, ClientAuthType.post.value]
+            return (self.is_confidential and method in protected_types) or (
+                not self.is_confidential and method == ClientAuthType.none.value
+            )
+
+        return True
 
     def validate_scopes(self, scopes):
         scopes = scopes[0].split(",")
@@ -838,3 +829,14 @@ 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,6 +2,7 @@
 import authlib.oauth2.rfc7009
 import bcrypt
 import flask
+from fence.models import JWTToken
 
 from cdislogging import get_logger
 
@@ -18,20 +19,20 @@ class RevocationEndpoint(authlib.oauth2.rfc7009.RevocationEndpoint):
     server should handle requests for token revocation.
     """
 
-    def query_token(self, token, token_type_hint, client):
+    def query_token(self, token, token_type_hint):
         """
         Look up a token.
 
         Since all tokens are JWT, just return the token.
         """
-        return token
+        return JWTToken(token)
 
-    def revoke_token(self, token):
+    def revoke_token(self, token, request):
         """
         Revoke a token.
         """
         try:
-            fence.jwt.blacklist.blacklist_encoded_token(token)
+            fence.jwt.blacklist.blacklist_encoded_token(token.encoded_string)
         except BlacklistingError as err:
             logger.info(
                 "Token provided for revocation is not valid. "