miguelgrinberg / Flask-HTTPAuth

Simple extension that provides Basic, Digest and Token HTTP authentication for Flask routes
MIT License
1.27k stars 228 forks source link

Fix for Flask 2.3.0 #159

Closed NORXND closed 1 year ago

NORXND commented 1 year ago

Recently, Flask and Werkzeug updated to version 2.3.0 which broke the token authentication in Flask-HTTPAuth. After my careful look in the library and long search for the cause, I have discovered by due to the latest changes in Werkzeug's Authorization class, there are some problems with tokens.

What I am looking into is:

Both classes have type, parameters, and token attributes. The token attribute supports auth schemes that use a single opaque token rather than key=value parameters, such as Bearer.

Neither class is a dict anymore, although they still implement getting, setting, and deleting auth[key] and auth.key syntax, as well as auth.get(key) and key in auth.

I have successfully found the solution to this problem by changing how the token is preserved in the Authorization class.

Formerly it was:

auth = Authorization(auth_type, {'token': token})

After my consideration, I have come to the conclusion that changing it to the form below will fix the problem:

auth = Authorization(auth_type, token=token)

What I also changed was the method of retrieving the token from:

token = auth['token']

to:

token = auth.token

In addition to my understanding, it's also the preferred way of setting, keeping and retrieving the token as of now.

I have also updated all applicable tests (didn't find anything to change in examples or docs) so they use that method.

I made every effort I could to not introduce any breaking change in a code, although I would recommend checking it once again to make sure everything is as it should be.

Before committing I ran the tests with the latest versions of every package needed: Test Passed
test_basic_custom_realm OK
test_basic_get_password OK
test_basic_hashed_password OK
test_basic_verify_password_async OK
test_basic_verify_password OK
test_digest_custom_realm OK
test_digest_get_password OK
test_digest_ha1_password OK
test_digest_no_qop OK
test_error_responses OK
test_multi_async OK (Modified)
test_multi OK (Modified)
test_roles_async OK
test_roles OK
test_token_async OK
test_token OK
lukio commented 1 year ago

Hi! @NORXND @miguelgrinberg, this PR is related to #160

miguelgrinberg commented 1 year ago

Not sure I can accept this, as it will break every user that is not on Werkzeug 2.3.

NORXND commented 1 year ago

Not sure I can accept this, as it will break every user that is not on Werkzeug 2.3.

Oh, you pointed out the problem I have unfortunately not considered and for sure it's not something that should be ignored.

If we assume Werkzeug will keep their version format, couldn't we make use of werkzeug.__version__ and then wrap changes in if's?

Or maybe create something like a subpackage (like Flask[async]) that would be for Workzeug 2.3.0+

I'm looking forward to any ideas that could make this acceptable for the whole userbase!

Thanks!

davidism commented 1 year ago

Sorry, I thought I had checked this package when refactoring, but I didn't consider that adding support for token auth would break custom support for it.

If you always assign auth.token, the checks for auth.token instead of auth["token"] elsewhere would work for all versions. No need to check werkzeug.__version__, an invalid argument (old Werkzeug) will raise a TypeError.

if self.header is None or self.header == 'Authorization':
    auth = request.authorization

    if auth is None:
        scheme, _, token = request.headers.get("Authorization", "").partition(" ")

        if scheme and token:
            try:
                auth = Authorization(scheme, token=token)
            except TypeError:
                # Werkzeug < 2.3
                auth = Authorization(scheme)
                auth.token = token
elif self.header in request.headers:
    try:
        auth = Authorization(self.scheme, token=request.headers[self.header])
    except TypeError:
        # Werkzeug < 2.3
        auth = Authorization(self.scheme)
        auth.token = request.headers[self.header]
else:
    auth = None
NORXND commented 1 year ago

@davidism Just added this to the PR, hope this will clear everything up and the module will be able to work again. Thank you for your comment!

codecov-commenter commented 1 year ago

Codecov Report

Merging #159 (ee2df61) into main (15498a3) will decrease coverage by 4.65%. The diff coverage is 27.27%.

@@             Coverage Diff             @@
##              main     #159      +/-   ##
===========================================
- Coverage   100.00%   95.35%   -4.65%     
===========================================
  Files            1        1              
  Lines          272      280       +8     
  Branches        43       43              
===========================================
- Hits           272      267       -5     
- Misses           0       12      +12     
- Partials         0        1       +1     
Impacted Files Coverage Δ
src/flask_httpauth.py 95.35% <27.27%> (-4.65%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

miguelgrinberg commented 1 year ago

I have implemented this fix in a slightly different way. Also added a new matrix dimension to the tests for pre-2.3 and post-2.3 Flask/Werkzeug releases. Not too happy about complicating my build, but I don't see any other way to prevent another breakage. Thanks, everyone!