rat: improve error handling for secret link clashes

invenio_rdm_records/tokens/errors.py

@@ -19,25 +19,43 @@ class ResourceAccessTokenError(RESTException):
 class MissingTokenIDError(ResourceAccessTokenError):
     """Resource access token error for missing token ID."""
 
-    description = _('Missing "kid" key with personal access token ID in JWT header.')
+    description = _(
+        'Missing "kid" key with personal access token ID in JWT header of '
+        "resource access token."
+    )
 
 
 class InvalidTokenIDError(ResourceAccessTokenError):
     """Resource access token error for invalid token ID."""
 
-    description = _('"kid" JWT header value not a valid personal access token ID.')
+    description = _(
+        '"kid" JWT header value of resource access token not a valid personal '
+        "access token ID."
+    )
+
+
+class TokenDecodeError(ResourceAccessTokenError):
+    """Resource access token error for token decoding errors."""
+
+    description = _("Failed to decode resource access token.")
 
 
 class InvalidTokenError(ResourceAccessTokenError):
     """Resource access token error for invalid token."""
 
-    description = _("The token is invalid.")
+    description = _("The resource access token is invalid.")
+
+
+class InvalidTokenSubjectError(ResourceAccessTokenError):
+    """Resource access token error for invalid subject of a token."""
+
+    description = _("The resource access token subject is invalid.")
 
 
 class ExpiredTokenError(InvalidTokenError):
     """Resource access token error for expired token."""
 
-    description = _("The token is expired.")
+    description = _("The resource access token is expired.")
 
 
 class RATFeatureDisabledError(ResourceAccessTokenError):

invenio_rdm_records/tokens/resource_access.py

@@ -19,6 +19,7 @@
     InvalidTokenError,
     InvalidTokenIDError,
     MissingTokenIDError,
+    TokenDecodeError,
 )
 from .scopes import tokens_generate_scope
 
@@ -37,6 +38,8 @@ def validate_rat(token):
     try:
         headers = jwt.get_unverified_header(token)
         access_token_id = headers.get("kid")
+    except jwt.DecodeError:
+        raise TokenDecodeError()
     except jwt.InvalidTokenError:
         raise InvalidTokenError()
 
@@ -71,6 +74,8 @@ def validate_rat(token):
         if (issued_at + token_lifetime) < datetime.utcnow():
             raise ExpiredTokenError()
 
+    except jwt.DecodeError:
+        raise TokenDecodeError()
     except jwt.InvalidTokenError:
         raise InvalidTokenError()
 

invenio_rdm_records/utils.py

@@ -22,7 +22,11 @@
 from .requests.access.permissions import AccessRequestTokenNeed
 from .secret_links import LinkNeed, SecretLink
 from .tokens import RATNeed, validate_rat
-from .tokens.errors import InvalidTokenError, RATFeatureDisabledError
+from .tokens.errors import (
+    InvalidTokenSubjectError,
+    RATFeatureDisabledError,
+    TokenDecodeError,
+)
 
 
 def get_or_create_user(email):
@@ -83,34 +87,49 @@ def get(self, key, default=None):
 
 def verify_token(identity):
     """Verify the token and provide identity with corresponding need."""
-    token = request.args.get("token", session.get("token", None))
+    secret_link_token_arg = "token"
+    token = request.args.get(
+        secret_link_token_arg,
+        session.get(secret_link_token_arg, None),
+    )
+    has_secret_link_token = False
 
-    session["token"] = token
     if token:
         try:
             data = SecretLink.load_token(token)
             if data:
                 identity.provides.add(LinkNeed(data["id"]))
+            session[secret_link_token_arg] = token
+            has_secret_link_token = True
         except SignatureExpired:
             flash(_("Your shared link has expired."))
 
-    resource_access_token = request.args.get(
-        current_app.config.get("RDM_RESOURCE_ACCESS_TOKEN_REQUEST_ARG", None), None
-    )
-    if resource_access_token:
-        if not current_app.config.get("RDM_RESOURCE_ACCESS_TOKENS_ENABLED", False):
+    # NOTE: This logic is getting very complex becuase of possible arg naming conflicts
+    # for the Zenodo use-case. It can be simplified once the conflict changes
+    rat_enabled = current_app.config.get("RDM_RESOURCE_ACCESS_TOKENS_ENABLED", False)
+    rat_arg = current_app.config.get("RDM_RESOURCE_ACCESS_TOKEN_REQUEST_ARG", None)
+    # we can have a "naming conflict" if both secret links and RATs use the same arg key
+    rat_arg_name_conflict = rat_arg == secret_link_token_arg
+    rat = request.args.get(rat_arg, None)
+    if rat and not (rat_arg_name_conflict and has_secret_link_token):
+        if not rat_enabled:
             raise RATFeatureDisabledError()
 
