From 430a89bfde56e37a8c7b104e995b6c4e1b215eab Mon Sep 17 00:00:00 2001
From: Manuel Vacelet <manuel.vacelet@enalean.com>
Date: Tue, 21 Jan 2014 17:38:22 +0100
Subject: [PATCH] request #5951 - Avoid hardcoded redirect on login when
 possible

When we configure Tuleap behind a reverse proxy that does URL & domain rewriting
most of the time, it doesn't play well with hardcoded redirect (with the domain
name in the redirect request).

As we only need the domain when we change of protocol (http <-> https), starting
this commit, Tuleap will do relative redirects (without domain name) when there
is no protocol change.

As a matter of fact, when Tuleap is configured in such a setup, it should stick
to one protocol (either full HTTP or full HTTPS) and not allow both.

Change-Id: I789d914b466d573453a6c33c8f7b4783f0c66bcd
---
 src/common/include/HTTPRequest.class.php           |    3 +
 src/common/include/URL.class.php                   |    2 +-
 src/common/include/URLRedirect.class.php           |   61 ++++++++++++++
 src/common/include/URLVerification.class.php       |    2 -
 src/common/user/LoginPresenter.class.php           |    1 -
 src/www/include/session.php                        |    2 +-
 src/www/include/utils.php                          |   62 +--------------
 src/www/my/redirect.php                            |   11 +--
 .../simpletest/common/include/URLRedirectTest.php  |   84 ++++++++++++++++++++
 tests/simpletest/common/include/URLTest.php        |    1 +
 .../common/include/URLVerificationTest.php         |   17 +++-
 11 files changed, 169 insertions(+), 77 deletions(-)
 create mode 100644 tests/simpletest/common/include/URLRedirectTest.php

diff --git a/src/common/include/HTTPRequest.class.php b/src/common/include/HTTPRequest.class.php
index 81c0d0c..89b8d85 100644
--- a/src/common/include/HTTPRequest.class.php
+++ b/src/common/include/HTTPRequest.class.php
@@ -136,6 +136,9 @@ class HTTPRequest extends Codendi_Request {
         }
     }
 
+    public function isSSL() {
+        return isset($_SERVER['HTTPS']) && ($_SERVER['HTTPS'] == 'on');
+    }
 }
 
 ?>
diff --git a/src/common/include/URL.class.php b/src/common/include/URL.class.php
index 2c03ae3..a4be3de 100644
--- a/src/common/include/URL.class.php
+++ b/src/common/include/URL.class.php
@@ -144,7 +144,7 @@ class URL {
             return $group_id;
         } else return null;
     }
-    
+
     function getProjectDao() {
         return new ProjectDao(CodendiDataAccess::instance());
     }
diff --git a/src/common/include/URLRedirect.class.php b/src/common/include/URLRedirect.class.php
index 76dcff3..e55acb0 100644
--- a/src/common/include/URLRedirect.class.php
+++ b/src/common/include/URLRedirect.class.php
@@ -51,6 +51,67 @@ class URLRedirect {
         $url = $this->buildReturnToLogin($_SERVER);
         $GLOBALS['HTML']->redirect($url);
     }
