New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

/auth/me status code 200 after token expiration #118

Closed
Jeffwhen opened this Issue Jul 6, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@Jeffwhen
Copy link

Jeffwhen commented Jul 6, 2018

Request to /auth/me is not rejected with 403 after token expiration. Is this intended?

@Jeffwhen Jeffwhen changed the title `/auth/me` status code 200 after token expiration /auth/me status code 200 after token expiration Jul 6, 2018

@ahopkins

This comment has been minimized.

Copy link
Owner

ahopkins commented Jul 8, 2018

@Jeffwhen Thanks for pointing this out. I would not call this intended, because it certainly would not be intuitive. However, now that you mention it, I 100% see how this happens. I am going to put together a patch for this and will release it in the next day or two. This is a security risk in my opinion as it allows potentially for data to be leaked unexpectedly. After all, that is the whole point that these tokens expire and become useless.

Thank you very much for your contribution, and if you would like to take a stab at adding the protection feel free to submit a PR.

UPDATE

@Jeffwhen So, I played around with this some more and gave it a closer look. Initially, I was thinking that this made sense because the protected decorator is not applied to the internal endpoints.

However, the retrieve_user handler receives the payload from Authentication.extract_payload.

payload = self.instance.auth.extract_payload(request)

Calling extract_payload with the default arguments should cause the token decoding to bubble up to a 403 with a body:

    {'reasons': ['Signature has expired'], 'exception': 'Unauthorized'}

What version of Sanic JWT do you have running? Do you have a custom /auth/me endpoint?

@Jeffwhen

This comment has been minimized.

Copy link
Author

Jeffwhen commented Jul 9, 2018

That method extract_payload is throwless. In this case, it only returns None after token expiration. The version is 1.1.2.

@ahopkins

This comment has been minimized.

Copy link
Owner

ahopkins commented Aug 5, 2018

    with freeze_time(datetime.utcnow() + timedelta(seconds=(60 * 35))):
        assert isinstance(exp, datetime)
        assert datetime.utcnow() > exp

        _, response = sanic_app.test_client.get(
            "/auth/me",
            headers={"Authorization": "Bearer {}".format(access_token)},
        )

        assert response.status == 403
        assert "Signature has expired" in response.json.get("reasons")

        _, response = sanic_app.test_client.get(
            "/protected/user",
            headers={"Authorization": "Bearer {}".format(access_token)},
        )

        assert response.status == 403
        assert "Signature has expired" in response.json.get("reasons")

I added a new test for this. See above.

I am about to push some changes that will get included in the next release. See also #116. One of the big changes you will see is adding protected decorator to /auth/me and (in general) how exceptions are being reported.

@ahopkins ahopkins added this to the v1.2 milestone Aug 5, 2018

@ahopkins ahopkins closed this Aug 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment