Make the remember feature optional

[alxy]

It can be discussed, if the default value should be false.

[LukeTowers]

I'm not opposed to this change, @daftspunk do you have any input on the matter? @alxy wants to be able to not always tell the authentication manager to remember the credentials, currently it's hard coded to do so.

Also up for discussion is whether the default behaviour should be to not remember, and require a value passed in order to remember. @alxy That would be trickier to handle as we would be changing the existing behaviour for current users of the plugin.

[LukeTowers]

@alxy do you still want this change?

[ChVuagniaux]

I currently working on an implementation of the same kind that this was done for the backend (octobercms/october#2924)

[LukeTowers]

@ChVuagniaux if @alxy doesn't want to work on this anymore could you provide a PR along the same lines of what you provided for the backend?

[alxy]

This is a simple PR that already implements the required functionality.

[LukeTowers]

@alxy The backend implementation provides three modes, is that supported in this one as well?

[alxy]

No, but I don't think this is needed in this plugin. In case the remember option is false, the session will be destroyed once the browser is closed, if it is true the session will be kept alive. For anything more advanced I would require the plugin user tp roll out a custom component containing that logic.

[LukeTowers]

I think it would be reasonable to support looking at the value of session.lifetime (the third mode) to determine the length of the session. Thoughts? (octobercms/october#2924 (comment))

[KGE]

@LukeTowers why is this PR still not merged ? First of all, I think the default behavior should not be "Remeber forever".. it is not safe.

But at least this PR make it possible to post a remember value ( and as I can see it is defaulting to true so will not impact older versions ). And posting "false" will keep the user logged in until the session expire ( like "the third mode" ), the remember option ( to false ) will just avoid writing the cookie, it will not impact the session.

[LukeTowers]

@KGE I would like it to mirror the behaviour of the backend modes, as detailed in the above comment with the three options:

1. authentication doesn't expire
2. authentication expires based on session lifetime
3. user logging in chooses

If @alxy doesn't want to implement it, then resubmit your own PR doing so and I'll look at merging it.

[KGE]

I agree, mirroring the backend modes is the way to do it. I will look into it.

[LukeTowers]

@KGE glad to hear it, looking forwards to seeing what you come up with!

[bennothommo]

Closing this in favour of the implementation within #312.