+
+    public function makeReturnToUrl(HTTPRequest $request, $url) {
+        $urlToken = parse_url($url);
+
+        $finaleUrl = '';
+
+        $server_url = '';
+        if(array_key_exists('host', $urlToken) && $urlToken['host']) {
+            $server_url = $urlToken['scheme'].'://'.$urlToken['host'];
+            if(array_key_exists('port', $urlToken) && $urlToken['port']) {
+                $server_url .= ':'.$urlToken['port'];
+            }
+        } else {
+            if ($request->isSSL() && $this->shouldRedirectToHTTP($request)) {
+                $server_url = 'http://'.$GLOBALS['sys_default_domain'];
+            }
+        }
+
+        $finaleUrl = $server_url;
+
+        if(array_key_exists('path', $urlToken) && $urlToken['path']) {
+            $finaleUrl .= $urlToken['path'];
+        }
+
+        if($request->existAndNonEmpty('return_to')) {
+            $rt = 'return_to='.urlencode($request->get('return_to'));
+
+            if(array_key_exists('query', $urlToken) && $urlToken['query']) {
+                $finaleUrl .= '?'.$urlToken['query'].'&amp;'.$rt;
+            }
+            else {
+                $finaleUrl .= '?'.$rt;
+            }
+            if (strstr($request->get('return_to'),'pv=2')) {
+                $finaleUrl .= '&pv=2';
+            }
+        }
+        else {
+            if(array_key_exists('query', $urlToken) && $urlToken['query']) {
+                $finaleUrl .= '?'.$urlToken['query'];
+            }
+        }
+
+        if(array_key_exists('fragment', $urlToken) && $urlToken['fragment']) {
+            $finaleUrl .= '#'.$urlToken['fragment'];
+        }
+
+        return $finaleUrl;
+    }
+
+    private function shouldRedirectToHTTP(HTTPRequest $request) {
+        return $this->SSLIsNotMandatory() && $this->userAskedForHTTP($request);
+    }
+
+    private function SSLIsNotMandatory() {
+        return ! $GLOBALS['sys_force_ssl'];
+    }
+
+    private function userAskedForHTTP(HTTPRequest $request) {
+        return ! ($request->existAndNonEmpty('stay_in_ssl') && $request->get('stay_in_ssl'));
+    }
 }
 
 ?>
diff --git a/src/common/include/URLVerification.class.php b/src/common/include/URLVerification.class.php
index d8c3dac..ecc09cb 100644
--- a/src/common/include/URLVerification.class.php
+++ b/src/common/include/URLVerification.class.php
@@ -567,10 +567,8 @@ class URLVerification {
      * @return void
      */
     function header($location) {
-
         header('Location: '.$location);
         exit;
-
     }
 
 }
diff --git a/src/common/user/LoginPresenter.class.php b/src/common/user/LoginPresenter.class.php
index 7dc902d..d55e9bf 100644
--- a/src/common/user/LoginPresenter.class.php
+++ b/src/common/user/LoginPresenter.class.php
@@ -54,7 +54,6 @@ class User_LoginPresenter {
     }
 
     public function toggle_ssl() {
-        return true;
         return $this->toggle_ssl;
     }
 
