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

[WPB-6144] Prevent MLS one-to-one messaging for a blocking user (q1-2024) - no dependencies on the notification subsystem #3922

Merged
merged 7 commits into from
Mar 11, 2024

Conversation

mdimjasevic
Copy link
Contributor

The PR obsoletes PR #3900.

This is a backport of PR #3889 and PR #3906 from develop to q1-2024.

Tracked by https://wearezeta.atlassian.net/browse/WPB-6144.

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

mdimjasevic and others added 2 commits March 5, 2024 11:23
* Test: no MLS 1-to-1 when a connection is blocked

* Test: a test with expected behavior after blocking the connection

* Check if sending a msg to 1-to-1 and not connected

* Add a changelog

* WIP: Debugging a test failure

* Update the confirming test

* Revert "Check if sending a msg to 1-to-1 and not connected"

This reverts commit c4af150.

* WIP: generalise the Update.blockConv handler

* Connections: Also block MLS one2one conv when blocking conn

* Test: Parameterise over One2OneScenario

* Add the missing connection ID in an internal endpoint

* Wrap a function comment for readability

* Introduce a Galley internal endpoint: blocking a qualified conversation

* WIP: Check if an MLS 1-1 conv exists before blocking

What is left to do is to make this check work for an MLS 1-1 conv that
can be remote

* Make upsertOne2OneConv always take a Conv ID

Brig can determine this ID based on protocol of the conversation or read it from
the DB. Inventing this in galley causes more trouble for having two One2One
convs for proteus and mls.

* WIP: Remove user from 1:1 MLS conv when they block someone

* WIP: Remove mls clients on connection block

* fixup! WIP: Remove mls clients on connection block

* Make sure 1-1 conv is established before updating

* Finalise the bug-confirming test

* Remove debugging output from application code

* Fix a changelog

* Remove redundant constraints

* Properly check if an MLS 1-1 conversation exists before blocking it

* Remove more of unused code

* Remove an unused connection ID in an internal Galley endpoint for blocking a conv

---------

Co-authored-by: Akshay Mankar <akshay@wire.com>
* Don't remove MLS clients from a 1-1 conversation

* Update the changelog
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Mar 5, 2024
services/brig/src/Brig/API/Connection/Remote.hs Outdated Show resolved Hide resolved
services/brig/src/Brig/API/Connection/Remote.hs Outdated Show resolved Hide resolved
case HTTP.statusCode (HTTP.responseStatus response) of
403 -> pure False
400 -> pure False
_ {- 200 is assumed -} -> do
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we throw for other non-200 values?

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 this did the job the way it is. I'm reworking this in a PR for unblocking a user, and there I take a finer grained approach.

mdimjasevic and others added 2 commits March 8, 2024 16:11
Co-authored-by: Igor Ranieri Elland <54423+elland@users.noreply.github.com>
Co-authored-by: Igor Ranieri Elland <54423+elland@users.noreply.github.com>
@mdimjasevic mdimjasevic merged commit 3e70480 into q1-2024 Mar 11, 2024
7 checks passed
@mdimjasevic mdimjasevic deleted the wpb-6144/backport-to-q1-2024-attempt-2 branch March 11, 2024 12:51
@echoes-hq echoes-hq bot added the echoes: unplanned Any work item that isn’t part of the product or technical roadmap. label Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: unplanned Any work item that isn’t part of the product or technical roadmap. ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants