-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix: Don't use affiliation to decide whether to observe the role #1501
Conversation
Hi, thanks for your contribution! |
d6f90a1
to
35ba341
Compare
CLA is signed |
Why is this not the case? We haven't seen any problems with that? |
Thanks so much for the quick review, and sorry for not getting back to you til now. In most cases, role and affiliation will match tidily. It's only in more esoteric setups, where the MUC is being externally controlled AND Jitsi isn't permitted to upgrade everyone to owner where it wouldn't. Are you as concerned about the original purpose of this removed line as I am? I don't know why that test is there, but it feels like more than an abundance of caution. The compromise I suggested (expanding the if statement to include the "admin" affiliation) would both work in my setup, and be compliant with the default roles in XEP-0045. Let me know if you'd like me to update this PR. |
I think we won't run into this ourselves because we always use owner, but LGTM in general. |
Thinking about this some more - this needs companion PRs in some places. Whilst this is technically correct, I'm not sure you'd want to take it without the other MRs being in place too. I'm going to revert this to draft until I find time to get the companion pieces of work done. |
@Fishbowler Any updates on this? |
To be honest, the amount of work required to get all of the Jitsi components to behave correctly in this edge case is more than I'm willing to take on. I've worked around it for now using an XMPP shim, telling lies about roles and affiliations to both Jitsi and the XMPP server. |
ISSUE: In a constrained environment, where Jitsi is backed by an XMPP server with MUCs already defined, users with an affiliation of "admin" and a role of "moderator" appear as a moderator to everyone but themselves.
Whilst XEP-0045 defines the owner affiliation to have a default role of moderator, this isn't always the case.
That being said, this line was in for a reason, so happy to talk about how to meet that need. Perhaps look at constraining it to
const newRole = (member.affiliation === 'owner' | member.affiliation === 'admin') ? member.role : 'none';
?