diff --git a/src/www/include/session.php b/src/www/include/session.php
index a166edb..35767fa 100644
--- a/src/www/include/session.php
+++ b/src/www/include/session.php
@@ -11,7 +11,7 @@
 $G_SESSION=array();
 
 function session_issecure() {
-	return (getenv('HTTPS') == 'on');
+    return HTTPRequest::instance()->isSSL();
 }
 
 function session_make_url($loc) {
diff --git a/src/www/include/utils.php b/src/www/include/utils.php
index 9782936..52b2442 100644
--- a/src/www/include/utils.php
+++ b/src/www/include/utils.php
@@ -1306,66 +1306,10 @@ function util_translate_desc_ugroup($desc) {
     return util_translate($desc, "ugroup_", "_desc_key", "project_ugroup");
 }
 
-function util_make_return_to_url($url) {
-    $request = HTTPRequest::instance();
-
-    $urlToken = parse_url($url);
-
-    $finaleUrl = '';
-
-    if(array_key_exists('host', $urlToken) && $urlToken['host']) {
-        $server_url = $urlToken['scheme'].'://'.$urlToken['host'];
-        if(array_key_exists('port', $urlToken) && $urlToken['port']) {
-            $server_url .= ':'.$urlToken['port'];
-        }
-    }
-    else {
-        if (session_issecure()
-            && ($GLOBALS['sys_force_ssl']
-                || (isset($_REQUEST['stay_in_ssl']) && $_REQUEST['stay_in_ssl'])
-                )) {
-            $server_url = 'https://'.$GLOBALS['sys_https_host'];
-        }
-        else {
-            $server_url = 'http://'.$GLOBALS['sys_default_domain'];
-        }
-    }
-
-    $finaleUrl = $server_url;
-
-    if(array_key_exists('path', $urlToken) && $urlToken['path']) {
-        $finaleUrl .= $urlToken['path'];
-    }
-    
-    if($request->existAndNonEmpty('return_to')) {
-        $rt = 'return_to='.urlencode($request->get('return_to'));
-    
-        if(array_key_exists('query', $urlToken) && $urlToken['query']) {
-            $finaleUrl .= '?'.$urlToken['query'].'&amp;'.$rt;
-        }
-        else {
-            $finaleUrl .= '?'.$rt;
-        }
-	if (strstr($_REQUEST['return_to'],'pv=2')) {
-	    $finaleUrl .= '&pv=2';
-	}	
-    }
-    else {
-        if(array_key_exists('query', $urlToken) && $urlToken['query']) {
-            $finaleUrl .= '?'.$urlToken['query'];
-        }
-    }
-
-    if(array_key_exists('fragment', $urlToken) && $urlToken['fragment']) {
-        $finaleUrl .= '#'.$urlToken['fragment'];
-    }
-
-    return $finaleUrl;
-}
-
 function util_return_to($url) {
-    $finaleUrl = util_make_return_to_url($url);
-    $GLOBALS['Response']->redirect($finaleUrl);
+    $request      = HTTPRequest::instance();
+    $url_redirect = new URLRedirect();
+    $GLOBALS['Response']->redirect($url_redirect->makeReturnToUrl($request, $url));
     exit;
 }
 
diff --git a/src/www/my/redirect.php b/src/www/my/redirect.php
index 54d85c2..5c29322 100644
--- a/src/www/my/redirect.php
+++ b/src/www/my/redirect.php
@@ -1,5 +1,6 @@
 <?php
 /**
+ * Copyright (c) Enalean, 2014. All Rights Reserved.
  * Copyright (c) STMicroelectronics, 2005. All Rights Reserved.
  *
  * Originally written by Manuel Vacelet, 2005
@@ -49,14 +50,6 @@ if($request->valid($vReturnTo)) {
 
     $return_to = trim($request->get('return_to'));
 
-    $use_ssl = session_issecure() || $GLOBALS['sys_force_ssl'];
-    
-    if ($use_ssl) {
-        $server_url = "https://".$GLOBALS['sys_https_host'];
-    } else {
-        $server_url = "http://".$GLOBALS['sys_default_domain'];
-    }
-    
     if (preg_match("/^[A-Za-z]+:\/\//i", $return_to)) {
         $return_url = $return_to;
     } else {	
@@ -84,6 +77,6 @@ else {
 
 <p><big><?= $redirect; ?></big></p>
 
-<?
+<?php
 ($pv == 2) ? $HTML->pv_footer(array()) : site_footer(array());
 ?>
\ No newline at end of file
diff --git a/tests/simpletest/common/include/URLRedirectTest.php b/tests/simpletest/common/include/URLRedirectTest.php
new file mode 100644
index 0000000..9e328fc
--- /dev/null
+++ b/tests/simpletest/common/include/URLRedirectTest.php
@@ -0,0 +1,84 @@
+<?php
+/**
+ * Copyright (c) Enalean, 2014. All Rights Reserved.
+ *
+ * This file is a part of Tuleap.
+ *
+ * Tuleap is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Tuleap 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 the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Tuleap. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+class URLRedirect_MakeUrlTest extends TuleapTestCase {
+    private $request;
+    private $url;
+
+    public function setUp() {
+        $this->url     = new URLRedirect();
+        $this->request = mock('HTTPRequest');
+        $GLOBALS['sys_force_ssl'] = 1;
+        $GLOBALS['sys_https_host'] = 'example.com';
+        $GLOBALS['sys_default_domain'] = 'example.com';
+        parent::setUp();
+    }
+
+    public function tearDown() {
+        unset($GLOBALS['sys_force_ssl']);
+        unset($GLOBALS['sys_https_host']);
+        unset($GLOBALS['sys_default_domain']);
+        parent::tearDown();
+    }
+
+    public function itStayInSSLWhenForceSSLIsOn() {
+        stub($this->request)->isSSL()->returns(true);
+        $GLOBALS['sys_force_ssl'] = 1;
+
+        $this->assertEqual('/my/index.php', $this->url->makeReturnToUrl($this->request, '/my/index.php'));
+    }
+
+    public function itRedirectToHttpWhenForceSSLIsOffAndNoStayInSSL() {
+        stub($this->request)->isSSL()->returns(true);
+        $GLOBALS['sys_force_ssl'] = 0;
+        stub($this->request)->existAndNonEmpty('stay_in_ssl')->returns(true);
+        stub($this->request)->get('stay_in_ssl')->returns(0);
+
+        $this->assertEqual('http://example.com/my/index.php', $this->url->makeReturnToUrl($this->request, '/my/index.php'));
+    }
+
+    public function itRedirectToHttpWhenForceSSLIsOffAndNoStayInSSL2() {
+        stub($this->request)->isSSL()->returns(true);
+        $GLOBALS['sys_force_ssl'] = 0;
+        stub($this->request)->existAndNonEmpty('stay_in_ssl')->returns(false);
+        stub($this->request)->get('stay_in_ssl')->returns(false);
+
+        $this->assertEqual('http://example.com/my/index.php', $this->url->makeReturnToUrl($this->request, '/my/index.php'));
+    }
+
+
+    public function itStayInSSLWhenForceSSLIsOffAndNoStayInSSL() {
+        stub($this->request)->isSSL()->returns(true);
+        $GLOBALS['sys_force_ssl'] = 0;
+        stub($this->request)->existAndNonEmpty('stay_in_ssl')->returns(true);
+        stub($this->request)->get('stay_in_ssl')->returns(1);
+
+        $this->assertEqual('/my/index.php', $this->url->makeReturnToUrl($this->request, '/my/index.php'));
+    }
+
+    public function itStayUnencryptedWhenForceSSLIsOffAndNoStayInSSL() {
+        stub($this->request)->isSSL()->returns(false);
+        $GLOBALS['sys_force_ssl'] = 0;
+        stub($this->request)->existAndNonEmpty('stay_in_ssl')->returns(true);
+        stub($this->request)->get('stay_in_ssl')->returns(0);
+
+        $this->assertEqual('/my/index.php', $this->url->makeReturnToUrl($this->request, '/my/index.php'));
+    }
+}
\ No newline at end of file
diff --git a/tests/simpletest/common/include/URLTest.php b/tests/simpletest/common/include/URLTest.php
index 7d5e471..cde9cab 100644
--- a/tests/simpletest/common/include/URLTest.php
+++ b/tests/simpletest/common/include/URLTest.php
@@ -257,4 +257,5 @@ class URLTest extends UnitTestCase {
     }
 
 }
+
 ?>
diff --git a/tests/simpletest/common/include/URLVerificationTest.php b/tests/simpletest/common/include/URLVerificationTest.php
index 85a330e..6cfdd5d 100644
--- a/tests/simpletest/common/include/URLVerificationTest.php
+++ b/tests/simpletest/common/include/URLVerificationTest.php
@@ -71,6 +71,14 @@ class URLVerificationTest extends TuleapTestCase {
 
     function setUp() {
         parent::setUp();
+
+        $GLOBALS['Response']           = mock('Layout');
+        $GLOBALS['sys_allow_anon']     = 1;
+        $GLOBALS['sys_default_domain'] = 'default';
+        $GLOBALS['sys_https_host']     = 'default';
+        $GLOBALS['sys_force_ssl']      = 0;
+        unset($GLOBALS['group_id']);
+
         $this->fixtures = dirname(__FILE__).'/_fixtures';
         $GLOBALS['Language'] = new MockBaseLanguage($this);
 
@@ -84,10 +92,11 @@ class URLVerificationTest extends TuleapTestCase {
     function tearDown() {
         UserManager::clearInstance();
         unset($GLOBALS['Language']);
-        $GLOBALS['sys_allow_anon'] = 1;
-        $GLOBALS['sys_default_domain'] = 1;
-        $GLOBALS['sys_force_ssl'] = 1;
-        $GLOBALS['sys_https_host'] = 1;
+        unset($GLOBALS['Response']);
+        unset($GLOBALS['sys_allow_anon']);
+        unset($GLOBALS['sys_default_domain']);
+        unset($GLOBALS['sys_force_ssl']);
+        unset($GLOBALS['sys_https_host']);
         unset($GLOBALS['group_id']);
         unset($_REQUEST['type_of_search']);
         parent::tearDown();
-- 
1.7.9.5

