From e9a00c78bc2b812418b704b076833db7bd9f26ca Mon Sep 17 00:00:00 2001
From: Andjelko Horvat <comel@vingd.com>
Date: Wed, 4 Sep 2013 11:12:48 +0000
Subject: [PATCH] Add and use SimpleSAML_Utilities::setCookie() function (issue
 #567).

git-svn-id: https://simplesamlphp.googlecode.com/svn/trunk@3269 44740490-163a-0410-bde0-09ae8108e29a
---
 lib/SimpleSAML/SessionHandler.php             | 22 ++-----
 lib/SimpleSAML/Utilities.php                  | 60 +++++++++++++++++++
 lib/SimpleSAML/XHTML/IdPDisco.php             | 16 ++---
 lib/SimpleSAML/XHTML/Template.php             | 19 +++---
 modules/cdc/lib/Server.php                    | 22 ++++---
 modules/consent/lib/Consent/Store/Cookie.php  | 23 +++----
 modules/core/www/cleardiscochoices.php        |  3 +-
 modules/core/www/loginuserpass.php            |  4 +-
 modules/core/www/loginuserpassorg.php         |  4 +-
 modules/discopower/lib/PowerIdPDisco.php      | 16 +++--
 .../multiauth/lib/Auth/Source/MultiAuth.php   | 18 +++---
 modules/negotiate/www/disable.php             |  8 ++-
 modules/negotiate/www/enable.php              |  7 ++-
 13 files changed, 139 insertions(+), 83 deletions(-)

diff --git a/lib/SimpleSAML/SessionHandler.php b/lib/SimpleSAML/SessionHandler.php
index ad136e904..a61844ed5 100644
--- a/lib/SimpleSAML/SessionHandler.php
+++ b/lib/SimpleSAML/SessionHandler.php
@@ -130,29 +130,17 @@ abstract class SimpleSAML_SessionHandler {
 	 * @param string $name  The name of the session cookie.
 	 * @param string|NULL $value  The value of the cookie. Set to NULL to delete the cookie.
 	 */
-	public function setCookie($name, $value) {
+	public function setCookie($name, $value, array $params = NULL) {
 		assert('is_string($name)');
 		assert('is_string($value) || is_null($value)');
 
-		$params = $this->getCookieParams();
-
-		// Do not set secure cookie if not on HTTPS
-		if ($params['secure'] && !SimpleSAML_Utilities::isHTTPS()) {
-			SimpleSAML_Logger::warning('Setting secure cookie on http not allowed.');
-			return;
-		}
-
-		if ($value === NULL) {
-			$expire = time() - 365*24*60*60;
-		} elseif ($params['lifetime'] === 0) {
-			$expire = 0;
+		if ($params !== NULL) {
+			$params = array_merge($this->getCookieParams(), $params);
 		} else {
-			$expire = time() + $params['lifetime'];;
+			$params = $this->getCookieParams();
 		}
 
-		if (!setcookie($name, $value, $expire, $params['path'], $params['domain'], $params['secure'], $params['httponly'])) {
-			throw new SimpleSAML_Error_Exception('Error setting cookie - headers already sent.');
-		}
+		SimpleSAML_Utilities::setCookie($name, $value, $params);
 	}
 
 }
diff --git a/lib/SimpleSAML/Utilities.php b/lib/SimpleSAML/Utilities.php
index 7447db632..ad814918c 100644
--- a/lib/SimpleSAML/Utilities.php
+++ b/lib/SimpleSAML/Utilities.php
@@ -2354,4 +2354,64 @@ class SimpleSAML_Utilities {
 		return substr(strtoupper(PHP_OS),0,3) == 'WIN';
 	}
 
+
+	/**
+	 * Set a cookie.
+	 *
+	 * @param string $name  The name of the session cookie.
+	 * @param string|NULL $value  The value of the cookie. Set to NULL to delete the cookie.
+	 * @param array|NULL $params  Cookie parameters.
+	 * @param bool $throw  Whether to throw exception if setcookie fails.
+	 */
+	public static function setCookie($name, $value, array $params = NULL, $throw = TRUE) {
+		assert('is_string($name)');
+		assert('is_string($value) || is_null($value)');
+
+		$default_params = array(
+			'lifetime' => 0,
+			'expire' => NULL,
+			'path' => '/',
+			'domain' => NULL,
+			'secure' => FALSE,
+			'httponly' => TRUE,
+			'raw' => FALSE,
+		);
+
+		if ($params !== NULL) {
+			$params = array_merge($default_params, $params);
+		} else {
+			$params = $default_params;
+		}
+
+		// Do not set secure cookie if not on HTTPS
+		if ($params['secure'] && !self::isHTTPS()) {
+			SimpleSAML_Logger::warning('Setting secure cookie on http not allowed.');
+			return;
+		}
+
+		if ($value === NULL) {
+			$expire = time() - 365*24*60*60;
+		} elseif (isset($params['expire'])) {
+			$expire = $params['expire'];
+		} elseif ($params['lifetime'] === 0) {
+			$expire = 0;
+		} else {
+			$expire = time() + $params['lifetime'];
+		}
+
+		if ($params['raw']) {
+			$success = setrawcookie($name, $value, $expire, $params['path'], $params['domain'], $params['secure'], $params['httponly']);
+		} else {
+			$success = setcookie($name, $value, $expire, $params['path'], $params['domain'], $params['secure'], $params['httponly']);
+		}
+
+		if (!$success) {
+			if ($throw) {
+				throw new SimpleSAML_Error_Exception('Error setting cookie - headers already sent.');
+			} else {
+				SimpleSAML_Logger::warning('Error setting cookie - headers already sent.');
+			}
+		}
+	}
+
 }
diff --git a/lib/SimpleSAML/XHTML/IdPDisco.php b/lib/SimpleSAML/XHTML/IdPDisco.php
index fb17ac156..c0563ad21 100644
--- a/lib/SimpleSAML/XHTML/IdPDisco.php
+++ b/lib/SimpleSAML/XHTML/IdPDisco.php
@@ -190,13 +190,15 @@ class SimpleSAML_XHTML_IdPDisco {
 	protected function setCookie($name, $value) {
 		$prefixedName = 'idpdisco_' . $this->instance . '_' . $name;
 
-		/* We save the cookies for 90 days. */
-		$saveUntil = time() + 60*60*24*90;
-
-		/* The base path for cookies. This should be the installation directory for simpleSAMLphp. */
-		$cookiePath = '/' . $this->config->getBaseUrl();
-
-		setcookie($prefixedName, $value, $saveUntil, $cookiePath);
+		$params = array(
+			/* We save the cookies for 90 days. */
+			'lifetime' => (60*60*24*90),
+			/* The base path for cookies. This should be the installation directory for simpleSAMLphp. */
+			'path' => ('/' . $this->config->getBaseUrl()),
+			'httponly' => FALSE,
+		);
+
+		SimpleSAML_Utilities::setCookie($prefixedName, $value, $params, FALSE);
 	}
 
 
diff --git a/lib/SimpleSAML/XHTML/Template.php b/lib/SimpleSAML/XHTML/Template.php
index 9bb475949..affdfb117 100644
--- a/lib/SimpleSAML/XHTML/Template.php
+++ b/lib/SimpleSAML/XHTML/Template.php
@@ -706,17 +706,14 @@ class SimpleSAML_XHTML_Template {
 		}
 
 		$name = $config->getString('language.cookie.name', 'language');
-		$domain = $config->getString('language.cookie.domain', NULL);
-		$path = $config->getString('language.cookie.path', '/');
-		$lifetime = $config->getInteger('language.cookie.lifetime', 60*60*24*900);
-
-		if ($lifetime === 0) {
-			$expire = 0;
-		} else {
-			$expire = time() + $lifetime;
-		}
-
-		setcookie($name, $language, $expire, $path, $domain);
+		$params = array(
+			'lifetime' => ($config->getInteger('language.cookie.lifetime', 60*60*24*900)),
+			'domain' => ($config->getString('language.cookie.domain', NULL)),
+			'path' => ($config->getString('language.cookie.path', '/')),
+			'httponly' => FALSE,
+		);
+
+		SimpleSAML_Utilities::setCookie($name, $language, $params, FALSE);
 	}
 
 }
diff --git a/modules/cdc/lib/Server.php b/modules/cdc/lib/Server.php
index dfc4f5a6e..5f7636ccc 100644
--- a/modules/cdc/lib/Server.php
+++ b/modules/cdc/lib/Server.php
@@ -203,8 +203,14 @@ class sspmod_cdc_Server {
 	 * @return array  The response.
 	 */
 	private function handleDelete(array $request) {
+		$params = array(
+			'path' => '/',
+			'domain' => '.' . $this->domain,
+			'secure' => TRUE,
+			'httponly' => FALSE,
+		);
 
-		setcookie('_saml_idp', 'DELETE', time() - 86400 , '/', '.' . $this->domain, TRUE);
+		SimpleSAML_Utilities::setCookie('_saml_idp', NULL, $params, FALSE);
 		return 'ok';
 	}
 
@@ -392,13 +398,15 @@ class sspmod_cdc_Server {
 			$cookie = $tmp[1];
 		}
 
-		if ($this->cookieLifetime === 0) {
-			$expire = 0;
-		} else {
-			$expire = time() + $this->cookieLifetime;
-		}
+		$params = array(
+			'lifetime' => $this->cookieLifetime,
+			'path' => '/',
+			'domain' => '.' . $this->domain,
+			'secure' => TRUE,
+			'httponly' => FALSE,
+		);
 
-		setcookie('_saml_idp', $cookie, $expire, '/', '.' . $this->domain, TRUE);
+		SimpleSAML_Utilities::setCookie('_saml_idp', $cookie, $params, FALSE);
 	}
 
 }
