vircadia / vircadia-native-core

Vircadia open source agent-based metaverse ecosystem.
https://vircadia.com/
Other
533 stars 176 forks source link

Teleport permissions are fully ignored in v2021.1.0-rc2 #1021

Open JulianGro opened 3 years ago

JulianGro commented 3 years ago

So apparently the teleport permissions are worked around in https://github.com/vircadia/vircadia/pull/991 I realize that the "Everyone" setting wasn't working, but I feel like circumventing the permission system entirely is the wrong "fix" here.

digisomni commented 3 years ago

Thing is though, apps like the Radar app let you see and hop all over the place anyway. There's nothing preventing someone from teleporting to you in a domain if they see you on the PAL (and they always will). They can use a script, the scripting console, or an app.

This is just meant to make it easier on new and veteran users alike to quickly jump around to people.

We however do need to fix security and privacy in general. The concept of ACL Zones is probably one such thing that can assist with that.

JulianGro commented 3 years ago

Maybe the radar app should abide to permissions as well.. I get that the system isn't flawless, but that is no reason to just nuke it. Especially if you aren't even nuking it entirely. You still give the user the option to not allow people to teleport to them..

digisomni commented 3 years ago

The system is not being nuked though, those permissions were and still are primarily for the "Connections" tab to prevent or allow people from seeing you online and finding out which server you are on.

The Radar app is a 3rd party app, while the creator could be asked to change it, it helps outline the idea that some people might not agree and refuse to change that. It's an example of how not adding the feature to the People app doesn't exactly change much, just makes things more inconvenient for people.

If the concern is privacy/security, then we should try to work on that in a way that's functional and practical instead of security theater.

JulianGro commented 3 years ago

It is nuking the system. It doesn't matter what other functions that settings has. Fact is that it is supposed to not let people teleport to you, and it actually did that in the People app until that functionality was circumvented. The functionality should either be restored, or it should at the very least not give you the option to not allow people to teleport to you if it doesn't do anything. Maybe I don't want you to jump to me and I choose that setting because of that. Of course there is ways to get around that, but you are literally taking the option away from me while also telling me that I have it.

digisomni commented 3 years ago

Okay, maybe this is along the lines of what you're thinking? Functionality will have to be added to the app in order to make it function as a status indicator. Again, the actual functionality implemented has always been for cross-server and account-wide status. In terms of making it useful as a "Available", "Busy", "Do Not Disturb" indicator for people to choose to respect well..... that has to be implemented.

All that has been done here is fixing a bug that has causes a decent bit of trouble for people, especially new people that are being onboarded.

I think a new issue should be filed asking for the feature in the 'People' app: status indicators, and then teleport can be enabled or disabled to a person as a courtesy based on that. That would be a convenience feature though, not a security or privacy feature as it does nothing in the way of that... If there are security or privacy concerns, then we should be thinking about solutions to that end.

JulianGro commented 3 years ago

A bug in one option was fixed by breaking the other three options entirely..

digisomni commented 3 years ago

This should...... not break the functionality in the "Connections" tab. This should only apply to the "Nearby" tab so it didn't break that. enabled: selected && pal.activeTab == "nearbyTab" && isPresent;

JulianGro commented 3 years ago

I guess I wasn't being 100% clear, but you should know what I mean.

digisomni commented 3 years ago

I can look into breaking it out and implementing a status feature but it doesn't look like there are any easy ways without passing it to the metaverse. In essence, there maybe should be "Status" (what) and then an actual "Visibility" (who) instead of the catch-all "Availability".

JulianGro commented 3 years ago

Well a status feature might be an interesting idea, but the issue here is that the teleport permissions are just ignored now.

digisomni commented 3 years ago

After looking at it further, I don't see the direct way to fix it back to its old correct functionality (where 'Everyone' did indeed work...)

Will have to see about setting this up correctly with a non-hacky functionality at some point.

Aitolda commented 3 years ago

I don't think the "everyone, friends only" option ever affected teleporting to someone when you were in the same domain, it was teleporting to someone in another domain. As far as I can recall, teleport within the domain was always just set as something admins could do but no one else could, but what Julian is suggesting could be a nice feature to eventually add.

digisomni commented 3 years ago

I remember in High Fidelity I could indeed teleport to people in the same domain when I was not an admin, we had to have it set to "Everyone" though.

Aitolda commented 3 years ago

I remember in High Fidelity I could indeed teleport to people in the same domain when I was not an admin, we had to have it set to "Everyone" though.

Hmm. I don't remember that for certain, but it's been a VERY long year so perhaps you're right.

JulianGro commented 3 years ago

It specifically says "jump to your location" after already telling you that they can see which domain you are in. To me "location" suggest coordinates rather than just a domain.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

daleglass commented 3 years ago

Hmm, so what do we do about this one? It seems like either we can try to restore the old behavior or keep the new one. Do we have a firm consensus over what is the better choice?

JulianGro commented 3 years ago

We never reached consensus. I was against removing the original behaviour and Kalila insisted in removing it to fix a bug with it. I think the text was also never changed to fit the new behaviour more but I might be wrong about that.

stale[bot] commented 2 years ago

Hello! Is this still an issue?

stale[bot] commented 2 years ago

Hello! Is this still an issue?