Skip to content

Fix infinite loop when impersonated session time runs out#7221

Merged
oriolgual merged 14 commits intodecidim:developfrom
mainio:fix/impersonate_infinite_loop2
Mar 5, 2021
Merged

Fix infinite loop when impersonated session time runs out#7221
oriolgual merged 14 commits intodecidim:developfrom
mainio:fix/impersonate_infinite_loop2

Conversation

@lahdeero
Copy link
Copy Markdown
Contributor

@lahdeero lahdeero commented Jan 25, 2021

🎩 What? Why?

There is chance that impersonation.js inits reloads endlessly, this happens when impersonated sessions time runs out at the same time as a user is loading or reloading a page. We also have to prevent a redirect on ajax request, because otherwise reload in javacript cancels the redirect back to admin panel.

Testing (UPDATED)

  1. Go to admin panel -> Participants -> Impersonations -> Impersonate
  2. Fill reason, pick "Example authorization" and insert a unique document number which ends with the letter X (eg. 12345X)
  3. Click "Impersonate" and start timer (at the same time)
  4. Go to assemblies index page for example, wait until timer is on 29 minutes and 59 seconds and hit reload or processes link for example. (You are in the infinite loop without this fix).

📋 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!

@mrcasals
Copy link
Copy Markdown
Contributor

@decidim/product do you want to try to replicate this bug?

@mrcasals
Copy link
Copy Markdown
Contributor

There's a failing job that's already fixed on develop, I guess the branch was created prior to the fix 😄

@mrcasals
Copy link
Copy Markdown
Contributor

In order to try to replicate the bug, I created a review app here: https://decidim-staging-pr-277.herokuapp.com/

@lahdeero
Copy link
Copy Markdown
Contributor Author

Review app throws:

There was a problem with our server
Please try again later.

When clicking impersonate. @mrcasals

Copy link
Copy Markdown

@qlty-cloud-legacy qlty-cloud-legacy bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 9472 lines exceeds the maximum allowed for the inline comments feature.

@lahdeero lahdeero force-pushed the fix/impersonate_infinite_loop2 branch from 0d25e10 to 710474f Compare February 8, 2021 09:07
@lahdeero
Copy link
Copy Markdown
Contributor Author

lahdeero commented Mar 4, 2021

I think this is purely technical fix, dont you agree? @mrcasals. If you want to replicate this, you should change SESSION_TIME_IN_MINUTES to something like 1 minute, in decidim-core/app/models/decidim/impersonation_log.rb.

@oriolgual oriolgual merged commit f22e006 into decidim:develop Mar 5, 2021
@ahukkanen ahukkanen deleted the fix/impersonate_infinite_loop2 branch March 5, 2021 19:47
entantoencuanto added a commit that referenced this pull request Mar 5, 2021
* develop:
  Fix infinite loop when impersonated session time runs out (#7221)
  New Crowdin updates (#7543)
  Migrate Admin menus to Menu Registry Part 2 (#7382)
  Replace xls with xlsx (#7421)
  Use cache_key_with_version instead of cache version (#7532)
  Add support for ElectionGuard voting scheme (#7454)
  Fix record encryptor trying to decrypt empty strings (#7542)
  Revert "Don't schedule CI jobs for locales PRs (#7534)" (#7546)
  New Crowdin updates (#7540)
  New Crowdin updates (#7539)
@mrcasals mrcasals added type: fix PRs that implement a fix for a bug module: admin module: core labels Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: admin module: core type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants