JWTIdentity raises common error JWTIdentityError

[Maillol]

Hello,

What do these changes do?

All exceptions raised by the JWTIdentityPolicy class are subclasses of the PyJWTError type

Are there changes in behavior for the user?

None, multiple inheritance with the old error types has been used to maintain backward compatibility.

Related issue number

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes

note: Currently, JWTIdentityPolicy is not documented. Would you like me to add documentation and update the demo?

[Dreamsorcerer]

Seems like a sensible addition, though I'd note that the exception is not mentioned in the PyJWT documentation (except buried in a changelog), which probably limits the ability of other users to discover this.

[codecov]

Codecov Report

Attention: Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.

Project coverage is 94.17%. Comparing base (3ad4302) to head (9b66e29).
Report is 139 commits behind head on master.

Files with missing lines	Patch %	Lines
tests/test_jwt_identity.py	94.44%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #840      +/-   ##
==========================================
+ Coverage   94.13%   94.17%   +0.03%     
==========================================
  Files          11       11              
  Lines         597      618      +21     
  Branches       30       22       -8     
==========================================
+ Hits          562      582      +20     
- Misses         26       27       +1     
  Partials        9        9              

Flag	Coverage Δ
unit	94.17% <95.45%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Maillol]

Seems like a sensible addition, though I'd note that the exception is not mentioned in the PyJWT documentation (except buried in a changelog), which probably limits the ability of other users to discover this.

jwt will do a lot of checking at the payload level and may throw exceptions such as, InvalidSignatureError or ExpiredSignatureError. All exceptions are PyJWTError subclasses.

I use a middleware to catch PyJWTError and send 401 HTTP response. This pull request allow to catch invalid authorization scheme with the same middleware.

[Dreamsorcerer]

All exceptions are PyJWTError subclasses.

Yes, my point is that other users may struggle to figure this out, given it does not appear in the docs. Maybe worth a PR to that library as well to document it properly.

[Maillol]

The CI is failing, the mypy command did not succeed.

Run mypy
aiohttp_security/jwt_identity.py:17:20: error: Incompatible types in assignment
(expression has type "tuple[type[ValueError]]", variable has type
"tuple[type[PyJWTError], type[ValueError]]")  [assignment]
        _bases_error = (ValueError, )
                       ^~~~~~~~~~~~~~
aiohttp_security/jwt_identity.py:26:34: error: Invalid base class  [misc]
    class InvalidAuthorizationScheme(*_bases_error):
                                     ^
Found 2 errors in 1 file (checked 27 source files)
Error: Process completed with exit code 1.

I can declare the InvalidAuthorizationScheme exception like this to make Mypy pass.

InvalidAuthorizationScheme = type("InvalidAuthorizationScheme",
                                  (jwt.PyJWTError, ValueError) if HAS_JWT else (ValueError,), {})

But it feels a bit tricky to me.

I might have another solution. Currently, PyJWT is only required if JWTIdentityPolicy is used, and we cannot import jwt directly in aiohttp_security/jwt_identity.py because jwt_identity is imported in aiohttp_security/__init__.py.

We can preserve this behavior by defining a __getattr__ method in the aiohttp_security/init.py module (PEP 562) like this:

def __getattr__(name: str) -> Any:
    if name == "JWTIdentityPolicy":
        try:
            import jwt
        except ImportError as error:
            raise RuntimeError('Please install `PyJWT`') from error

        from .jwt_identity import JWTIdentityPolicy
        return JWTIdentityPolicy

and remove try/except around import jwt in aiohttp_security/jwt_identity.py.

Do you have a preference for the approach?

[Dreamsorcerer]

I think it's probably fine as it is. I'll sort out the mypy errors later, just a little busy with other things right now.