Skip to content

Commit

Permalink
Merge pull request #2412 from drgrice1/cookie-lifetime-use-session-ke…
Browse files Browse the repository at this point in the history
…y-timeout

Remove the $CookieLifeTime setting and use $sessionKeyTimeout instead.
  • Loading branch information
pstaabp committed May 15, 2024
2 parents 3de656d + 392932f commit 2062a7f
Show file tree
Hide file tree
Showing 14 changed files with 108 additions and 54 deletions.
26 changes: 14 additions & 12 deletions conf/defaults.config
Original file line number Diff line number Diff line change
Expand Up @@ -891,8 +891,8 @@ $default_permission_level = $userRoles{student};
# Session options
################################################################################

# $sessionKeyTimeout defines seconds of inactivity before a key expires
$sessionKeyTimeout = 60*30;
# $sessionTimeout defines seconds of inactivity before a user's session expires.
$sessionTimeout = 60*30;

# $sessionKeyLength defines the length (in characters) of the session key
$sessionKeyLength = 32;
Expand All @@ -917,12 +917,13 @@ $gatewayGracePeriod = 120;

# Setting $session_management_via="key" uses the key database for session
# management. If password authentication is used, then a user can opt to use a
# session cookie with a duration determined by the $CookieLifeTime setting below
# by checking the "Remember Me" checkbox on the login page.
# session cookie with a duration determined by the $sessionTimeout setting
# above by checking the "Remember Me" checkbox on the login page.

# Setting $session_management_via="session_cookies" uses a session cookie to
# manage the session. Note that even in this case a key is stored in the
# database which is compared to the key stored in the session cookie.
# database which is compared to the key stored in the session cookie. The
# lifetime of the cookie is determined by the $sessionTimeout setting above.

# Note that the key database method is less secure as the key must be embeded in
# the page and added as a url parameter in order to maintain the session. These
Expand Down Expand Up @@ -950,13 +951,14 @@ $CookieSameSite = "Lax";
# The default is 0, as 1 will not work without https.
$CookieSecure = 0;

# The CookieLifeTime setting determines how long the browser should retain the
# cookie. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie.
# The CookieLifeTime value should be numeric and in seconds, or should be set to
# "session", in which case the cookie will expire when the browser session ends
# (a "session cookie"). Note that a value of 0 also means the cookie will expire
# when the browser session ends. The default value is 7 days.
$CookieLifeTime = 604800;
# If $useSessionCookie is set to 1, then a "session" cookie will be used. This
# means that the cookie will be deleted when the browser session ends.
# Typically, this is when the browser is closed. Note that the browser defines
# when this is, and some browser's also allow sessions to be restored when the
# browser is reopened. In any case, a user's session will end when the session
# is idle for more than the number of seconds the $sessionTimeout value is set
# to.
$useSessionCookie = 0;

