Skip to content
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

JWT tween is too restrictive and block some read actions #718

Open
asaunier opened this issue Jan 31, 2018 · 5 comments
Open

JWT tween is too restrictive and block some read actions #718

asaunier opened this issue Jan 31, 2018 · 5 comments

Comments

@asaunier
Copy link
Member

including the personal feed in the homepage or the user profile pages.

Seen in the logs:

2018-01-31 20:08:10,891 DEBUG [c2corg_api.tweens.jwt_database_validation][waitress] JWT VALIDATION
2018-01-31 20:08:10,897 INFO  [sqlalchemy.engine.base.Engine][waitress] ROLLBACK
2018-01-31 20:08:10,897 DEBUG [c2corg_api.tweens.rate_limiting][waitress] RATE LIMITING FOR METHOD OPTIONS
2018-01-31 20:08:10,898 DEBUG [c2corg_api][waitress] debug_authorization of url http://localhost:6545/personal-feed?pl=fr (view name '' against context <c2corg_api.RootFactory object at 0x7f685fe24b00>): Allowed (NO_PERMISSION_REQUIRED)
2018-01-31 20:08:10,907 DEBUG [c2corg_api.tweens.jwt_database_validation][waitress] JWT VALIDATION
2018-01-31 20:08:10,909 INFO  [sqlalchemy.engine.base.Engine][waitress] BEGIN (implicit)
2018-01-31 20:08:10,910 INFO  [sqlalchemy.engine.base.Engine][waitress] SELECT users.token.value AS users_token_value, users.token.expire AS users_token_expire, users.token.userid AS users_token_userid 
FROM users.token 
WHERE users.token.value = %(value_1)s AND users.token.expire > %(expire_1)s 
 LIMIT %(param_1)s
2018-01-31 20:08:10,910 INFO  [sqlalchemy.engine.base.Engine][waitress] {'param_1': 1, 'expire_1': datetime.datetime(2018, 1, 31, 19, 8, 10, 907661), 'value_1': 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE1MTg1MTUzNTEsInN1YiI6Mzc3LCJ1c2VybmFtZSI6ImFsZXgifQ.MY0Dg4MS3ySE-2YVVEwOVt55eHpIWWMc_Ohiv4rOJt4'}
2018-01-31 20:08:10,913 INFO  [sqlalchemy.engine.base.Engine][waitress] SELECT users."user".blocked AS users_user_blocked 
FROM users."user" 
WHERE users."user".id = %(id_1)s
2018-01-31 20:08:10,913 INFO  [sqlalchemy.engine.base.Engine][waitress] {'id_1': 377}
@asaunier asaunier self-assigned this Jan 31, 2018
@asaunier
Copy link
Member Author

asaunier commented Feb 1, 2018

Actually it is not related to the rate limiting: the behaviour can be observed in prod as well whereas rate limiting is not deployed yet:

  • impossible to authenticate for a blocked user
  • if already authenticated when blocked, the user can perform no request with the "authorization" header (JWT token), including read requests such as personal feed (in the home page) or viewing a user profile.

This is related to the is_valid_token check in the JWT tween: https://github.com/c2corg/v6_api/blob/master/c2corg_api/tweens/jwt_database_validation.py#L28

This function always returns a 403 error when the user is found as blocked:

if user_is_blocked:
raise HTTPForbidden('account blocked')

Perhaps we could restrict the blocking to write actions?

@asaunier asaunier changed the title Rate limiting (?) is too restrictive and block some read actions JWT tween is too restrictive and block some read actions Feb 1, 2018
@brunobesson
Copy link
Member

Perhaps we could restrict the blocking to write actions?

Seems fine to me.

@asaunier
Copy link
Member Author

asaunier commented Feb 1, 2018

This could be done by checking the method (POST, PUT, DELETE) of the request, as already done in the rate limiting tween: https://github.com/c2corg/v6_api/blob/master/c2corg_api/tweens/rate_limiting.py#L23

@cbeauchesne
Copy link
Member

Perhaps we could restrict the blocking to write actions

ok for me too

@asaunier asaunier removed their assignment Feb 2, 2018
@lbesson
Copy link
Member

lbesson commented May 3, 2021

It is a bit complicated to block only write actions:

  • suspending an account into discourse means the user cannot login
  • login / logout are POST, so probably simply blocking POST, PUT and DELETE won't be enough and more complicated logic will be required

I think logging out a blocked user could be fine and much simpler?

But then we would need https://github.com/c2corg/c2c_ui/blob/a4d340a09bd89fe1dedfe72b00311ace79737622/src/js/apis/c2c/index.js#L28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants