fix: `InactivitySessionTimeoutMiddleware`: Check for `last_login` (allow Django admin login + access)
Reapply this PR: The original PR was closed due to inactivity,
Please reconsider and if you do not want to merge it a valid reason why not would be much appreciated.
PR fulfills these requirements
- [x] Commit message(s) and PR title follows the format
[fix|feat|ci|chore|doc]: TICKET-ID: Short description of change madeex.fix: DEV-XXXX: Removed inconsistent code usage causing intermittent errors - [x] Tests for the changes have been added/updated (for bug fixe s/features)
- [ ] Docs have been added/updated (for bug fixes/features)
- [x] Best efforts were made to ensure docs/code are concise and coherent (checked for spelling/grammatical errors, commented out code, debug logs etc.)
- [x] Self-reviewed and ran all changes on a local instance (for bug fixes/features)
Note: I do not (yet) understand what should be the TICKET-ID and how to decide the commit message with this prefix DEV-XXXX, therefore I left it empty for now and just added the commit message. (Suggestion: Add documentation for outside collaborators how they can determine this).
Change has impacts in these area(s)
(check all that apply)
- [ ] Product design
- [ ] Backend (Database)
- [ ] Backend (API)
- [ ] Frontend
Not really sure which one to choose.
Describe the reason for change
Allows login into the Django admin "Out-of-the-box", and makes sure no last_login comparison is made when never logged in. See also the issue Admin Page - Not letting login #4083
What does this fix?
Allow users to login via the Django admin login screen (/admin/login)
What is the new behavior?
The InactivitySessionTimeoutMiddleware won't automatically logout a user when last_login is not set in the user session.
What is the current behavior?
The InactivitySessionTimeoutMiddleware logs out all users which do not have the last_login value set > 0 in the user session.
What libraries were added/updated?
N/A
Does this change affect performance?
No
Does this change affect security?
No
What alternative approaches were there?
I suggest to completely remove the InactivitySessionTimeoutMiddleware in a future release. Session expiration time can be set out of the box via the SESSION_COOKIE_AGE.
If manual extension / adjustion of the session time is required, you can use the .set_expiry() method. (In the /users/login view for example).
What feature flags were used to cover this change?
N/A
Does this PR introduce a breaking change?
(check only one)
- [ ] Yes, and covered entirely by feature flag(s)
- [ ] Yes, and covered partially by feature flag(s)
- [X] No
- [ ] Not sure (briefly explain the situation below)
What level of testing was included in the change?
(check all that apply)
- [ ] e2e
- [X] integration
- [ ] unit
Which logical domain(s) does this change affect?
(for bug fixes/features, be as precise as possible. ex. Authentication, Annotation History, Review Stream etc.) Authentication
Deploy request for heartex-docs pending review.
Visit the deploys page to approve it
| Name | Link |
|---|---|
| Latest commit | 0654a722b8300cb4410cb9ffa43d2b2f32471035 |
Deploy request for label-studio-docs-new-theme pending review.
Visit the deploys page to approve it
| Name | Link |
|---|---|
| Latest commit | 0654a722b8300cb4410cb9ffa43d2b2f32471035 |
@makseq Hi, could you merge the branch manually? All workflows fail because they require a token:
As I'm an outside contributor, I don't have access to this token. And now I receive notifications the pipeline fails, each and every night ;)
Deploy Preview for label-studio-storybook canceled.
| Name | Link |
|---|---|
| Latest commit | 0654a722b8300cb4410cb9ffa43d2b2f32471035 |
| Latest deploy log | https://app.netlify.com/sites/label-studio-storybook/deploys/681335a6e77eab00080d4662 |
@maerteijn sorry for the delay, could you please resolve Linter errors? And I think we can merge it finally
cd label-studio
poetry install
poetry shell
make fmt-all