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

Feat: Add custom/global Sandboxes #1879

Merged
merged 9 commits into from
Oct 12, 2024
Merged

Conversation

SpecialAro
Copy link
Member

Pre-flight Checklist

Please ensure you've completed all of the following.

Description of Change

Enables custom sandboxes or global sandbox

Motivation and Context

This feature fixes #1084 and enables the grouping services in the same custom sandbox (by default, services still have their own sandbox). It was also introduced a feature to disable sandboxes completely, which puts all services in the same "global" sandbox.

⚠️ KNOWN ISSUES:
Services in the same sandbox share the same webviewContents - that is why I introduced a workaround in the Service.ts. For this reason, some issues may arise, e.g. zooming in/out/reset in a service within a sandbox affects other services in the same sandbox.

Screenshots

image

In the screenshot, services on the right are services on its own sandbox (without a custom sandbox), and services on the left are services on a custom sandbox (named "GOOGLE" in this example). Should we add a title to each column so it is clearer?

Checklist

  • My pull request is properly named
  • The changes respect the code style of the project (pnpm prepare-code)
  • pnpm test passes
  • I tested/previewed my changes locally

Release Notes

@SpecialAro SpecialAro requested a review from a team August 19, 2024 18:15
@SpecialAro SpecialAro self-assigned this Aug 19, 2024
@vraravam
Copy link
Contributor

@SpecialAro - how is this "stored" for folks using the ferdium server?

@SpecialAro
Copy link
Member Author

I haven't updated the logic for using Ferdium server, the file is only stored locally. Users can replicate or have different sandboxes in different machines. But we can also implement it if you think it is worth it!

@vraravam
Copy link
Contributor

But we can also implement it if you think it is worth it!

im actually against this feature itself - since it violates the security aspects of sandboxing. but, since you have implemented it, i would assume that you also have found a need for it.

@SpecialAro
Copy link
Member Author

I understand you and I'm really up to make all the warnings possible that turning this feature on is a security risk - as we loose the sandbox itself.

The only use I found for it (and I think it is what the users in the issue were requesting as well) is not having to login independently on each service (for instance, I can login once in GMAIL and it will allow me to use that same account without having to login in other google services inside the custom sandbox. I can also add services that use Google auth to login more easily without the need of inputing the password) - as a regular web browser does.

Nevertheless, I totally agree this is somewhat of a security risk... So I won't be sad if this isn't merged at the end or if we should work on it a bit more 😁

I just think that users should be the ones to know if they want to risk it or not - for instance, that's the same with the webRTC settings for exposing IPs - at the end, the default option should always be the safest one, but users may be able to change it if they need it.

Let me know what you think!

@maddin79
Copy link

maddin79 commented Aug 24, 2024

Hey, thank you for this. I really need this feature. I have 2FA auth and SSO in my company and have several applications running in Ferdium. I have to login several times a day and that is really annoying.
I must admit that I do not really see the benefit of sandboxes. Yes, applications can not talk to each other and that is more "secure". But nowadays, especially in a company environments, everything is build up that applications know from each other or share a context, especially when using SSO.
I also think that developers should not take control over the decisions of users. The sandbox can be the standard, but if the user decides to turn it off, then this is fine (of course with warnings). That is actually the reason why I hate Windows and love Linux ...

So, please merge it. I'm really looking forward to check this out. @SpecialAro Thanks again for your work and thanks to all the Ferdium people... great tool ... love it.

Best
Martin

@vraravam
Copy link
Contributor

@maddin79 - the principle of being secure has been there for quite a long time - even pre-Ferdium.
There are other followup issues that would come with each implementation/feature. I dont disagree that this can be useful for some swathe of users, but the default should be what the pre-existing users are expecting.

Also, as called out multiple times, unless the code-contributor-base increases, any support for new functionality will need effort which might not happen. (Remember that this is a volunteer-run project.) So, with each new added functionality, the breadth of such coverage will also need involvement. Are you or others signing up for this? (I am definitely not willing to do so due to other commitments).

@maddin79
Copy link

@vraravam As I said, the current state of Ferdium should be the standard setting. But a user should be free to choose if sandboxes are enabled or not.

You referring to security, but what security do you mean? Are browsers without sandboxes not secure to use and nobody should use them?
Security comes from knowing what I'm doing and being careful of what I'm sharing, clicking or opening in the web. If users are forced to certain things they don't want, they go around it and mostly with insecure solutions.

I totally see your point of the lack of contributors and I'm sorry that I can not provide more help. But is it really a new feature to turn of sandboxes? And @SpecialAro did the work for this.

@vraravam
Copy link
Contributor

But is it really a new feature to turn of sandboxes? And @SpecialAro did the work for this.

im equally concerned with the support for this feature and who will do that support. If @SpecialAro is signing up for this, that concern can be mitigated.

@blacksd
Copy link

blacksd commented Oct 11, 2024

Just subscribed to this thread. I'd really love to see this feat make its way to the stable version. A three-step authentication for every service in an Enterprise is extremely time-consuming, and leads to insecure user workarounds.
Controlling the sandbox is easier and more effective; Android for Work and the similar profile-based Apple implementation took this path before; it's more secure to control (even nuke) a single sandbox than multiple logins.

@vraravam
Copy link
Contributor

@SpecialAro - since the current mechanism/meaning of sandbox in ferdium is that each individual service is already in its own sandbox, can we rename all wording here to depict the shared nature of the sandboxes? If so, then we can go ahead with merging this PR (after making those wording changes).

Since this is a significant departure from the current state of ferdium, I also propose that we do a major version bump - wdyt?

Copy link
Contributor

@vraravam vraravam left a comment

Choose a reason for hiding this comment

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

I reviewed once more, and I think the PR can be merged.

@vraravam vraravam merged commit 035c581 into ferdium:develop Oct 12, 2024
5 checks passed
@SpecialAro SpecialAro deleted the sandboxes branch October 12, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to share session data between selected services
4 participants