Skip to content

Fix session timeout conflicting with remember me#7467

Merged
leio10 merged 16 commits intodecidim:developfrom
mainio:fix/timeout_conflicting_remember_me
Jun 23, 2021
Merged

Fix session timeout conflicting with remember me#7467
leio10 merged 16 commits intodecidim:developfrom
mainio:fix/timeout_conflicting_remember_me

Conversation

@lahdeero
Copy link
Copy Markdown
Contributor

@lahdeero lahdeero commented Feb 26, 2021

🎩 What? Why?

After #7282 "remember_me" can be quite confusing, because we are singing users automatically out even if they have selected "remember me". Here we add new config "enable_remember_me" which is true by default. Also we stop automatically singing out users who have selected "remember me".

Additionally difference between Devise.timeout_in and Decidim.expire_session_after can be also confusing for system admins who are not familiar with rails. Here we combine Devise.timeout_in and Decidim.expire_session_after configs, so that Devise.timeout_in is set to same value as Decidim.expire_session_after.

📌 Related Issues

#7282

Testing

  1. Select "remember me" in sign in form
  2. Idle for expire_session_after
  3. You are not seeing inactivity warning anymore.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

♥️ Thank you!

- Enable rememberable module only if expire_session_after isn't instance
of ActiveSupport::Duration
- Session expire warning and automatic sign_out now depends on expire_session_after time.
@lahdeero lahdeero changed the title Fix timeout conflicting with remember me Fix session timeout conflicting with remember me Feb 26, 2021
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

Maybe we could use durations primarily in the configurations not to mix'n'match different kinds of configuration options?

@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Apr 6, 2021

@lahdeero could you address @andreslucena's feedback, please? 😄

@ahukkanen
Copy link
Copy Markdown
Contributor

@mrcasals We are waiting for @andreslucena input.

@lahdeero
Copy link
Copy Markdown
Contributor Author

@mrcasals or @leio10 can you review now please? @andreslucena has approved the changes

Copy link
Copy Markdown
Contributor

@leio10 leio10 left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@leio10 leio10 merged commit 498f0e0 into decidim:develop Jun 23, 2021
@ahukkanen ahukkanen deleted the fix/timeout_conflicting_remember_me branch June 23, 2021 15:02
entantoencuanto added a commit that referenced this pull request Jun 29, 2021
* develop: (47 commits)
  New Crowdin updates (#8150)
  Move the webpacker config override to @decidim/webpacker (#8158)
  Fix admin stylesheet dynamic imports (#8154)
  Fix session timeout conflicting with remember me (#7467)
  Allow to create online meetings without an URL (#8152)
  Fix verification route issues (#8146)
  Fix dont save timeout path to session (#8142)
  Fix access to import CSV results in accountability (#8132)
  Fix user report notification reported user name (#8130)
  Allow users to comment and delete their own comments (#8072)
  New Crowdin updates (#8124)
  Fix webpacker issues (#8136)
  Add comments in participatory space presentation page stats block (#8034)
  Split NPM dependencies to more granular packages (#8121)
  Metric is not shown when value is zero for blocked and reported users (#8117)
  Add missing templates translations (#8133)
  Add missing translation for authorization_modals (#8129)
  Polls in meetings (#8065)
  Fix flaky test on initiatives (#8128)
  Filter participants admin (#8104)
  ...
@andreslucena andreslucena linked an issue Jun 30, 2021 that may be closed by this pull request
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.

What's the use of "remember me"?

5 participants