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

Don't push if an user account has expired #58

Merged
merged 7 commits into from
Sep 18, 2020
Merged

Conversation

MatMaul
Copy link
Contributor

@MatMaul MatMaul commented Aug 28, 2020

No description provided.

@MatMaul MatMaul force-pushed the mv/no-push-validity branch from 91bfee4 to d05a48a Compare August 28, 2020 15:07
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

I'd prefer if you could factor out the code into a single function that's called in both places, but otherwise lgtm.

@MatMaul MatMaul force-pushed the mv/no-push-validity branch 3 times, most recently from 184f9cd to 88bef9c Compare September 7, 2020 15:24
@MatMaul MatMaul force-pushed the mv/no-push-validity branch from 88bef9c to 5263d42 Compare September 7, 2020 15:28
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Just a small point about cleanup, otherwise the logic looks sound :)



@defer.inlineCallbacks
def is_account_expired(user_id: str, store, current_ts: int):
Copy link
Member

Choose a reason for hiding this comment

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

We generally try not to pass the store around between functions, but instead always reference it from the homeserver object.

However, I think it would make more sense if this method were moved next to get_expiration_ts_for_user in synapse/storage/data_stores/main/registration.py. Then you could simply do yield store.is_account_expired(...), rather than having to import is_account_expired everywhere you want to use it.

Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

LGTM assuming CI passes!

@anoadragon453 anoadragon453 merged commit c3bca37 into dinsic Sep 18, 2020
@anoadragon453 anoadragon453 deleted the mv/no-push-validity branch September 18, 2020 15:50
anoadragon453 added a commit that referenced this pull request Oct 9, 2020
…oa/freeze_rooms

* 'dinsic' of github.com:matrix-org/synapse-dinsic:
  Only assert valid next_link params when provided (#65)
  Don't push if an user account has expired (#58)
  Swap method calls in RoomAccessTestCase.test_change_rules (#64)
  Make all rooms noisy by default (#60)
  Make AccessRules use the public rooms directory instead of checking a room's join rules on rule change (#63)
  Override the power levels defaults, enforce mod requirement for invites, admin requirements for unknown state events (#61)
  RoomAccessRules cleanup (#62)
  Add a config option for validating 'next_link' parameters against a domain whitelist (#8275)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants