Cookie code refactor, Use CGI::Cookie and support samesite

Dockerfile

@@ -272,7 +272,7 @@ RUN echo "PATH=$PATH:$APP_ROOT/webwork2/bin" >> /root/.bashrc \
 
 # Phase 6 - install additional Perl modules from CPAN (not packaged for Ubuntu or outdated in Ubuntu)
 
-RUN cpanm install Statistics::R::IO \
+RUN cpanm install Statistics::R::IO CGI::Cookie \
     && rm -fr ./cpanm /root/.cpanm /tmp/*
 
 # Now installed from Ubuntu packages:

bin/check_modules.pl

@@ -32,7 +32,6 @@
 
 my @apache2ModulesList = qw(
 	Apache2::Request
-	Apache2::Cookie
 	Apache2::ServerRec
 	Apache2::ServerUtil
 );
@@ -44,6 +43,7 @@
 	Benchmark
 	Carp
 	CGI
+	CGI:Cookie
 	Class::Accessor
 	Dancer
 	Dancer::Plugin::Database

lib/WeBWorK/Authen.pm

@@ -1,13 +1,12 @@
 ################################################################################
 # WeBWorK Online Homework Delivery System
-# Copyright © 2000-2018 The WeBWorK Project, http://openwebwork.sf.net/
-# $CVSHeader: webwork2/lib/WeBWorK/Authen.pm,v 1.63 2012/06/06 22:03:15 wheeler Exp $
-# 
+# Copyright © 2000-2020 The WeBWorK Project, https://openwebworkorg.wordpress.com/
+#
 # This program is free software; you can redistribute it and/or modify it under
 # the terms of either: (a) the GNU General Public License as published by the
 # Free Software Foundation; either version 2, or (at your option) any later
 # version, or (b) the "Artistic License" which comes with this package.
-# 
+#
 # This program is distributed in the hope that it will be useful, but WITHOUT
 # ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
 # FOR A PARTICULAR PURPOSE.  See either the GNU General Public License or the
@@ -77,7 +76,7 @@ our $GENERIC_ERROR_MESSAGE = "";  # define in new
 ## WeBWorK-tr end modification 
 #####################
 
-use constant COOKIE_LIFESPAN => 60*60*24*30; # 30 days
+# Replaced constant COOKIE_LIFESPAN with a course environment setting which defaults to 30 days.
 #use constant GENERIC_ERROR_MESSAGE => "Invalid user ID or password.";
 
 
@@ -378,27 +377,6 @@ sub get_credentials {
 			#croak ("cookieUser = $cookieUser and paramUser = ". $r->param("user") . " are different.");
 			$self->maybe_kill_cookie; # use parameter "user" rather than cookie "user";
 		}
-# I don't understand this next segment.
-# If both key and $cookieKey exist then why not just ignore the cookieKey?
-
-# 		if (defined $cookieKey and defined $r->param("key")) {
-# 			$self -> {user_id} = $cookieUser;
-# 			$self -> {password} = $r->param("passwd");
-# 			$self -> {login_type} = "normal";
-# 			$self -> {credential_source} = "params_and_cookie";
-# 			$self -> {session_key} = $cookieKey;
-# 			$self -> {cookie_timestamp} = $cookieTimeStamp;
-# 			if ($cookieKey ne $r->param("key")) {
-# 				warn ("cookieKey = $cookieKey and param key = " . $r -> param("key") . " are different, perhaps"
-# 					 ." because you opened several windows for the same site and then backed up from a newer one to an older one."
-# 					 ."  Avoid doing so.");
-# 			$self -> {credential_source} = "conflicting_params_and_cookie";
-# 			}
-# 			debug("params and cookie user '", $self->{user_id}, "' credential_source = '", $self->{credential_source},
-# 				"' params and cookie session key = '", $self->{session_key}, "' cookie_timestamp '", $self->{cookieTimeStamp}, "'");
-# 			return 1;
-# 		} els
-
 # Use session key for verification
 # else   use cookieKey for verification
 # else    use cookie user name but use password provided by request.
@@ -866,42 +844,17 @@ sub fetchCookie {
 
 	my $courseID = $urlpath->arg("courseID");
 
-	# AP2 - Apache2::Cookie needs $r, Apache::Cookie doesn't
-    #my %cookies = WeBWorK::Cookie->fetch( MP2 ? $r : () );
-    #my $cookie = $cookies{"WeBWorKCourseAuthen.$courseID"};
-
 	my $cookie = undef;
-	if (MP2) {
-
-		my $jar = undef;
- 		eval {
-       			$jar = $r->jar; #table of cookies
-  		};
-  		if (ref $@ and $@->isa("APR::Request::Error") ) {
-			debug("Error parsing cookies, will use a partial result");
-     			$jar = $@->jar; # table of successfully parsed cookies
-  		};
-		if ($jar) {
-			$cookie = uri_unescape($jar->get("WeBWorKCourseAuthen.$courseID"));
-		};
-	} else {
-		my %cookies = WeBWorK::Cookie->fetch();
-		$cookie = $cookies{"WeBWorKCourseAuthen.$courseID"};
-		if ($cookie) {
-			debug("found a cookie for this course: '", $cookie->as_string, "'");
-			$cookie = $cookie->value;
-		}
-	}
+	my %cookies = WeBWorK::Cookie->fetch();
+	$cookie = $cookies{"WeBWorKCourseAuthen.$courseID"};
 
 	if ($cookie) {
-        #debug("found a cookie for this course: '", $cookie->as_string, "'");
-        #debug("cookie has this value: '", $cookie->value, "'");
-        #my ($userID, $key) = split "\t", $cookie->value;
-        debug("cookie has this value: '", $cookie, "'");
-        my ($userID, $key, $timestamp) = split "\t", $cookie;
+		#debug("found a cookie for this course: '", $cookie->as_string, "'");
+	        debug("cookie has this value: '", $cookie->value, "'");
+		my ($userID, $key, $timestamp) = split "\t", $cookie->value;
 		if (defined $userID and defined $key and $userID ne "" and $key ne "") {
 			debug("looks good, returning userID='$userID' key='$key'");
- 			return $userID, $key, $timestamp;
+			return $userID, $key, $timestamp;
 		} else {
 			debug("malformed cookie. returning nothing.");
 			return;
@@ -916,27 +869,36 @@ sub sendCookie {
 	my ($self, $userID, $key) = @_;
 	my $r = $self->{r};
 	my $ce = $r->ce;
-	
+
 	my $courseID = $r->urlpath->arg("courseID");
-	
+
  	my $timestamp = time();
-
-	my $cookie = WeBWorK::Cookie->new($r,
-		-name    => "WeBWorKCourseAuthen.$courseID",
- 		-value   => "$userID\t$key\t$timestamp",
-		-path    => $ce->{webworkURLRoot},
-		-secure  => 0,
+
+	my $sameSite  = ( defined($ce->{CookieSameSite}  ) ) ? $ce->{CookieSameSite}  : "Strict" ; # Default to Strict
+	my $secure    = ( defined($ce->{CookieSecure}    ) ) ? $ce->{CookieSecure}    : 0 ;        # Default to NOT setting secure as it requires https
+	my $lifetime  = ( defined($ce->{CookieLifeTime}  ) ) ? $ce->{CookieLifeTime}  : "+6h" ;    # Default to 6 hours (for when session_management_via is session_cookie)
+	my $lifetime2 = ( defined($ce->{CookieLifeTime2} ) ) ? $ce->{CookieLifeTime2} : "+30d" ;   # Default to 30 days (for when session_management_via is NOT session_cookie)
+
+	my $cookie = WeBWorK::Cookie->new(
+		-name     => "WeBWorKCourseAuthen.$courseID",
+		-value    => "$userID\t$key\t$timestamp",
+		-path     => $ce->{webworkURLRoot},
+		-samesite => $sameSite,
+		-secure   => $secure,
 	);
 
 	if ($ce->{session_management_via} ne "session_cookie") {
-		my $expires = time2str("%a, %d-%h-%Y %H:%M:%S %Z", time+COOKIE_LIFESPAN, "GMT");
-		$cookie -> expires($expires);
+		$cookie->expires( $lifetime2 );
+		$cookie->max_age( $lifetime2 );
+	} else {
+		$cookie->expires( $lifetime );
+		$cookie->max_age( $lifetime );
 	}
  	if ($r->hostname ne "localhost" && $r->hostname ne "127.0.0.1") {
- 		$cookie -> domain($r->hostname);    # if $r->hostname = "localhost" or "127.0.0.1", then this must be omitted.
+		$cookie->domain($r->hostname);    # if $r->hostname = "localhost" or "127.0.0.1", then this must be omitted.
 	}
-	
-	#debug("about to add Set-Cookie header with this string: '", $cookie->as_string, "'");
+
+#	debug("about to add Set-Cookie header with this string: '", $cookie->as_string, "'");
  	eval {$r->headers_out->set("Set-Cookie" => $cookie->as_string);};
  	if ($@) {croak $@; }
 }
@@ -945,21 +907,25 @@ sub killCookie {
 	my ($self) = @_;
 	my $r = $self->{r};
 	my $ce = $r->ce;
-	
+
 	my $courseID = $r->urlpath->arg("courseID");
-
-	my $expires = time2str("%a, %d-%h-%Y %H:%M:%S %Z", time-60*60*24, "GMT");
-	my $cookie = WeBWorK::Cookie->new($r,
-		-name => "WeBWorKCourseAuthen.$courseID",
-		-value => "\t",
-		-expires => $expires,
-		-path => $ce->{webworkURLRoot},
-		-secure => 0,
+
+	my $sameSite = ( defined($ce->{CookieSameSite} ) ) ? $ce->{CookieSameSite} : "Strict" ;
+	my $secure   = ( defined($ce->{CookieSecure}   ) ) ? $ce->{CookieSecure}   : 0 ;
+
+	my $cookie = WeBWorK::Cookie->new(
+		-name      => "WeBWorKCourseAuthen.$courseID",
+		-value     => "\t",
+		'-max-age' => "-1d", # 1 day ago
+		-expires   => "-1d", # 1 day ago
+		-path      => $ce->{webworkURLRoot},
+		-samesite  => $sameSite,
+		-secure    => $secure,
 	);
  	if ($r->hostname ne "localhost" && $r->hostname ne "127.0.0.1") {
- 		$cookie -> domain($r->hostname);  # if $r->hostname = "localhost" or "127.0.0.1", then this must be omitted.
+		$cookie -> domain($r->hostname);  # if $r->hostname = "localhost" or "127.0.0.1", then this must be omitted.
 	}
-	
+
 	#debug( "killCookie is about to set an expired cookie");
 	#debug("about to add Set-Cookie header with this string: '", $cookie->as_string, "'");
  	eval {$r->headers_out->set("Set-Cookie" => $cookie->as_string);};

lib/WeBWorK/ContentGenerator/Logout.pm

@@ -1,7 +1,6 @@
 ################################################################################
 # WeBWorK Online Homework Delivery System
-# Copyright © 2000-2018 The WeBWorK Project, http://openwebwork.sf.net/
-# $CVSHeader: webwork2/lib/WeBWorK/ContentGenerator/Logout.pm,v 1.17 2012/06/08 22:50:50 wheeler Exp $
+# Copyright © 2000-2020 The WeBWorK Project, https://openwebworkorg.wordpress.com/
 # 
 # This program is free software; you can redistribute it and/or modify it under
 # the terms of either: (a) the GNU General Public License as published by the
@@ -27,7 +26,6 @@ use strict;
 use warnings;
 #use CGI qw(-nosticky );
 use WeBWorK::CGI;
-use WeBWorK::Cookie;
 use WeBWorK::Localize;
 
 
@@ -38,20 +36,6 @@ sub pre_header_initialize {
 	my $db = $r->db;
 	my $authen = $r->authen;
 
-	# get rid of stored authentication info (this is kind of a hack. i have a better way
-	# in mind but it requires pretty much rewriting Authen/Login/Logout. :-( FIXME)
-#	$authen->forget_verification;
-#	
-#	my $cookie = WeBWorK::Cookie->new($r,
-#		-name => "WeBWorKAuthentication",
-#		-value => "",
-#		-expires => "-1D",
-#		-domain => $r->hostname,
-#		-path => $ce->{webworkURLRoot},
-#		-secure => 0,
-#	);
-#	$r->headers_out->set("Set-Cookie" => $cookie->as_string);
-#
 	my $userID = $r->param("user_id");
 	my $keyError = '';
 #	eval { $db->deleteKey($userID) };

lib/WeBWorK/Cookie.pm

@@ -1,7 +1,6 @@
 ################################################################################
 # WeBWorK Online Homework Delivery System
-# Copyright © 2000-2018 The WeBWorK Project, http://openwebwork.sf.net/
-# $CVSHeader: webwork2/lib/WeBWorK/Cookie.pm,v 1.1 2006/06/29 21:10:52 sh002i Exp $
+# Copyright © 2000-2020 The WeBWorK Project, https://openwebworkorg.wordpress.com/
 # 
 # This program is free software; you can redistribute it and/or modify it under
 # the terms of either: (a) the GNU General Public License as published by the
@@ -18,7 +17,7 @@ package WeBWorK::Cookie;
 
 =head1 NAME
 
-WeBWorK::Cookie - inherit from either Apache::Cookie or Apache2::Cookie
+WeBWorK::Cookie - inherit from either Apache::Cookie or CGI::Cookie
 depending on mod_perl version.
 
 =head1 SYNOPSIS
@@ -35,13 +34,13 @@ use warnings;
 
 use constant MP2 => ( exists $ENV{MOD_PERL_API_VERSION} and $ENV{MOD_PERL_API_VERSION} >= 2 );
 
-# This class inherits from Apache::Cookie under mod_perl and Apache2::Cookie under mod_perl2
+# This class inherits from Apache::Cookie under mod_perl and CGI::Cookie under mod_perl2
 BEGIN {
 	if (MP2) {
-		require Apache2::Cookie;
-        require APR::Request::Error;
-		Apache2::Cookie->import;
-		push @WeBWorK::Cookie::ISA, "Apache2::Cookie";
+		#require APR::Request::Error;
+		require CGI::Cookie;
+		CGI::Cookie->import;
+		push @WeBWorK::Cookie::ISA, "CGI::Cookie";
 	} else {
 		require Apache::Cookie;
 		Apache::Cookie->import;