-        rat_signer, payload = validate_rat(resource_access_token)
-        schema_cls = current_app.config.get("RDM_RESOURCE_ACCESS_TOKENS_SUBJECT_SCHEMA")
-        if schema_cls:
-            try:
-                rat_need_data = schema_cls().load(payload)
-            except ValidationError:
-                raise InvalidTokenError()
-        else:
-            rat_need_data = payload
-        identity.provides.add(RATNeed(rat_signer, **rat_need_data))
+        try:
+            rat_signer, payload = validate_rat(rat)
+            schema_cls = current_app.config.get(
+                "RDM_RESOURCE_ACCESS_TOKENS_SUBJECT_SCHEMA"
+            )
+            if schema_cls:
+                try:
+                    rat_need_data = schema_cls().load(payload)
+                except ValidationError:
+                    raise InvalidTokenSubjectError()
+            else:
+                rat_need_data = payload
+            identity.provides.add(RATNeed(rat_signer, **rat_need_data))
+        except TokenDecodeError:
+            pass
 
     access_request_token = request.args.get(
         "access_request_token", session.get("access_request_token", None)

tests/tokens/test_resource_access.py

@@ -19,6 +19,7 @@
     InvalidTokenError,
     InvalidTokenIDError,
     MissingTokenIDError,
+    TokenDecodeError,
 )
 from invenio_rdm_records.tokens.scopes import tokens_generate_scope
 
@@ -105,7 +106,7 @@ def create_record_w_file(client, record, headers, is_published=True):
 def test_rat_validation_failed(app, db, uploader, superuser_identity, oauth2_client):
     """Test possible validation failings of Resource access tokens."""
     # case: no headers in the token
-    with pytest.raises(InvalidTokenError):
+    with pytest.raises(TokenDecodeError):
         validate_rat("not_a.valid_jwt")
 
     # case: headers don't have "kid"
@@ -528,7 +529,7 @@ def test_rec_files_permissions_with_rat_invalid_token_error(
         record_file_url, query_string={"resource_access_token": rat_token}
     )
     assert res.status_code == 400
-    assert res.json["message"] == "The token is invalid."
+    assert res.json["message"] == "The resource access token is invalid."
     uploader.logout(client)
 
     # other user with token can NOT access the files
@@ -537,13 +538,13 @@ def test_rec_files_permissions_with_rat_invalid_token_error(
         record_file_url, query_string={"resource_access_token": rat_token}
     )
     assert res.status_code == 400
-    assert res.json["message"] == "The token is invalid."
+    assert res.json["message"] == "The resource access token is invalid."
     test_user.logout(client)
 
     # anonymous user with token can NOT access the files
     res = client.get(record_file_url, query_string={"resource_access_token": rat_token})
     assert res.status_code == 400
-    assert res.json["message"] == "The token is invalid."
+    assert res.json["message"] == "The resource access token is invalid."
 
 
 def test_rec_files_permissions_with_rat_missing_token_id_error(
@@ -589,7 +590,7 @@ def test_rec_files_permissions_with_rat_missing_token_id_error(
     assert res.status_code == 400
     assert (
         res.json["message"]
-        == 'Missing "kid" key with personal access token ID in JWT header.'
+        == 'Missing "kid" key with personal access token ID in JWT header of resource access token.'
     )
     uploader.logout(client)
 
@@ -601,7 +602,7 @@ def test_rec_files_permissions_with_rat_missing_token_id_error(
     assert res.status_code == 400
     assert (
         res.json["message"]
-        == 'Missing "kid" key with personal access token ID in JWT header.'
+        == 'Missing "kid" key with personal access token ID in JWT header of resource access token.'
     )
     test_user.logout(client)
 
@@ -610,7 +611,7 @@ def test_rec_files_permissions_with_rat_missing_token_id_error(
     assert res.status_code == 400
     assert (
         res.json["message"]
-        == 'Missing "kid" key with personal access token ID in JWT header.'
+        == 'Missing "kid" key with personal access token ID in JWT header of resource access token.'
     )
 
 
@@ -657,7 +658,7 @@ def test_rec_files_permissions_with_rat_invalid_token_id_error(
     assert res.status_code == 400
     assert (
         res.json["message"]
-        == '"kid" JWT header value not a valid personal access token ID.'
+        == '"kid" JWT header value of resource access token not a valid personal access token ID.'
     )
     uploader.logout(client)
 
@@ -669,7 +670,7 @@ def test_rec_files_permissions_with_rat_invalid_token_id_error(
     assert res.status_code == 400
     assert (
         res.json["message"]
-        == '"kid" JWT header value not a valid personal access token ID.'
+        == '"kid" JWT header value of resource access token not a valid personal access token ID.'
     )
     test_user.logout(client)
 
@@ -678,7 +679,7 @@ def test_rec_files_permissions_with_rat_invalid_token_id_error(
     assert res.status_code == 400
     assert (
         res.json["message"]
-        == '"kid" JWT header value not a valid personal access token ID.'
+        == '"kid" JWT header value of resource access token not a valid personal access token ID.'
     )
 
 
@@ -716,7 +717,7 @@ def test_rec_files_permissions_with_rat_expired_token_error(
         record_file_url, query_string={"resource_access_token": rat_token}
     )
     assert res.status_code == 400
-    assert res.json["message"] == "The token is expired."
+    assert res.json["message"] == "The resource access token is expired."
     uploader.logout(client)
 
     # other user with token can NOT access the files
@@ -725,13 +726,13 @@ def test_rec_files_permissions_with_rat_expired_token_error(
         record_file_url, query_string={"resource_access_token": rat_token}
     )
     assert res.status_code == 400
-    assert res.json["message"] == "The token is expired."
+    assert res.json["message"] == "The resource access token is expired."
     test_user.logout(client)
 
     # anonymous user with token can NOT access the files
     res = client.get(record_file_url, query_string={"resource_access_token": rat_token})
     assert res.status_code == 400
-    assert res.json["message"] == "The token is expired."
+    assert res.json["message"] == "The resource access token is expired."
 
 
 def test_rec_files_permissions_with_rat_wrong_file(