api/v1/revoke_token HTTP status code change#115
Conversation
| if token_type is not None: | ||
| data["token_type_hint"] = token_type | ||
| url = self._requestor.reddit_url + const.REVOKE_TOKEN_PATH | ||
| self._post(url, success_status=codes["no_content"], **data) |
There was a problem hiding this comment.
Seems like this should be 200.
The authorization server responds with HTTP status code 200 if the
token has been revoked successfully or if the client submitted an
invalid token.
There was a problem hiding this comment.
I don't believe there isn't a need to specify that status code because the default is 200.
There was a problem hiding this comment.
Of course. Thanks for pointing that out.
|
Regarding the issue with old refresh tokens remaining active, it is my understanding that Reddit was eventually supposed to make refresh tokens single use. Assuming they do that, then this chain observation won't be a possibility. With that said, it would be nice to get an update from them on that. |
Support 202 HTTP responses
|
Did I accidentally introduce extra commits instead of doing a rebase? This could be fixed if it's done right now |
|
I squashed and merged so everything is fine. |
2 participants
Feature Summary and Justification
Changes success status for token revocation from 204 to 200.
Reddit changed the status code of the response for token revocation attempts that are made with the correct authorization headers (but not necessarily the right token) from 204 to 200, presumably to be RFC 7009 compliant. The only thing from an employee I could find on redditdev about this was this comment.
This doesn't affect prawcore's ability to revoke a token, but it does create an unfortunate bug. Because prawcore sees a 200 instead of a 204, it throws a response exception before it can clear the token. Fortunately, the next request results in a 401 (because the token is now invalid), which makes prawcore clear the token and refresh the access token.
There is a much bigger issue here regarding refresh tokens that I discovered while running tests. Right now, after you authorize a code, you're given an initial refresh token, which I'll call token_0. When you use refresh token 0, you're given refresh token_1, and when you use refresh token_1, you're given token_2, and so on. They form a list, say
[token_0, token_1, token_2, token_3, ...]. Every token you're issued remains valid. Even after you use token n, you're allowed use a previous refresh token, token_i, and using that refresh token will get you token_i+1. (The accompanying access tokens are different every time, of course.) The important thing to note is that revoking a refresh token does NOT revoke any other tokens in the list. This means that if a token is leaked, a malicious actor could upgrade it and keep using it before you get a chance to revoke it, or they could keep keep using that token if you don't know which token was leaked, as revoking the future token doesn't revoke the previous one. At present, PRAW's FileToken manager discards the old token without revoking it, and the old refresh_token setting upgrades the token in memory whenever it sees a new refresh token in a response, but doesn't store the upgraded tokens. This leaves a unknown number of still vaild refresh tokens lying around. Of course, you can revoke the tokens by revoking access to the app via prefs/apps/, but this may be somewhat undesirable, as one app can have many refresh tokens of all different kinds of scopes authorized at the same time. I've emailed Reddit and asked them to add an option to revoke all refresh tokens associated with one known refresh token.References