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: Firefox policies w/ extensions integration #1436

Closed
wants to merge 8 commits into from

Conversation

eyJhb
Copy link
Contributor

@eyJhb eyJhb commented Aug 11, 2020

Description

Ability to provide a list of extensions, that will be installed in a correct way (none-hacky), which is supported from the Mozilla point of view.

Currently this will not work, as it also requires some work to be done on how the Firefox addons are generated.

This also relies on the work here NixOS/nixpkgs#94898 , as a custom patch is required to make the policies on a pr. profile basis.
It can be made user wide, but that would not be that useful for some.

Maybe supporting both would be good? Any input @rycee , on what you think?

This is the config I use atm.

{ config, pkgs, lib, ... }:

let
  ffxpis = pkgs.recurseIntoAttrs (pkgs.callPackage ./modules/firefox {});
in {
    programs.firefox = {
      enable = true;
      package = pkgs.firefox-policies;

      profiles = {
        "default" = {
          id = 0;
          extensions = with ffxpis; [
            https-everywhere
            bitwarden
            i-dont-care-about-cookies
            ublock-origin
            surfingkeys_ff
          ];


          settings = {
            "toolkit.legacyUserProfileCustomizations.stylesheets" = true;
            "svg.context-properties.content.enabled" = true;
          };
          extraPolicies = {
            DontCheckDefaultBrowser = false;
            OfferToSaveLogins = false;
          };

          userChrome = ''
          '';
        };
      };
    };
}

Where I use this version, to generate my plugins. Slightly modified default.nix, with a python rewrite of the current version - https://gitlab.com/eyJhb/nix-firefox-addons

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@rycee
Copy link
Member

rycee commented Aug 12, 2020

Hmm, I don't have any particular opinion. I image it would be nice to have a per-profile setup. The extensions are now installed per-profile although the setting is global, I've been thinking about moving the option to be per-profile.

I don't understand how it could work with the XDG_RUNTIME_DIR, though. Since that directory gets deleted regularly and the HM activation only happens occasionally you might not actually start Firefox while the file is available? XDG_RUNTIME_DIR seems like an odd choice for this file in general, it doesn't seem ephemeral and non-essential.

@eyJhb
Copy link
Contributor Author

eyJhb commented Aug 12, 2020

The current way is somewhat hacky compared to this, as it is a more official way of doing it (using policies, that is).

I have however not thought about /run/user/.../ being deleted that often. The reason for placing it there, is that it is the place where Firefox will read the policies from, if the patch in the Nixpkgs is not applied (which allows for the pr. Profile settings/extensions). Not sure if there is a better way.

Ideally we would get the nixpkgs PR merged, butI can also see forcing users to use that specific Firefox with that patch is not ideal. But so is forcing any Firefox version/package on a user isn't. So I think that supporting both a normal FF version and a patched one is ideal. Or is that just me?

Is there a way to make the /run/user/.../, file always available using HM? I am guessing the only way would be a systemd unit, or patching the package like in the other PR that is open.

@eyJhb
Copy link
Contributor Author

eyJhb commented Aug 13, 2020

Thinking I will integrate it with a systemd path unit, which will ensure that the file exists :) Not sure it will be before Friday/Saturday. Any comments is appreciated !

@rycee
Copy link
Member

rycee commented Aug 13, 2020

Should be mentioned that a systemd unit would make the module incompatible with macos. But I imagine the use of XDG_RUNTIME_DIR already is causing an incompatibility 😕

I'm wondering if not this policy file is intended for something completely different than actually setting up Firefox in a general way? The way its done using XDG_RUNTIME_DIR seems to very focused on some use-case that doesn't match ours. Perhaps it's more for when you'd like to run a containerized Firefox?

@eyJhb eyJhb closed this Aug 13, 2020
@eyJhb eyJhb reopened this Aug 13, 2020
@eyJhb
Copy link
Contributor Author

eyJhb commented Aug 13, 2020

Slip of a finger regarding the closing.

It is actually called enterprise policies, so it is a way for a company to configure it in a special way. Normally it would reside in/etc/firefox/policies.json, until someone complained about it, and got it enabled for using pr. User. The directory chosen does not seem optimal no. But there should be a similar place for Mac OS X I would suppose?

@rycee
Copy link
Member

rycee commented Aug 13, 2020

Yeah, I imagine it should be available for macos as well but I don't really know anything about it :-/ If it's not available then I fear this would have to go in a separate module firefox-policies.nix, or something, which is only enabled for Linux hosts.

@eyJhb
Copy link
Contributor Author

eyJhb commented Aug 13, 2020

We might have to pull in a Mac user then, and see how that works. That could be a option yes, and would be quite nice if that is the end result.

I guess that would have to be cleared first. However, how do you feel about the nixpkgs patch?

@rycee
Copy link
Member

rycee commented Aug 13, 2020

I don't really know anything about the Firefox code to say whether the patch itself is fine or not but in general I imagine it would be quite nice to have per-profile policies 🙂

But I worry a bit about requiring the user to use a special firefox-policies package. It might be worth doing a first iteration of this feature in HM without per-profile policies and then bring in the per-profile support later?

@eyJhb
Copy link
Contributor Author

eyJhb commented Aug 13, 2020

The code itself is a thing, but it was more the general idea of having it :)

Hmm, that might be a good thing yes. Let me try and see, if I can clean this PR up a little bit. I will be making the changes in the assumption, that it will work on Mac too, and that it can replace the current way extensions work. If that is not the case, it should be easy to put it back in again :)

Need to figure out how systemd wants to cooperate with a path unit, but I am not sure that can be a solution at all, sadly... I think the best choice it a timer script, that runs ever X secs to check if the file exists, if not symlink it :)

@rycee
Copy link
Member

rycee commented Aug 13, 2020

Since the runtime dir is created when you log in I imagine it should be sufficient with a oneshot service wanted by default.target.

@eyJhb
Copy link
Contributor Author

eyJhb commented Aug 18, 2020

Update - Not forgotten, but have had (and still have) a lot of preparations for family gathering.
Hoping to work on this soon again!

@eyJhb
Copy link
Contributor Author

eyJhb commented Sep 12, 2020

@rycee I have made it into a oneshot service, but it does not seem to run the service on nixos-rebuild switch, or am I doing something wrong?

@eyJhb
Copy link
Contributor Author

eyJhb commented Sep 19, 2020

Seems to work with systemd.user.startServices = true, as the bash implementation did not work well. This should either be set for the user, or indicate it in a option.

Besides this, I am still debating if it should have the option to include the patch for Firefox, so that we can have a pr. profile policies.json. I really like the idea of it! Because you could have one for uni, hacking (accept all certs), work, personal each with its own extensions, bookmarks, etc.

Any comment is appreciated @rycee !

@eyJhb
Copy link
Contributor Author

eyJhb commented Sep 28, 2020

@rycee it should be working now, is there anything that should be changed? And can this even be merged? :)

EDIT: Considering removing the pr. profile policies, as it is nice to have, but maybe not many will use it, etc...

@eyJhb
Copy link
Contributor Author

eyJhb commented Dec 3, 2020

Should maybe be reworked to work with NixOS/nixpkgs#91724 instead :)

@stale
Copy link

stale bot commented Apr 28, 2021

Thank you for your contribution! I marked this pull request as stale due to inactivity. If this remains inactive for another 7 days, I will close this PR. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously, when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the issue

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Apr 28, 2021
@stale stale bot closed this May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants