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

WIP: MSC1772: Matrix spaces #1772

Draft
wants to merge 75 commits into
base: master
from
Draft

WIP: MSC1772: Matrix spaces #1772

wants to merge 75 commits into from

Conversation

@ara4n
Copy link
Member

@ara4n ara4n commented Jan 3, 2019

Rendered

N.B. group access control has been split out into #2962

Obsoletes #1215

@ara4n ara4n added the proposal label Jan 3, 2019
@ara4n ara4n mentioned this pull request Jan 3, 2019
ara4n added 2 commits Jan 3, 2019
md
@turt2live turt2live added the T-Core label Jan 3, 2019
ara4n added 3 commits Jan 3, 2019
md
Copy link
Member

@jcgruenhage jcgruenhage left a comment

looks good

proposals/1772-groups-as-rooms.md Outdated Show resolved Hide resolved
proposals/1772-groups-as-rooms.md Outdated Show resolved Hide resolved
proposals/1772-groups-as-rooms.md Outdated Show resolved Hide resolved
proposals/1772-groups-as-rooms.md Outdated Show resolved Hide resolved
ara4n added 5 commits Jan 6, 2019
proposals/1772-groups-as-rooms.md Outdated Show resolved Hide resolved
proposals/1772-groups-as-rooms.md Outdated Show resolved Hide resolved
proposals/1772-groups-as-rooms.md Outdated Show resolved Hide resolved
proposals/1772-groups-as-rooms.md Outdated Show resolved Hide resolved
proposals/1772-groups-as-rooms.md Outdated Show resolved Hide resolved
This was referenced Feb 28, 2019
@auscompgeek

This comment has been hidden.

@Sorunome

This comment has been hidden.

@KamNull

This comment has been hidden.

@BloodyIron

This comment has been hidden.

Copy link

@erkinalp erkinalp left a comment

Trust issue wrt flairs

proposals/1772-groups-as-rooms.md Show resolved Hide resolved
proposals/1772-groups-as-rooms.md Outdated Show resolved Hide resolved
richvdh added 11 commits Mar 16, 2021
These should have been part of MSC2962.
if/when we need it, we should follow MSC2875
I think the peeking thing is specific to access control.
we're not doing this bit yet.
richvdh added 2 commits Mar 17, 2021
XXX: we need to deterministically specify where the cycles get cut.
I think kegan found a solution for this when implementing MSC2946 in Dendrite.
Comment on lines +184 to +185

This comment has been minimized.

@richvdh

richvdh Mar 18, 2021
Member

the original thread got hijacked, so once again:

@ara4n why do you think this needs to be deterministic, and why does it need to be specified here?

This comment has been minimized.

@t3chguy

t3chguy Mar 18, 2021
Member

If it isn't deterministic & specced then different clients will cut differently and thus show different "top level" spaces

E.g in an A -> B -> A space hierarchy, client A might show A -> B, the other B -> A and this would on the surface look very broken

This comment has been minimized.

@richvdh

richvdh Mar 18, 2021
Member

E.g in an A -> B -> A space hierarchy, client A might show A -> B, the other B -> A and this would on the surface look very broken

right, yes. I don't think that the solution in MSC2946 (which is simply to stop walking once you find a repeat) is going to help here, because it all depends where you start from.

This comment has been minimized.

@richvdh

richvdh Mar 18, 2021
Member

The alternative seems to be to specify a way that clients can agree which is the rootmost space in a loop. For example, you might say "the lexicographical lowest room id in the loop is the root". But then you still encounter effects which look very broken when a space that was the root suddenly becomes several levels down the hierarchy because someone introduced a cycle.

This comment has been minimized.

@t3chguy

t3chguy Mar 18, 2021
Member

the lexicographical lowest room id in the loop is the root

Yup, that is what was proposed internally at one point (and is what I implemented in EleWeb iirc)

This comment has been minimized.

@clokep

clokep Apr 6, 2021
Member

Does this make sense to include in here then or does it make sense to leave this up to implementations?

This comment has been minimized.

@richvdh

richvdh Apr 7, 2021
Member

I think it probably does make sense to make a recommendation here, yes.

richvdh added 2 commits Mar 22, 2021
@gfodor

This comment has been hidden.

```js
{
"type": "m.space.child",
"state_key": "!abcd:example.com",

This comment has been minimized.

@Cadair

Cadair Mar 31, 2021
Contributor

It's worth noting here that once a room id is added to a space in the state key it can't ever be removed again. While this is a very minimal metadata leak (only really showing the existence of a room with that id) is it worth considering some method of obfuscating the ids in the state key? So that the actual room id could be redacted with the content?

This comment has been minimized.

@richvdh

richvdh Apr 7, 2021
Member

yeah, one solution here would be to make the state_key a hash of the room id, and embed the unhashed room id in the body of the event - so that the event could be redacted leaving only the hashed id behind.

For me, that feels a bit overengineered given the scope of the information leak (room IDs aren't really that sensitive). It's maybe something we could come back to in future.

This comment has been minimized.

@ShadowJonathan

ShadowJonathan Apr 7, 2021
Contributor

Maybe a future MSC could define a "paranoid" setting which'd enable this, either by default or optionally, to indeed make statekeys hashed?

@turt2live turt2live marked this pull request as draft Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment