Check that the Bearer header is properly formatted #491

MattBlack85 · 2017-10-20T09:36:13Z

While testing some things we noticed that passing a header with a typo like Beaver makes the app behaving like it was Bearer. I don't know if this is a desired behaviour or not but it seems odd to me. I made the check a little bit more rigid and added test cases for that.
Feel free to close and kill this if this is an intended behaviour

skion · 2017-12-12T23:14:07Z

tests/oauth2/rfc6749/test_tokens.py

@@ -59,9 +61,17 @@ class TokenTest(TestCase):
    bearer_headers = {
        'Authorization': 'Bearer vF9dft4qmT'
    }
+    fake_bearer_headers = {
+        'Authorization': 'Beaver vF9dft4qmT'


Maybe add some further odd tokens? Such as

BearerNoSpace Bearer Multi Space BeaverNoSpace Beaver Multi Space

MattBlack85 · 2018-02-06T11:36:16Z

@skion I finally added those tests, it seems tho that travis is disabled for the repo now?

skion · 2018-02-06T13:20:13Z

That's potentially due to the moving of the repo, stay tuned on #511.

I think you'll need to apply the same patch to these two lines still, now that #488 has been merged.

MattBlack85 · 2018-02-06T13:36:37Z

@skion thanks, I'll implement the check also for JWT

MattBlack85 · 2018-02-06T14:08:16Z

@skion I think the case Bearer Multi Space or Bearer DoubleSpace are valid header and shouldn't fail the validation cause the header type is valid. Instead what will happen is that the token will fail the validation cause it will be '' or a truncated token. What do you think of these cases?

MattBlack85 · 2018-02-06T14:23:05Z

sorry for the noise, the .split() will give more than 2 elements and it won't pass the validation anyway, my bad

skion · 2018-02-06T14:26:13Z

FWIW: The Bearer token header is described like this:

b64token    = 1*( ALPHA / DIGIT /
                  "-" / "." / "_" / "~" / "+" / "/" ) *"="
credentials = "Bearer" 1*SP b64token

So multiple spaces after the Bearer keyword should not be a problem.

MattBlack85 · 2018-02-06T15:00:18Z

@skion updated my PR, everything should be ok now, let me know in case

skion · 2018-02-06T15:09:31Z

oauthlib/oauth2/rfc6749/tokens.py

        return self.request_validator.validate_jwt_bearer_token(
            token, request.scopes, request)

    def estimate_type(self, request):
-        token = request.headers.get('Authorization', '')[7:]
-        if token.startswith('ey') and token.count('.') in (2, 4):
+        token_type, token, *_ = request.headers.get('Authorization', '').split(' ')


That syntax won't fly on Python 2.7 unfortunately...

And should we use .split() instead of .split(' ') to allow for multispaces then?

Also, this can now raise a ValueError, which I'm not sure is desired (though haven't checked in detail).

dang you're right, too focused on python3. For the multispace I don't know, doesn't 1*SP stays for 1 space?

I know the feeling :)

Re 1*SP is one or more spaces... unnatural indeed.

aaaaa gotcha, fixing also that then! :)

fixed both @skion

JonathanHuot · 2018-03-15T13:07:39Z

Somehow Travis-CI didn't add a link to the build, maybe because it has been canceled before starting (5months ago, by who, why?..)

So I have restarted the build which is passing, and adding a link : https://travis-ci.org/oauthlib/oauthlib/builds/290372743

MattBlack85 · 2018-03-15T13:11:55Z

thanks @JonathanHuot 👍

JonathanHuot · 2018-03-29T14:08:47Z

Would you want to add a new unit test which test headers with multiple spaces between Bearer and <token> ?

As @skion pointed out, it's validated by the RFC.

Once done, the PR LGTM, ready for the merge! :-)

MattBlack85 · 2018-03-29T14:46:12Z

@JonathanHuot I added that header here https://github.com/MattBlack85/oauthlib/blob/38c56b20185ed84989563283f5855f82ef6d91ae/tests/oauth2/rfc6749/test_tokens.py#L68

wasn't this what @skion had in mind?

JonathanHuot · 2018-03-29T15:03:45Z

I let him reply, however I understood that we need to accept several spaces 1*SP after Bearer.

The line you mention is testing several spaces 1*SP after Beaver (a wrong request, then), which is also good to have.

skion · 2018-03-29T18:20:09Z

Yes, I think you're missing that positive test case, maybe make this into a list like the fake headers and add a test with multiple spaces between Bearer and the token there?

MattBlack85 · 2018-04-03T14:47:21Z

@skion looking again at the PR it seems I already made that check here https://github.com/oauthlib/oauthlib/pull/491/files#diff-6096f6f429036633b8a4a4156878b1c4R72 with related test https://github.com/oauthlib/oauthlib/pull/491/files#diff-6096f6f429036633b8a4a4156878b1c4R110

JonathanHuot · 2018-04-03T18:46:51Z

LGTM :-)

skion reviewed Dec 12, 2017

View reviewed changes

skion mentioned this pull request Dec 17, 2017

Openid connect jwt #488

Merged

MattBlack85 changed the title ~~Check that the Bearer header is properly format~~ Check that the Bearer header is properly formatted Feb 6, 2018

skion reviewed Feb 6, 2018

View reviewed changes

skion added this to the 3.0.0 milestone Mar 18, 2018

Check that the Bearer header is properly formatted

38c56b2

skion modified the milestones: 3.0.0, 2.1.0 Mar 20, 2018

JonathanHuot approved these changes Apr 3, 2018

View reviewed changes

JonathanHuot and others added 2 commits April 3, 2018 20:49

Merge branch 'master' into proper_bearer_header_check

02c1428

Merge branch 'master' into proper_bearer_header_check

32622a3

skion merged commit 27702f4 into oauthlib:master May 26, 2018

bungoume added a commit to bungoume/oauthlib that referenced this pull request May 7, 2019

Add token tests from oauthlib#491

d4a3a41

bungoume mentioned this pull request May 7, 2019

token_type should be case insensitive #674

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check that the Bearer header is properly formatted #491

Check that the Bearer header is properly formatted #491

MattBlack85 commented Oct 20, 2017

skion Dec 12, 2017

MattBlack85 Dec 13, 2017

MattBlack85 commented Feb 6, 2018

skion commented Feb 6, 2018 •

edited

MattBlack85 commented Feb 6, 2018

MattBlack85 commented Feb 6, 2018 •

edited

MattBlack85 commented Feb 6, 2018

skion commented Feb 6, 2018

MattBlack85 commented Feb 6, 2018

skion Feb 6, 2018 •

edited

skion Feb 6, 2018

MattBlack85 Feb 6, 2018

skion Feb 6, 2018

MattBlack85 Feb 6, 2018

MattBlack85 Feb 6, 2018

JonathanHuot commented Mar 15, 2018

MattBlack85 commented Mar 15, 2018

JonathanHuot commented Mar 29, 2018

MattBlack85 commented Mar 29, 2018 •

edited

JonathanHuot commented Mar 29, 2018

skion commented Mar 29, 2018 •

edited

MattBlack85 commented Apr 3, 2018

JonathanHuot commented Apr 3, 2018

Check that the Bearer header is properly formatted #491

Check that the Bearer header is properly formatted #491

Conversation

MattBlack85 commented Oct 20, 2017

skion Dec 12, 2017

Choose a reason for hiding this comment

MattBlack85 Dec 13, 2017

Choose a reason for hiding this comment

MattBlack85 commented Feb 6, 2018

skion commented Feb 6, 2018 • edited

MattBlack85 commented Feb 6, 2018

MattBlack85 commented Feb 6, 2018 • edited

MattBlack85 commented Feb 6, 2018

skion commented Feb 6, 2018

MattBlack85 commented Feb 6, 2018

skion Feb 6, 2018 • edited

Choose a reason for hiding this comment

skion Feb 6, 2018

Choose a reason for hiding this comment

MattBlack85 Feb 6, 2018

Choose a reason for hiding this comment

skion Feb 6, 2018

Choose a reason for hiding this comment

MattBlack85 Feb 6, 2018

Choose a reason for hiding this comment

MattBlack85 Feb 6, 2018

Choose a reason for hiding this comment

JonathanHuot commented Mar 15, 2018

MattBlack85 commented Mar 15, 2018

JonathanHuot commented Mar 29, 2018

MattBlack85 commented Mar 29, 2018 • edited

JonathanHuot commented Mar 29, 2018

skion commented Mar 29, 2018 • edited

MattBlack85 commented Apr 3, 2018

JonathanHuot commented Apr 3, 2018

skion commented Feb 6, 2018 •

edited

MattBlack85 commented Feb 6, 2018 •

edited

skion Feb 6, 2018 •

edited

MattBlack85 commented Mar 29, 2018 •

edited

skion commented Mar 29, 2018 •

edited