Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Block ability to read via sync if mau limit exceeded #3670

Merged
merged 10 commits into from Aug 14, 2018
Merged

Block ability to read via sync if mau limit exceeded #3670

merged 10 commits into from Aug 14, 2018

Conversation

neilisfragile
Copy link
Contributor

@neilisfragile neilisfragile commented Aug 9, 2018 •
edited

Will just log out instead when matrix-org/matrix-spec-proposals#1467 exists

@neilisfragile neilisfragile requested review from erikjohnston and removed request for erikjohnston August 9, 2018 12:26
@neilisfragile neilisfragile changed the title Block ability to read via sync if mau limit exceeded WIP Block ability to read via sync if mau limit exceeded Aug 9, 2018
@neilisfragile
Copy link
Contributor Author

@matrixbot: test this please

@neilisfragile neilisfragile changed the title WIP Block ability to read via sync if mau limit exceeded Block ability to read via sync if mau limit exceeded Aug 10, 2018
@neilisfragile neilisfragile mentioned this pull request Aug 14, 2018
Merged
erikjohnston
erikjohnston reviewed Aug 14, 2018
View reviewed changes
synapse/api/auth.py Outdated
"""Checks if the user should be rejected for some external reason,
such as monthly active user limiting or global disable flag

Args:
user_id(str): If present, checks for presence against existing MAU cohort
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be clearer to say something like:

user_id(str|None): The user ID of the requester, or None if registering.

So that the docstring doesn't require readers to understand what the MAU cohort is to know roughly whats going on

synapse/api/auth.py
@@ -775,15 +775,24 @@ def check_in_room_or_world_readable(self, room_id, user_id):
)

@defer.inlineCallbacks
def check_auth_blocking(self):
def check_auth_blocking(self, user_id=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should call this check_server_limits or something

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I agree, since it will be a change in a lot of places I'll submit a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err given I have a few outstanding, I'll wait until a few more of them land

@erikjohnston
Copy link
Member

Looks good, other than some suggestions about the docs to make it easier to understand.

I am wondering whether we actually are doing the checks in the right place, as thinking about it I'd expect the various checks to be next to each other. I know I suggested putting the check here, but do you remember why we decided to not put the register/login checks in synapse/rest rather than handlers?

@neilisfragile
Copy link
Contributor Author

I think the reason was out of simplicity of not needing to insert at multiple entry points (including any future ones) The idea being that all checks must pass through the handler so would catch everything

@neilisfragile neilisfragile merged commit 414d54b into develop Aug 14, 2018
@neilisfragile neilisfragile deleted the neilj/mau_sync_block branch August 14, 2018 15:21
richvdh added a commit that referenced this pull request Aug 22, 2018
@richvdh
Merge tag 'v0.33.3'
48fec67 
Features
--------

- Add support for the SNI extension to federation TLS connections. Thanks to @vojeroen! ([\#3439](#3439))
- Add /_media/r0/config ([\#3184](#3184))
- speed up /members API and add `at` and `membership` params as per MSC1227 ([\#3568](#3568))
- implement `summary` block in /sync response as per MSC688 ([\#3574](#3574))
- Add lazy-loading support to /messages as per MSC1227 ([\#3589](#3589))
- Add ability to limit number of monthly active users on the server ([\#3633](#3633))
- Support more federation endpoints on workers ([\#3653](#3653))
- Basic support for room versioning ([\#3654](#3654))
- Ability to disable client/server Synapse via conf toggle ([\#3655](#3655))
- Ability to whitelist specific threepids against monthly active user limiting ([\#3662](#3662))
- Add some metrics for the appservice and federation event sending loops ([\#3664](#3664))
- Where server is disabled, block ability for locked out users to read new messages ([\#3670](#3670))
- set admin uri via config, to be used in error messages where the user should contact the administrator ([\#3687](#3687))
- Synapse's presence functionality can now be disabled with the "use_presence" configuration option. ([\#3694](#3694))
- For resource limit blocked users, prevent writing into rooms ([\#3708](#3708))

Bugfixes
--------

- Fix occasional glitches in the synapse_event_persisted_position metric ([\#3658](#3658))
- Fix bug on deleting 3pid when using identity servers that don't support unbind API ([\#3661](#3661))
- Make the tests pass on Twisted < 18.7.0 ([\#3676](#3676))
- Don’t ship recaptcha_ajax.js, use it directly from Google ([\#3677](#3677))
- Fixes test_reap_monthly_active_users so it passes under postgres ([\#3681](#3681))
- Fix mau blocking calulation bug on login ([\#3689](#3689))
- Fix missing yield in synapse.storage.monthly_active_users.initialise_reserved_users ([\#3692](#3692))
- Improve HTTP request logging to include all requests ([\#3700](#3700))
- Avoid timing out requests while we are streaming back the response ([\#3701](#3701))
- Support more federation endpoints on workers ([\#3705](#3705), [\#3713](#3713))
- Fix "Starting db txn 'get_all_updated_receipts' from sentinel context" warning ([\#3710](#3710))
- Fix bug where `state_cache` cache factor ignored environment variables ([\#3719](#3719))
- Fix bug in v0.33.3rc1 which caused infinite loops and OOMs ([\#3723](#3723))
- Fix bug introduced in v0.33.3rc1 which made the ToS give a 500 error ([\#3732](#3732))

Deprecations and Removals
-------------------------

- The Shared-Secret registration method of the legacy v1/register REST endpoint has been removed. For a replacement, please see [the admin/register API documentation](https://github.com/matrix-org/synapse/blob/master/docs/admin_api/register_api.rst). ([\#3703](#3703))

Internal Changes
----------------

- The test suite now can run under PostgreSQL. ([\#3423](#3423))
- Refactor HTTP replication endpoints to reduce code duplication ([\#3632](#3632))
- Tests now correctly execute on Python 3. ([\#3647](#3647))
- Sytests can now be run inside a Docker container. ([\#3660](#3660))
- Port over enough to Python 3 to allow the sytests to start. ([\#3668](#3668))
- Update docker base image from alpine 3.7 to 3.8. ([\#3669](#3669))
- Rename synapse.util.async to synapse.util.async_helpers to mitigate async becoming a keyword on Python 3.7. ([\#3678](#3678))
- Synapse's tests are now formatted with the black autoformatter. ([\#3679](#3679))
- Implemented a new testing base class to reduce test boilerplate. ([\#3684](#3684))
- Rename MAU prometheus metrics ([\#3690](#3690))
- add new error type ResourceLimit ([\#3707](#3707))
- Logcontexts for replication command handlers ([\#3709](#3709))
- Update admin register API documentation to reference a real user ID. ([\#3712](#3712))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@erikjohnston erikjohnston erikjohnston left review comments

Assignees

@erikjohnston erikjohnston

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@neilisfragile @erikjohnston