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

Session expiry banner on the source index page #5716

Closed
1 task
kushaldas opened this issue Jan 18, 2021 · 6 comments
Closed
1 task

Session expiry banner on the source index page #5716

kushaldas opened this issue Jan 18, 2021 · 6 comments

Comments

@kushaldas
Copy link
Contributor

Description

From the release QA steps

Steps to Reproduce

  • Log in to the application server and update the value of SESSION_EXPIRATION_MINUTES from 120 to 2 in /var/www/securedrop/config.py, then restart Apache with sudo systemctl restart apache2
  • ❌ Visit the Source interface index page, and wait for slightly more than 2 minutes. Reload the index page, and verify that no session expiry error message is displayed. Failed, I can see session expiry message.

Expected Behavior

  • no session banner should be shown

Actual Behavior

  • can see a "session expired" banner

Comments

Suggestions to fix, any other relevant information.

@emkll
Copy link
Contributor

emkll commented Jan 18, 2021

@kushaldas I can also reproduce this issue. This appears to be an edge case where the show_expiration_message key set to true is not cleared from the session cookie after the user was logged out of a session due to inactivity at some point. Resetting creating a new Tor identity, or signing in and logging off as a user (without timeout) will clear that from the cookie.

Based on my local testing, this scenario only occurs where the user has already been logged out due to inactivity (and thus the value stored in the cookie).

Provided the analysis above is true, it might not be worth addressing here, given how unlikely this is to occur. What do you think? Is this consistent with your local testing? If you can reproduce in other cases, please check for presence of a set show_expiration_message key (the value of cookie ss is base64 encoded, you can decode locally to inspect the contents

@nabla-c0d3
Copy link
Contributor

nabla-c0d3 commented Jan 20, 2021

FYI I fixed this in the following PR, when I refactored the session expiration logic: https://github.com/freedomofpress/securedrop/pull/5694/files#diff-c2cc3d723ead9193ae70023970aee1a19eaec93bf2c0ffbf1a9a19d89dfb00feL124 .

@kushaldas
Copy link
Contributor Author

Provided the analysis above is true, it might not be worth addressing here, given how unlikely this is to occur. What do you think? Is this consistent with your local testing?

Yes, I also prefer to have a fix later as this is a very rare case. I got the same behaviour as you documented here.

@sssoleileraaa
Copy link
Contributor

Here are some related STRs: #5741. I'll keep them separate for now during QA but it probably makes sense to merge them into this Issue if we can't fix one or any of them during QA.

@nabla-c0d3
Copy link
Contributor

This was fixed by #5694 and #5695.

@eloquence
Copy link
Member

Thanks, closing accordingly, we'll test session management extensively during QA as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants