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

Add warning about SESSIONSTORE #264

Open
mattia-b89 opened this issue Jan 8, 2023 · 8 comments
Open

Add warning about SESSIONSTORE #264

mattia-b89 opened this issue Jan 8, 2023 · 8 comments

Comments

@mattia-b89
Copy link

Setting recommended:
browser.sessionstore.privacy_level = 2

It has the drawback of break some site login
i.e. gitlab, that's [https://gitlab.gnome.org/GNOME/gnome-control-center/] does not remember user when close FF

So, we must warn the users of this chance!

@allo-
Copy link
Owner

allo- commented Jan 8, 2023

I agree that it may be dangerous and I really wonder why it defaults to not storing any information, because this probably really breaks user expectations.

I see it as appropriate for #50 and otherwise it should probably not be changed.
It was introduced in eff9ab0 as part of a larger commit that extended a few settings like adding more settings related to dns-prefetch to the checkbox and so on and should probably never had this default.

On the other hand I do not understand why it should break logins. It may break login pages after a sessions restore (missing CSRF fields or similar things), but I do not see why it should be a problem in normal use.

And do you have a suggestion how to phrase a warning?

allo- added a commit that referenced this issue Jan 8, 2023
This may have a different default in a "paranoid" profile, see #50.
@mattia-b89
Copy link
Author

mattia-b89 commented Jan 8, 2023

I agree that it may be dangerous and I really wonder why it defaults to not storing any information, because this probably really breaks user expectations.

I see it as appropriate for #50 and otherwise it should probably not be changed. It was introduced in eff9ab0 as part of a larger commit that extended a few settings like adding more settings related to dns-prefetch to the checkbox and so on and should probably never had this default.

On the other hand I do not understand why it should break logins. It may break login pages after a sessions restore (missing CSRF fields or similar things), but I do not see why it should be a problem in normal use.

Maybe in conjunction with other odd settings.

And do you have a suggestion how to phrase a warning?

Currently, it states:

This preference controls when to store extra information about a session: contents of forms, scrollbar positions, cookies, and POST data.

We could add:

Please, remember it could break some sites, as reported here #264. More details here.

@allo-
Copy link
Owner

allo- commented Jan 8, 2023

In general I'd like to explain in easy terms what a setting does and why it can break things, so people can understand the trade-off.

Currently we have this report that it broke the login, but not details why. So the safe option for the user would be to turn it off as there is no information what may be broken.

Can you go into detail with "does not remember user"? Is the username missing from the suggestions in the login box? Does it break logged in sessions? Can you login afterward correctly?
It would also be good when you can try to reproduce with a fresh profile with only that setting. When disabling the sessions store breaks other behavior than continuing sessions this may also be a Firefox bug (that could be reported and documented here until it is fixed). A good bugreport with feedback from Mozilla developers can also make a good "read more" link in the generator.

@mattia-b89
Copy link
Author

In general I'd like to explain in easy terms what a setting does and why it can break things, so people can understand the trade-off.

Currently we have this report that it broke the login, but not details why. So the safe option for the user would be to turn it off as there is no information what may be broken.

Can you go into detail with "does not remember user"? Is the username missing from the suggestions in the login box? Does it break logged in sessions? Can you login afterward correctly? It would also be good when you can try to reproduce with a fresh profile with only that setting. When disabling the sessions store breaks other behavior than continuing sessions this may also be a Firefox bug (that could be reported and documented here until it is fixed). A good bugreport with feedback from Mozilla developers can also make a good "read more" link in the generator.

Site: [https://gitlab.gnome.org/], on a fresh Profile,

  • I login and use the site;
  • then I close FF;
  • when I reopen FF, I am no longer logged in in the site;
    username and password are still suggested in the their own boxes;
    and I can log in again without problem;

I am not an expert, but if you think it could be a FF bug, I will open an issue on their bug tracker!

@allo-
Copy link
Owner

allo- commented Jan 8, 2023

Just to be sure: Did you check "Remember me" on the login page?

@allo-
Copy link
Owner

allo- commented Jan 8, 2023

If you did not check Remember me, I suppose that the sessionstore restores your session-cookie and without session-store (or when ending the session) the session cookie is deleted. "Remember me" should probably create a cookie with expiry date in the future that is independent from the session.

@mattia-b89
Copy link
Author

Just to be sure: Did you check "Remember me" on the login page?

Yes, I flag it

@allo-
Copy link
Owner

allo- commented Jan 8, 2023

I don't think gitlab uses more than cookies to store the login (session-id). So the store seems to interfere with keeping cookies in some way. If it deletes non-session cookies that are not expired yet I would consider it to be a Firefox bug as they should be stored independent from the session store.

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

No branches or pull requests

2 participants