################################################################################
# Two Factor Authentication
Expand Down
37 changes: 19 additions & 18 deletions conf/localOverrides.conf.dist
Original file line number Diff line number Diff line change
Expand Up @@ -523,12 +523,13 @@ $mail{feedbackRecipients} = [

# Setting $session_management_via="key" uses the key database for session
# management. If password authentication is used, then a user can opt to use a
# session cookie with a duration determined by the $CookieLifeTime setting below
# by checking the "Remember Me" checkbox on the login page.
# session cookie with a duration determined by the $sessionTimeout setting
# below by checking the "Remember Me" checkbox on the login page.

# Setting $session_management_via="session_cookies" uses a session cookie to
# manage the session. Note that even in this case a key is stored in the
# database which is compared to the key stored in the session cookie.
# database which is compared to the key stored in the session cookie. The
# lifetime of the cookie is determined by the $sessionTimeout setting below.

# Note that the key database method is less secure as the key must be embeded in
# the page and added as a url parameter in order to maintain the session. These
Expand All @@ -542,7 +543,7 @@ $mail{feedbackRecipients} = [
# This is the length of time (in seconds) after which a user's session becomes
# invalid if they have no activity. The default is 30 minutes (60*30 seconds).

#$sessionKeyTimeout = 60*60*2;
#$sessionTimeout = 60*60*2;

################################################################################
# Cookie control settings
Expand All @@ -551,12 +552,12 @@ $mail{feedbackRecipients} = [
# Set the value of the samesite attribute of the session cookie:
# See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite
# Notes about the $CookieSameSite options:
# The CookieLifeTime setting determines how long the browser should retain the
# cookie. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie.
# The CookieLifeTime value should be numeric and in seconds, or should be set to
# "session", in which case the cookie will expire when the browser session ends
# (a "session cookie"). Note that a value of 0 also means the cookie will expire
# when the browser session ends. The default value is 7 days.
# The "None" setting should only be used with HTTPS and when $CookieSecure is
# set to 1 below. The "None" setting is also less secure and can allow certain
# types of cross-site attacks. The "Strict" setting can break the links in the
# system generated feedback emails when read in a web mail client. Due to those
# factors, the "Lax" setting is probably the optimal choice for typical webwork2
# servers.
#$CookieSameSite = "None";
#$CookieSameSite = "Strict";
#$CookieSameSite = "Lax";
Expand All @@ -565,14 +566,14 @@ $mail{feedbackRecipients} = [
# The default is 0, as 1 will not work without https.
#$CookieSecure = 1;

# The CookieLifeTime setting determines how long the browser should retain the
# cookie. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie.
# The CookieLifeTime value should be numeric and in seconds, or should be set to
# "session", in which case the cookie will expire when the browser session ends
# (a "session cookie"). Note that a value of 0 also means the cookie will expire
# when the browser session ends. The default value is 7 days.
#$CookieLifeTime = 604800;
#$CookieLifeTime = "session";
# If $useSessionCookie is set to 1, then a "session" cookie will be used. This
# means that the cookie will be deleted when the browser session ends.
# Typically, this is when the browser is closed. Note that the browser defines
# when this is, and some browser's also allow sessions to be restored when the
# browser is reopened. In any case, a user's session will end when the session
# is idle for more than the number of seconds the $sessionTimeout value is set
# to.
#$useSessionCookie = 1;

################################################################################
# Two Factor Authentication
Expand Down
4 changes: 2 additions & 2 deletions conf/webwork2.mojolicious.dist.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
# the list (and move the old secrets down). The first secret is the only one
# that will be used for signing new cookies, but the old secrets will be used
# for validating signatures on existing cookies. Eventually the old secrets
# should be removed (roughly after the length of time set for $CookieLifeTime in
# localOverrides.conf or defaults.config).
# should be removed (roughly after the length of time set for $sessionTimeout
# in localOverrides.conf or defaults.config).
secrets:
- 607280d0b2c621220b554a1c6ed123aa1a96f2de

Expand Down
36 changes: 36 additions & 0 deletions htdocs/js/GatewayQuiz/gateway.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,39 @@
}
};

const basicWebserviceURL = `${webworkConfig?.webwork_url ?? '/webwork2'}/instructor_rpc`;

const updateTimeDelta = async () => {
const authenParams = {};
const user = document.getElementsByName('user')[0];
if (user) authenParams.user = user.value;
const sessionKey = document.getElementsByName('key')[0];
if (sessionKey) authenParams.key = sessionKey.value;

const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 10000);

const response = await fetch(basicWebserviceURL, {
method: 'post',
mode: 'same-origin',
body: new URLSearchParams({
...authenParams,
rpc_command: 'getCurrentServerTime',
courseID: timerDiv.dataset.courseId
}),
signal: controller.signal
}).catch(() => {
/* Errors are ignored */
});

clearTimeout(timeoutId);

if (response && response.ok) {
const data = await response.json();
timeDelta = Math.round(new Date().getTime() / 1000) - data.result_data.currentServerTime;
}
};

if (timerDiv) {
// Initialize the time variables and start the timer.
const dateNow = new Date();
Expand All @@ -120,6 +153,9 @@
timeDelta = browserTime - parseInt(timerDiv.dataset.serverTime);
gracePeriod = parseInt(timerDiv.dataset.gracePeriod);

updateTimeDelta();
setInterval(updateTimeDelta, (parseInt(timerDiv.dataset.sessionTimeout) - 60) * 1000);

const remainingTime = serverDueTime - browserTime + timeDelta;

if (!timerDiv.dataset.acting) {
Expand Down
11 changes: 5 additions & 6 deletions lib/WeBWorK/Authen.pm
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ sub checkPassword {
sub unexpired_session_exists {
my ($self, $userID) = @_;
my $Key = $self->{c}->db->getKey($userID);
return defined $Key && time <= $Key->timestamp + $self->{c}->ce->{sessionKeyTimeout};
return defined $Key && time <= $Key->timestamp + $self->{c}->ce->{sessionTimeout};
}

# Uses an existing session and session key if a key was found previously with a valid timestamp. Otherwise a random key
Expand Down Expand Up @@ -908,8 +908,8 @@ sub check_session {

my $timestampValid =
$ce->{session_management_via} eq 'session_cookie' && defined $self->{cookie_timestamp}
? $currentTime <= $self->{cookie_timestamp} + $ce->{sessionKeyTimeout}
: $currentTime <= $Key->timestamp + $ce->{sessionKeyTimeout};
? $currentTime <= $self->{cookie_timestamp} + $ce->{sessionTimeout}
: $currentTime <= $Key->timestamp + $ce->{sessionTimeout};

if ($keyMatches && $timestampValid && $updateTimestamp) {
$Key->timestamp($currentTime);
Expand Down Expand Up @@ -975,8 +975,7 @@ sub fetchCookie {
# The session cookie is actually sent by Mojolicious when the response is rendered.
sub sendCookie {
my ($self, $userID, $key) = @_;
my $c = $self->{c};
my $ce = $c->ce;
my $c = $self->{c};

return if $c->stash('disable_cookies');

Expand All @@ -987,7 +986,7 @@ sub sendCookie {
key => $key,
timestamp => time,
# Set how long the browser should retain the cookie.
expiration => $ce->{CookieLifeTime} eq 'session' ? 0 : $ce->{CookieLifeTime}
expiration => $c->ce->{useSessionCookie} ? 0 : $c->ce->{sessionTimeout}
);

return;
Expand Down
2 changes: 1 addition & 1 deletion lib/WeBWorK/Authen/Shibboleth.pm
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ sub check_session {
return 0 unless defined $Key;

my $keyMatches = (defined $possibleKey and $possibleKey eq $Key->key);
my $timestampValid = (time <= $Key->timestamp() + $ce->{sessionKeyTimeout});
my $timestampValid = (time <= $Key->timestamp() + $ce->{sessionTimeout});
if ($ce->{shibboleth}{manage_session_timeout}) {
# always valid to allow shib to take control of timeout
$timestampValid = 1;
Expand Down
2 changes: 1 addition & 1 deletion lib/WeBWorK/ConfigValues.pm
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ sub getConfigValues ($ce) {
type => 'popuplist'
},
{
var => 'sessionKeyTimeout',
var => 'sessionTimeout',
doc => x('Inactivity time before a user is required to login again'),
doc2 => x(
'Length of time, in seconds, a user has to be inactive before he is required to login again. '
Expand Down
4 changes: 4 additions & 0 deletions lib/WeBWorK/ContentGenerator/InstructorRPCHandler.pm
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ error occurs, then the response will contain an "error" key.
=cut

# FIXME: This is no longer "instructor" only. Even students can use the getCurrentServerTime command. Really, it never
# was "instructor" only. Usage of all commands is based on permissions, and there have always been non-instructor users
# that have some of these permissions. So this module and the corresponding route should really be renamed.

use JSON;

use WebworkWebservice;
Expand Down
2 changes: 1 addition & 1 deletion lib/WeBWorK/CourseEnvironment.pm
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ and course.conf files.
courseName => "name_of_course",
});
my $timeout = $courseEnv->{sessionKeyTimeout};
my $timeout = $courseEnv->{sessionTimeout};
my $mode = $courseEnv->{pg}->{options}->{displayMode};
# etc...
Expand Down
21 changes: 11 additions & 10 deletions lib/WebworkWebservice.pm
Original file line number Diff line number Diff line change
Expand Up @@ -243,16 +243,17 @@ sub command_permission {
my ($command) = @_;
return {
# WebworkWebservice::CourseActions
createCourse => 'create_and_delete_courses',
listUsers => 'access_instructor_tools',
addUser => 'modify_student_data',
dropUser => 'modify_student_data',
deleteUser => 'modify_student_data',
editUser => 'modify_student_data',
changeUserPassword => 'modify_student_data',
getCourseSettings => 'access_instructor_tools',
updateSetting => 'manage_course_files',
saveFile => 'modify_problem_sets',
createCourse => 'create_and_delete_courses',
listUsers => 'access_instructor_tools',
addUser => 'modify_student_data',
dropUser => 'modify_student_data',
deleteUser => 'modify_student_data',
editUser => 'modify_student_data',
changeUserPassword => 'modify_student_data',
getCourseSettings => 'access_instructor_tools',
updateSetting => 'manage_course_files',
saveFile => 'modify_problem_sets',
getCurrentServerTime => 'record_answers_after_open_date_with_attempts',

# WebworkWebservice::LibraryActions
listLib => 'access_instructor_tools',
Expand Down
11 changes: 10 additions & 1 deletion lib/WebworkWebservice/CourseActions.pm
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ sub listUsers {
$user->{num_user_sets} = $db->countUserSets($user->{user_id}) . '/' . $numGlobalSets;

my $Key = $db->getKey($user->{user_id});
$user->{login_status} = $Key && time <= $Key->timestamp + $ce->{sessionKeyTimeout} ? 'active' : 'inactive';
$user->{login_status} = $Key && time <= $Key->timestamp + $ce->{sessionTimeout} ? 'active' : 'inactive';
}

return {
Expand Down Expand Up @@ -535,4 +535,13 @@ sub saveFile {
};
}

sub getCurrentServerTime {
my ($invocant, $self, $params) = @_;

return {
ra_out => { currentServerTime => $self->c->submitTime },
text => 'Current server time'
};
}

1;
2 changes: 2 additions & 0 deletions templates/ContentGenerator/GatewayQuiz.html.ep
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@
data => {
server_time => int($submitTime),
server_due_time => $c->{set}->due_date,
session_timeout => $ce->{sessionTimeout},
course_id => $ce->{courseName},
grace_period => $ce->{gatewayGracePeriod},
alert_title => maketext('Test Time Notification'),
alert_three => maketext(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
% $previousTime = $record{time} if $previousTime < 0;
% my $upper_limit = $#scores > $#answers ? $#scores : $#answers;
%
<tr <%== $record{time} - $previousTime > $ce->{sessionKeyTimeout}
<tr <%== $record{time} - $previousTime > $ce->{sessionTimeout}
? 'class="table-rule"' : '' %>>
% # Show the problem seed for instructors.
% if ($isInstructor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
% if (
% $db->existsKeyWhere({
% user_id => $user->user_id,
% timestamp => { '>=' => time - $ce->{sessionKeyTimeout} }
% timestamp => { '>=' => time - $ce->{sessionTimeout} }
% })
% )
% {
Expand Down

0 comments on commit 2062a7f

Please sign in to comment.