diff --git a/modules/consent/lib/Consent/Store/Cookie.php b/modules/consent/lib/Consent/Store/Cookie.php
index d0e638e94..ba59e5c4d 100644
--- a/modules/consent/lib/Consent/Store/Cookie.php
+++ b/modules/consent/lib/Consent/Store/Cookie.php
@@ -266,26 +266,21 @@ class sspmod_consent_Consent_Store_Cookie extends sspmod_consent_Store
         assert('is_string($name)');
         assert('is_string($value)');
 
-        if ($value === null) {
-            $expire = 1; /* Delete by setting expiry in the past. */
-            $value = '';
-        } else {
-            $expire = time() + 90 * 24*60*60;
-        }
+        $globalConfig = SimpleSAML_Configuration::getInstance();
+        $params = array(
+            'lifetime' => (90*24*60*60),
+            'path' => ('/' . $globalConfig->getBaseURL()),
+            'httponly' => FALSE,
+        );
 
         if (SimpleSAML_Utilities::isHTTPS()) {
             /* Enable secure cookie for https-requests. */
-            $secure = true;
+            $params['secure'] = true;
         } else {
-            $secure = false;
+            $params['secure'] = false;
         }
 
-        $globalConfig = SimpleSAML_Configuration::getInstance();
-        $path = '/' . $globalConfig->getBaseURL();
-
-        setcookie($name, $value, $expire, $path, null, $secure);
+        SimpleSAML_Utilities::setCookie($name, $value, $params, FALSE);
     }
 
 }
