allo- / ffprofile

A tool to create firefox profiles with personalized defaults.
GNU Affero General Public License v3.0
784 stars 56 forks source link

[Bug] Engerprise Policy generation borked... #220

Closed Hammerfest closed 1 year ago

Hammerfest commented 3 years ago

Went through profile maker, chose none of the addons, declared not to have menu on all the time, etc etc.

Applied enterprise_policy folder to install directory. On next start all of the addons where installed, menu on all the time, etc etc, didnt bother to check everything, didnt need to.

Went through tool again and did fresh profile, same thing occurred.

allo- commented 3 years ago

I can reproduce the bug.

allo- commented 3 years ago

With no addon I cannot reproduce, but httpseverywhere only leads to httpseverywhere + canvasblocker.

allo- commented 3 years ago

"Disable System addon update" policy adds all addons.

I think it is some problem in the merging of the form data.

Somewhere here it should only add the policy when the option is checked. This works for updating the addons list and does not work for updating the enterprise policy:

https://github.com/allo-/firefox-profilemaker/blob/79ddef3918bb56b25d49d69973de9a0564988f53/forms.py#L56

Hammerfest commented 3 years ago

"Disable System addon update" policy adds all addons.

I think it is some problem in the merging of the form data.

Somewhere here it should only add the policy when the option is checked. This works for updating the addons list and does not work for updating the enterprise policy:

https://github.com/allo-/firefox-profilemaker/blob/79ddef3918bb56b25d49d69973de9a0564988f53/forms.py#L56

I did not use that policy, it was left off (its default state) unless your saying its code is the problem be it on or off

allo- commented 3 years ago

I did not use that policy

I just commented on my debugging.

The problem affects more enterprise policies and is probably caused by merging the policies of different forms. When you did not answer the questions on the tab at all, you don't get the addons. If I answer with only httpseverywhere, I get two addons. When changing an option with enterprise policy in another tab, I get many addons.

It must be something how the policies are merged together / how it is checked if the option is active. The linked code looks correctly, the enterprise_policy section is only touched when cleaned_data[option["name"]] is truthy, just like the addons list. Still they show different behavior.

allo- commented 3 years ago

httpseverywhere only leads to httpseverywhere + canvasblocker.

Looked like there could be an mistake by two settings files using the same option name, but it is not. And they are selected correctly for the addons list, but not when generating the enterprise policy.

Hammerfest commented 3 years ago

10-4, first dev I have seen to debug in a ticket, i like it :D TYVM for all your work

allo- commented 3 years ago

Noting down right/wrong behavior is helpful when debugging later. And maybe someone else has an idea. I don't see it now, but it should be fixed soon as it is a rather big bug, which only didn't cause more harm because only few people are using the enterprise policy export.

allo- commented 3 years ago

When saving the form with different combinations it sometimes gets the wrong result and sometimes not.

The linked code path should always reset the policy before merging the active options and I even get extensions in the list, which were never enabled in the session. If the parsing would be wrong, they would be in the addons list as well, but they are not.

Please tell me, if you noticed a pattern. It looks to me like the extensions in the list before the last enabled extension are enabled as well, but not always.

As test cases: https-everywhere (only) enables cookie autodelete. uMatrix only seems not to enable everything before.

allo- commented 2 years ago

@Hammerfest I believe this may be fixed by the latest commit. Can you test again?

Hammerfest commented 2 years ago

@Hammerfest I believe this may be fixed by the latest commit. Can you test again?

Will give it a go when I can this weekend