Skip to content

Add function to call websocket request from JavaScript #441

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

exeldro
Copy link
Contributor

@exeldro exeldro commented Apr 22, 2024

Description

Add function to call websocket request from JavaScript
This will require page permission full access

Motivation and Context

This makes browser source pages not having to setup a websocket connection to call websocket requests.
Other alternative to #439 and #423

How Has This Been Tested?

On windows 11

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@WizardCM WizardCM added the Enhancement New feature or improvement label May 18, 2024
@exeldro exeldro force-pushed the websocket_request branch 2 times, most recently from 762c4b8 to 79216de Compare March 26, 2025 09:25
@exeldro
Copy link
Contributor Author

exeldro commented Mar 26, 2025

Rebased

@exeldro exeldro force-pushed the websocket_request branch from 79216de to ab5209e Compare May 1, 2025 06:48
@exeldro exeldro force-pushed the websocket_request branch from ab5209e to cae791a Compare May 1, 2025 06:51
@exeldro exeldro force-pushed the websocket_request branch from cae791a to 0518960 Compare May 1, 2025 06:54
@exeldro exeldro requested a review from tytan652 May 1, 2025 06:57
@exeldro
Copy link
Contributor Author

exeldro commented May 1, 2025

Used better way to include obs-websocket-api.h
Added an error result for when obs-websocket is not loaded

Copy link
Contributor

@tytan652 tytan652 left a comment

Choose a reason for hiding this comment

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

obs-websocket-api is now correctly included

@RytoEX
Copy link
Member

RytoEX commented May 1, 2025

On this, I need some explanation or justification as to why this is needed. @Warchamp7 @WizardCM ?

@steveseguin
Copy link

steveseguin commented May 1, 2025

Just my take, RytoEx:

VDO.Ninja and SocialStream.Ninja make use of the browser source API extensively, but it's rather limited relative to the websocket API.

The browser has WebRTC data-channel support, which bypasses firewalls, so users can control their OBS remotely without proxy servers or opening up their firewall.

Rather than trying to enhance the browser source API, or having users try to enable/configure web-sockets, connection states, ports, and security, just having the websocket functionality be made available via an internal API makes life easier.

The permissions are marked as ControlLevel::All:, so it's possible to offer end users who wish to enable it a clear context of what permissions are being enabled, when setting up their browser source.

I've not tested it myself yet, but having to avoid setting up websockets as an end user to obtain the same functionality via browser source integrations would increase adoption of this advanced functionality.

On this, I need some explanation or justification as to why this is needed. @Warchamp7 @WizardCM ?

@Fenrirthviti
Copy link
Member

Fenrirthviti commented May 1, 2025

I am still very wary of adding this, as it is currently my understanding this just provides a full websocket connection without any authorization/interaction from an end user, correct?

If not, what is the exact flow for this, as an end-user who is adding a browser source (or browser dock) that utilizes this? Screenshots or documentation on how this will function in practice will be helpful here.

EDIT: As an additional note, the barrier to setting up something like external websockets control is part of the security, where a user has to make a conscious effort to enable it. You remove that barrier, and this is a significant risk.

@exeldro
Copy link
Contributor Author

exeldro commented May 1, 2025

The user has to set the browser access to full in the browser source properties, so it is not enabled by default.

My use case is the markdown source plugin. I want to resize the source based on the contents displayed in the browser source. My previous attempts forgetting this working are #439 and #423, but I was advised this is a better way.

@steveseguin
Copy link

steveseguin commented May 1, 2025

As exeldro notes,

At the very least currently, a user would need to select FULL ACCESS TO OBS, as a Page Permission, when setting up their OBS Browser source. "Full Access" already gives a remote user control to start/stop/record and change scenes.

(I would not be opposed to additional confirmation windows, prompting a user to the dangers. )

image

In the case of VDO.Ninja, a user would further have to include &remote, as a URL parameter, however this is specific to VDO.Ninja. Once activated, the remote user (an IRL streamer on mobile, for example), has remote control to change scenes of their OBS Studio.

For example:

image

Users on my end want much more control over their OBS remotely; layouts, transitions, better scene state access, control over sources.

@Fenrirthviti
Copy link
Member

Thanks.

I understand the use case, but I still feel like this isn't the right approach. Right now, even the "full control" option for browser sources is very limited in what it can read or access. Exposing websockets is providing, effectively, unfettered access to almost the entire OBS internal API. We have historically rejected other additions to the JS API for being too permissive/risky, and I feel like this is the most extreme case of that.

I don't know offhand what an alternate proposal here would be. I get that you want to make it easier for users to set up remote control of OBS from a source outside their network, but again, I feel like the barrier to getting that set up is very intentional already. There is no real strong communication to the end user that setting "full control" is that dangerous with this new addition. The stated examples of start/stop stream are fairly benign, not "read and exfiltrate every single thing in OBS" which this would allow.

This likely needs input and guidance from @Warchamp7 before it can proceed, but I'm just going to note I'm against merging this in the current state, and at the very least, this will need a lot more discussion, and should probably be an RFC first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or improvement
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

7 participants