Skip to content

Commit

Permalink
Match dashboard cookie and token lifetimes and increase its duration
Browse files Browse the repository at this point in the history
There are three relevant duration here:

- The length of the cookie for the dashboard
- The length of the token that is encoded in the cookies
- The length of the token that's part of JSConfig

This PR makes sure that all these match and increases its duration to 7
days.
  • Loading branch information
marcospri committed Aug 12, 2024
1 parent 77b7537 commit 446409a
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 29 deletions.
9 changes: 8 additions & 1 deletion lms/resources/_js_config/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import functools
import re
from datetime import timedelta
from enum import Enum
from typing import Any

Expand Down Expand Up @@ -251,9 +252,15 @@ def enable_error_dialog_mode(self, error_code, error_details=None, message=None)
)
)

def enable_dashboard_mode(self) -> None:
def enable_dashboard_mode(self, token_lifetime_seconds: int) -> None:
self._config.update(
{
"api": {
# We use a different lifetime for the token on the dashboards (that matches the cookies max_age)
"authToken": BearerTokenSchema(self._request).authorization_param(
self._lti_user, timedelta(seconds=token_lifetime_seconds)
)
},
"mode": JSConfig.Mode.DASHBOARD,
"dashboard": DashboardConfig(
user=self._get_user_info(),
Expand Down
7 changes: 5 additions & 2 deletions lms/validation/authentication/_bearer_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,22 @@ def __init__(self, request):
self._lti_user_service = request.find_service(iface=LTIUserService)
self._secret = request.registry.settings["jwt_secret"]

def authorization_param(self, lti_user: LTIUser) -> str:
def authorization_param(
self, lti_user: LTIUser, lifetime: timedelta = timedelta(hours=24)
) -> str:
"""
Return ``lti_user`` serialized into an authorization param.
Returns a ``"Bearer: <ENCODED_JWT>"`` string suitable for use as the
value of an authorization param.
:arg lti_user: the LTI user to return an auth param for
:arg lifetime: how long the token should be valid for
"""
token = self._jwt_service.encode_with_secret(
self._lti_user_service.serialize(lti_user) if lti_user else {},
self._secret,
lifetime=timedelta(hours=24),
lifetime=lifetime,
)

return f"Bearer {token}"
Expand Down
22 changes: 17 additions & 5 deletions lms/views/dashboard/views.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
from datetime import timedelta

from pyramid.httpexceptions import HTTPFound
from pyramid.view import forbidden_view_config, view_config

from lms.security import Permissions
from lms.services import OrganizationService
from lms.validation.authentication import BearerTokenSchema

AUTHORIZATION_DURATION_SECONDS = 60 * 60 * 24 * 7 # One week


@forbidden_view_config(
route_name="dashboard.launch.assignment",
Expand Down Expand Up @@ -84,7 +88,9 @@ def assignment_show(self):
Authenticated via the LTIUser present in a cookie making this endpoint accessible directly in the browser.
"""
assignment = self.dashboard_service.get_request_assignment(self.request)
self.request.context.js_config.enable_dashboard_mode()
self.request.context.js_config.enable_dashboard_mode(
AUTHORIZATION_DURATION_SECONDS
)
self._set_lti_user_cookie(self.request.response)
return {"title": assignment.title}

Expand All @@ -106,7 +112,9 @@ def course_show(self):
Authenticated via the LTIUser present in a cookie making this endpoint accessible directly in the browser.
"""
course = self.dashboard_service.get_request_course(self.request)
self.request.context.js_config.enable_dashboard_mode()
self.request.context.js_config.enable_dashboard_mode(
AUTHORIZATION_DURATION_SECONDS
)
self._set_lti_user_cookie(self.request.response)
return {"title": course.lms_name}

Expand All @@ -127,7 +135,9 @@ def courses(self):
Authenticated via the LTIUser present in a cookie making this endpoint accessible directly in the browser.
"""
self.request.context.js_config.enable_dashboard_mode()
self.request.context.js_config.enable_dashboard_mode(
AUTHORIZATION_DURATION_SECONDS
)
self._set_lti_user_cookie(self.request.response)
# Org names are not 100% ready for public consumption, let's hardcode a title for now.
return {"title": "All courses"}
Expand All @@ -139,7 +149,9 @@ def _set_lti_user_cookie(self, response):
return response
auth_token = (
BearerTokenSchema(self.request)
.authorization_param(lti_user)
.authorization_param(
lti_user, lifetime=timedelta(seconds=AUTHORIZATION_DURATION_SECONDS)
)
# White space is not allowed as a cookie character, remove the leading part
.replace("Bearer ", "")
)
Expand All @@ -150,6 +162,6 @@ def _set_lti_user_cookie(self, response):
httponly=True,
# Scope the cookie to all dashboard views
path="/dashboard",
max_age=60 * 60 * 24, # 24 hours, matches the lifetime of the auth_token
max_age=AUTHORIZATION_DURATION_SECONDS,
)
return response
13 changes: 10 additions & 3 deletions tests/unit/lms/resources/_js_config/__init___test.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from datetime import timedelta
from unittest.mock import create_autospec, sentinel

import pytest
Expand Down Expand Up @@ -694,10 +695,16 @@ def LTIEvent(self, patch):


class TestEnableDashboardMode:
def test_it(self, js_config, lti_user):
js_config.enable_dashboard_mode()
def test_it(self, js_config, lti_user, bearer_token_schema):
js_config.enable_dashboard_mode(token_lifetime_seconds=100)
config = js_config.asdict()

bearer_token_schema.authorization_param.assert_called_with(
lti_user, timedelta(seconds=100)
)
assert config["api"] == {
"authToken": bearer_token_schema.authorization_param.return_value
}
assert config["mode"] == JSConfig.Mode.DASHBOARD
assert config["dashboard"] == {
"user": {"display_name": lti_user.display_name, "is_staff": False},
Expand All @@ -715,7 +722,7 @@ def test_it(self, js_config, lti_user):

def test_user_when_staff(self, js_config, pyramid_request_staff_member, context):
js_config = JSConfig(context, pyramid_request_staff_member)
js_config.enable_dashboard_mode()
js_config.enable_dashboard_mode(token_lifetime_seconds=100)
config = js_config.asdict()

assert config["dashboard"]["user"] == {
Expand Down
32 changes: 14 additions & 18 deletions tests/unit/lms/views/dashboard/views_test.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from datetime import timedelta
from unittest.mock import create_autospec, sentinel

import pytest
Expand All @@ -7,7 +8,7 @@

from lms.resources._js_config import JSConfig
from lms.resources.dashboard import DashboardResource
from lms.views.dashboard.views import DashboardViews
from lms.views.dashboard.views import AUTHORIZATION_DURATION_SECONDS, DashboardViews

pytestmark = pytest.mark.usefixtures(
"h_api",
Expand All @@ -28,15 +29,13 @@ def test_assignment_redirect_from_launch(
response = views.assignment_redirect_from_launch()

BearerTokenSchema.return_value.authorization_param.assert_called_once_with(
pyramid_request.lti_user
pyramid_request.lti_user,
lifetime=timedelta(seconds=AUTHORIZATION_DURATION_SECONDS),
)
assert response == Any.instance_of(HTTPFound).with_attrs(
{"location": "http://example.com/dashboard/assignments/sentinel.id"}
)
assert (
response.headers["Set-Cookie"]
== "authorization=TOKEN; Max-Age=86400; Path=/dashboard; expires=Tue, 02-Apr-2024 12:00:00 GMT; secure; HttpOnly"
)
self.assert_cookie_value(response)

@freeze_time("2024-04-01 12:00:00")
@pytest.mark.usefixtures("BearerTokenSchema")
Expand All @@ -52,10 +51,7 @@ def test_assignment_show(self, views, pyramid_request, dashboard_service):
pyramid_request
)
pyramid_request.context.js_config.enable_dashboard_mode.assert_called_once()
assert (
pyramid_request.response.headers["Set-Cookie"]
== "authorization=TOKEN; Max-Age=86400; Path=/dashboard; expires=Tue, 02-Apr-2024 12:00:00 GMT; secure; HttpOnly"
)
self.assert_cookie_value(pyramid_request.response)

@freeze_time("2024-04-01 12:00:00")
@pytest.mark.usefixtures("BearerTokenSchema")
Expand All @@ -69,10 +65,7 @@ def test_course_show(self, views, pyramid_request, dashboard_service):

dashboard_service.get_request_course.assert_called_once_with(pyramid_request)
pyramid_request.context.js_config.enable_dashboard_mode.assert_called_once()
assert (
pyramid_request.response.headers["Set-Cookie"]
== "authorization=TOKEN; Max-Age=86400; Path=/dashboard; expires=Tue, 02-Apr-2024 12:00:00 GMT; secure; HttpOnly"
)
self.assert_cookie_value(pyramid_request.response)

@freeze_time("2024-04-01 12:00:00")
@pytest.mark.usefixtures("BearerTokenSchema")
Expand All @@ -84,10 +77,7 @@ def test_organization_show(self, views, pyramid_request):
views.courses()

pyramid_request.context.js_config.enable_dashboard_mode.assert_called_once()
assert (
pyramid_request.response.headers["Set-Cookie"]
== "authorization=TOKEN; Max-Age=86400; Path=/dashboard; expires=Tue, 02-Apr-2024 12:00:00 GMT; secure; HttpOnly"
)
self.assert_cookie_value(pyramid_request.response)

def test_assignment_show_with_no_lti_user(
self, views, pyramid_request, dashboard_service
Expand All @@ -105,6 +95,12 @@ def test_assignment_show_with_no_lti_user(
)
pyramid_request.context.js_config.enable_dashboard_mode.assert_called_once()

def assert_cookie_value(self, response):
assert (
response.headers["Set-Cookie"]
== f"authorization=TOKEN; Max-Age={AUTHORIZATION_DURATION_SECONDS}; Path=/dashboard; expires=Mon, 08-Apr-2024 12:00:00 GMT; secure; HttpOnly"
)

@pytest.fixture
def BearerTokenSchema(self, patch):
mock = patch("lms.views.dashboard.views.BearerTokenSchema")
Expand Down

0 comments on commit 446409a

Please sign in to comment.