Skip to content

Commit

Permalink
fix: DEV-3397: Added session expiration settings and conditions (#3114)
Browse files Browse the repository at this point in the history
* fix: DEV-3397: Added session expiration settings and conditions based on last login and last activity. Also made persistent session optional on login.

* fix: DEV-3397: Adding option to ignore certain URLs for inactivity timeout reset

* fix: DEV-3397: setting last_login to session on user registration

Co-authored-by: Wesley Lima <wesley@heartex.com>
  • Loading branch information
wesleylima and Wesley Lima authored Oct 31, 2022
1 parent 6bd24ca commit 3a022c7
Show file tree
Hide file tree
Showing 9 changed files with 176 additions and 1 deletion.
22 changes: 22 additions & 0 deletions label_studio/core/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django.core.handlers.base import BaseHandler
from django.core.exceptions import MiddlewareNotUsed
from django.middleware.common import CommonMiddleware
from django.contrib.auth import logout
from django.conf import settings

from django.utils.http import escape_leading_slashes
Expand Down Expand Up @@ -153,3 +154,24 @@ def process_view(self, request, view_func, view_args, view_kwargs):
if hasattr(request, 'user') and request.method not in SAFE_METHODS:
if request.user.is_authenticated:
request.user.update_last_activity()

class InactivitySessionTimeoutMiddleWare(CommonMiddleware):
"""Log the user out if they have been logged in for too long
or inactive for too long"""

# paths that don't count as user activity
NOT_USER_ACTIVITY_PATHS = []
def process_request(self, request) -> None:
if not hasattr(request, "session") or request.session.is_empty():
return

current_time = time.time()
last_login = request.session['last_login'] if 'last_login' in request.session else 0

# Check if this request is too far from when the login happened
if (current_time - last_login) > settings.MAX_SESSION_AGE:
logout(request)

# Push the expiry to the max every time a new request is made to a url that indicates user activity
if str(request.path_info) not in self.NOT_USER_ACTIVITY_PATHS:
request.session.set_expiry(settings.MAX_TIME_BETWEEN_ACTIVITY)
7 changes: 7 additions & 0 deletions label_studio/core/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import logging
import json

from datetime import timedelta
from label_studio.core.utils.params import get_bool_env, get_env

formatter = 'standard'
Expand Down Expand Up @@ -384,6 +385,12 @@
CSRF_COOKIE_SECURE = bool(int(get_env('CSRF_COOKIE_SECURE', SESSION_COOKIE_SECURE)))
CSRF_COOKIE_HTTPONLY = bool(int(get_env('CSRF_COOKIE_HTTPONLY', SESSION_COOKIE_SECURE)))

# The most time a login will last, regardless of activity
MAX_SESSION_AGE = int(get_env('MAX_SESSION_AGE', timedelta(days=8).total_seconds()))

# The most time that can elapse between activity with the server before the user is logged out
MAX_TIME_BETWEEN_ACTIVITY = int(get_env('MAX_TIME_BETWEEN_ACTIVITY', timedelta(minutes=15).total_seconds()))

SSRF_PROTECTION_ENABLED = get_bool_env('SSRF_PROTECTION_ENABLED', False)

# user media files
Expand Down
1 change: 1 addition & 0 deletions label_studio/core/settings/label_studio.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

MIDDLEWARE.append('organizations.middleware.DummyGetSessionMiddleware')
MIDDLEWARE.append('core.middleware.UpdateLastActivityMiddleware')
MIDDLEWARE.append('core.middleware.InactivitySessionTimeoutMiddleWare')

ADD_DEFAULT_ML_BACKENDS = False

Expand Down
9 changes: 9 additions & 0 deletions label_studio/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from moto import mock_s3
from copy import deepcopy
from pathlib import Path
from datetime import timedelta

from django.conf import settings
from projects.models import Project
Expand All @@ -21,6 +22,8 @@
from organizations.models import Organization
from types import SimpleNamespace

from label_studio.core.utils.params import get_bool_env, get_env

# if we haven't this package, pytest.ini::env doesn't work
try:
import pytest_env.plugin
Expand Down Expand Up @@ -419,3 +422,9 @@ def local_files_document_root_tempdir(settings):
def local_files_document_root_subdir(settings):
tempdir = Path(tempfile.gettempdir()) / Path('files')
settings.LOCAL_FILES_DOCUMENT_ROOT = str(tempdir)


@pytest.fixture(name="testing_session_timeouts")
def set_testing_session_timeouts(settings):
settings.MAX_SESSION_AGE = int(get_env('MAX_SESSION_AGE', timedelta(seconds=5).total_seconds()))
settings.MAX_TIME_BETWEEN_ACTIVITY = int(get_env('MAX_TIME_BETWEEN_ACTIVITY', timedelta(seconds=2).total_seconds()))
123 changes: 123 additions & 0 deletions label_studio/tests/sessions.tavern.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
---
test_name: sessions
strict: false
marks:
- usefixtures:
- django_live_url
- testing_session_timeouts
stages:
- id: signup
name: Sign up
request:
url: "{django_live_url}/user/signup"
data:
email: test_suites_user@heartex.com
password: 12345678
method: POST
response:
status_code: 302

- id: login
name: Login
request:
url: "{django_live_url}/user/login"
data:
email: test_suites_user@heartex.com
password: 12345678
method: POST
response:
status_code: 302

# A request right after login should be sucessful
- name: get_projects
request:
method: POST
url: '{django_live_url}/api/projects'
response:
save:
json:
pk: id
status_code: 201
delay_after: 2

# After MAX_TIME_BETWEEN_ACTIVITY has passed, the session will be over and requests will be denied
- name: get_projects
request:
method: POST
url: '{django_live_url}/api/projects'
response:
save:
json:
pk: id
status_code: 401

# login again
- id: login
name: Login
request:
url: "{django_live_url}/user/login"
data:
email: test_suites_user@heartex.com
password: 12345678
method: POST
response:
status_code: 302
delay_after: 1

# make another request within MAX_TIME_BETWEEN_ACTIVITY
- name: get_projects_1
request:
method: POST
url: '{django_live_url}/api/projects'
response:
save:
json:
pk: id
status_code: 201
delay_after: 1

# and five more
- name: get_projects_2
request:
method: POST
url: '{django_live_url}/api/projects'
response:
save:
json:
pk: id
status_code: 201
delay_after: 1

- name: get_projects_3
request:
method: POST
url: '{django_live_url}/api/projects'
response:
save:
json:
pk: id
status_code: 201
delay_after: 1

- name: get_projects_4
request:
method: POST
url: '{django_live_url}/api/projects'
response:
save:
json:
pk: id
status_code: 201
delay_after: 1

# and by now we reach MAX_SESSION_AGE and hte session end even if we were active
- name: get_projects_5
request:
method: POST
url: '{django_live_url}/api/projects'
response:
save:
json:
pk: id
status_code: 401

4 changes: 3 additions & 1 deletion label_studio/users/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class LoginForm(forms.Form):
email = forms.CharField(label='User') if settings.USE_USERNAME_FOR_LOGIN\
else forms.EmailField(label='Email')
password = forms.CharField(widget=forms.PasswordInput())
persist_session = forms.BooleanField(widget=forms.CheckboxInput(), required=False)

def clean(self, *args, **kwargs):
cleaned = super(LoginForm, self).clean()
Expand All @@ -47,7 +48,8 @@ def clean(self, *args, **kwargs):
user = auth.authenticate(email=email, password=password)

if user and user.is_active:
return {'user': user}
persist_session = cleaned.get('persist_session', False)
return {'user': user, 'persist_session': persist_session}
else:
raise forms.ValidationError(INVALID_USER_ERROR)

Expand Down
2 changes: 2 additions & 0 deletions label_studio/users/functions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license.
"""
import uuid
from time import time

from django import forms
from django.conf import settings
Expand Down Expand Up @@ -78,5 +79,6 @@ def proceed_registration(request, user_form, organization_form, next_page):
# save user to db
save_user = load_func(settings.SAVE_USER)
response = save_user(request, next_page, user_form)
request.session['last_login'] = time()

return response
4 changes: 4 additions & 0 deletions label_studio/users/templates/users/user_login.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
</p>
{% endfor %}
{% endif %}
<p>
<input type="checkbox" id="persist_session" name="persist_session" class="ls-checkbox" checked="checked" style="width: auto;" />
<label for="persist_session">Keep me logged in this browser</label>
</p>
<p><button type="submit" aria-label="Log In" class="ls-button ls-button_look_primary">Log in</button></p>
</form>

Expand Down
5 changes: 5 additions & 0 deletions label_studio/users/views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license.
"""
import logging
from time import time

from django.contrib.auth.decorators import login_required
from django.shortcuts import render, redirect, reverse
Expand Down Expand Up @@ -89,6 +90,10 @@ def user_login(request):
if form.is_valid():
user = form.cleaned_data['user']
auth.login(request, user, backend='django.contrib.auth.backends.ModelBackend')
request.session['last_login'] = time()
if form.cleaned_data['persist_session'] is not True:
# Set the session to expire when the browser is closed
request.session.set_expiry(0)

# user is organization member
org_pk = Organization.find_by_user(user).pk
Expand Down

0 comments on commit 3a022c7

Please sign in to comment.