-
-?>
diff --git a/modules/core/www/cleardiscochoices.php b/modules/core/www/cleardiscochoices.php
index 03456e4f6..7cf7fa03f 100644
--- a/modules/core/www/cleardiscochoices.php
+++ b/modules/core/www/cleardiscochoices.php
@@ -20,7 +20,7 @@ foreach($_COOKIE as $cookieName => $value) {
 	/* Delete the cookie. We delete it once without the secure flag and once with the secure flag. This
 	 * ensures that the cookie will be deleted in any case.
 	 */
-	setcookie($cookieName, '', time() - 24*60*60, $cookiePath);
+	SimpleSAML_Utilities::setCookie($cookieName, NULL, array('path' => $cookiePath, 'httponly' => FALSE), FALSE);
 }
 
 
@@ -35,4 +35,3 @@ if(array_key_exists('ReturnTo', $_REQUEST)) {
 /* Redirect to destination. */
 SimpleSAML_Utilities::redirect($returnTo);
 
-?>
\ No newline at end of file
diff --git a/modules/core/www/loginuserpass.php b/modules/core/www/loginuserpass.php
index 1a4e3f5ce..71da3aeae 100644
--- a/modules/core/www/loginuserpass.php
+++ b/modules/core/www/loginuserpass.php
@@ -56,7 +56,7 @@ if (!empty($_REQUEST['username']) || !empty($password)) {
 		$params = $sessionHandler->getCookieParams();
 		$params['expire'] = time();
 		$params['expire'] += (isset($_REQUEST['remember_username']) && $_REQUEST['remember_username'] == 'Yes' ? 31536000 : -300);
-		setcookie($source->getAuthId() . '-username', $username, $params['expire'], $params['path'], $params['domain'], $params['secure'], $params['httponly']);
+		SimpleSAML_Utilities::setCookie($source->getAuthId() . '-username', $username, $params, FALSE);
 	}
 
 	try {
@@ -96,5 +96,3 @@ if (isset($state['SPMetadata'])) {
 $t->show();
 exit();
 
-
-?>
\ No newline at end of file
diff --git a/modules/core/www/loginuserpassorg.php b/modules/core/www/loginuserpassorg.php
index ba43f9ba4..cda773bfc 100644
--- a/modules/core/www/loginuserpassorg.php
+++ b/modules/core/www/loginuserpassorg.php
@@ -59,7 +59,7 @@ if ($organizations === NULL || !empty($organization)) {
 			$params = $sessionHandler->getCookieParams();
 			$params['expire'] = time();
 			$params['expire'] += (isset($_REQUEST['remember_username']) && $_REQUEST['remember_username'] == 'Yes' ? 31536000 : -300);
-			setcookie($source->getAuthId() . '-username', $username, $params['expire'], $params['path'], $params['domain'], $params['secure'], $params['httponly']);
+			SimpleSAML_Utilities::setCookie($source->getAuthId() . '-username', $username, $params, FALSE);
 		}
 
 		try {
@@ -97,5 +97,3 @@ if (isset($state['SPMetadata'])) {
 $t->show();
 exit();
 
-
-?>
\ No newline at end of file
diff --git a/modules/discopower/lib/PowerIdPDisco.php b/modules/discopower/lib/PowerIdPDisco.php
index bdd4a79e8..072e6afb3 100644
--- a/modules/discopower/lib/PowerIdPDisco.php
+++ b/modules/discopower/lib/PowerIdPDisco.php
@@ -302,13 +302,13 @@ class sspmod_discopower_PowerIdPDisco extends SimpleSAML_XHTML_IdPDisco {
 			$newCookie = $tmp[1];
 		}
 
-		if ($this->cdcLifetime === NULL) {
-			$expire = 0;
-		} else {
-			$expire = time() + $this->cdcLifetime;
-		}
-
-		setcookie('_saml_idp', $newCookie, $expire, '/', $this->cdcDomain, TRUE);
+		$params = array(
+			'lifetime' => $this->cdcLifetime,
+			'domain' => $this->cdcDomain,
+			'secure' => TRUE,
+			'httponly' => FALSE,
+		);
+		SimpleSAML_Utilities::setCookie('_saml_idp', $newCookie, $params, FALSE);
 	}
 
 
@@ -339,5 +339,3 @@ class sspmod_discopower_PowerIdPDisco extends SimpleSAML_XHTML_IdPDisco {
 	}
 
 }
-
-?>
\ No newline at end of file
diff --git a/modules/multiauth/lib/Auth/Source/MultiAuth.php b/modules/multiauth/lib/Auth/Source/MultiAuth.php
index 737eb28a1..2b975d4fe 100644
--- a/modules/multiauth/lib/Auth/Source/MultiAuth.php
+++ b/modules/multiauth/lib/Auth/Source/MultiAuth.php
@@ -199,15 +199,17 @@ class sspmod_multiauth_Auth_Source_MultiAuth extends SimpleSAML_Auth_Source {
 
 		$cookieName = 'multiauth_source_' . $this->authId;
 
-		/* We save the cookies for 90 days. */
-		$saveUntil = time() + 60*60*24*90;
-
-		/* The base path for cookies. 
-		This should be the installation directory for simpleSAMLphp. */
 		$config = SimpleSAML_Configuration::getInstance();
-		$cookiePath = '/' . $config->getBaseUrl();
-
-		setcookie($cookieName, $source, $saveUntil, $cookiePath);
+		$params = array(
+			/* We save the cookies for 90 days. */
+			'lifetime' => (60*60*24*90),
+			/* The base path for cookies.
+			This should be the installation directory for simpleSAMLphp. */
+			'path' => ('/' . $config->getBaseUrl()),
+			'httponly' => FALSE,
+		);
+
+		SimpleSAML_Utilities::setCookie($cookieName, $source, $params, FALSE);
 	}
 
 	/**
diff --git a/modules/negotiate/www/disable.php b/modules/negotiate/www/disable.php
index 021fb7bc4..ca8bb8d00 100644
--- a/modules/negotiate/www/disable.php
+++ b/modules/negotiate/www/disable.php
@@ -9,8 +9,14 @@
  * @version $Id$
  */
 
+$params = array(
+    'expire' => (mktime(0,0,0,1,1,2038)),
+    'secure' => FALSE,
+    'httponly' => TRUE,
+);
+SimpleSAML_Utilities::setCookie('NEGOTIATE_AUTOLOGIN_DISABLE_PERMANENT', 'True', $params, FALSE);
+
 $globalConfig = SimpleSAML_Configuration::getInstance();
-setcookie('NEGOTIATE_AUTOLOGIN_DISABLE_PERMANENT', 'True', mktime(0,0,0,1,1,2038), '/', SimpleSAML_Utilities::getSelfHost(), FALSE, TRUE);
 $session = SimpleSAML_Session::getInstance();
 $session->setData('negotiate:disable', 'session', FALSE, 24*60*60);
 $t = new SimpleSAML_XHTML_Template($globalConfig, 'negotiate:disable.php');
diff --git a/modules/negotiate/www/enable.php b/modules/negotiate/www/enable.php
index 340b9dcd0..e28bde1f9 100644
--- a/modules/negotiate/www/enable.php
+++ b/modules/negotiate/www/enable.php
@@ -9,8 +9,13 @@
  * @version $Id$
  */
 
+$params = array(
+	'secure' => FALSE,
+	'httponly' => TRUE,
+);
+SimpleSAML_Utilities::setCookie('NEGOTIATE_AUTOLOGIN_DISABLE_PERMANENT', NULL, $params, FALSE);
+
 $globalConfig = SimpleSAML_Configuration::getInstance();
-setcookie('NEGOTIATE_AUTOLOGIN_DISABLE_PERMANENT', 'False', time() - 3600, '/', SimpleSAML_Utilities::getSelfHost(), FALSE, TRUE);
 $session = SimpleSAML_Session::getInstance();
 $session->setData('negotiate:disable', 'session', FALSE, 24*60*60);
 $t = new SimpleSAML_XHTML_Template($globalConfig, 'negotiate:enable.php');
-- 
GitLab