Merge pull request #2104 from nickvergessen/ticket/12261

[ticket/12261] Remove web root path from login redirect url

phpBB/includes/functions.php

@@ -2344,7 +2344,7 @@ function reapply_sid($url)
 */
 function build_url($strip_vars = false)
 {
-	global $user, $phpbb_root_path;
+	global $config, $user, $phpEx, $phpbb_root_path;
 
 	$page = $user->page['page'];
 
@@ -2357,6 +2357,12 @@ function build_url($strip_vars = false)
 	// URL
 	if ($url_parts === false || empty($url_parts['scheme']) || empty($url_parts['host']))
 	{
+		// Remove 'app.php/' from the page, when rewrite is enabled
+		if ($config['enable_mod_rewrite'] && strpos($page, 'app.' . $phpEx . '/') === 0)
+		{
+			$page = substr($page, strlen('app.' . $phpEx . '/'));
+		}
+
 		$page = $phpbb_root_path . $page;
 	}
 
@@ -4902,7 +4908,7 @@ function page_header($page_title = '', $display_online_list = true, $item_id = 0
 		'S_TOPIC_ID'			=> $topic_id,
 
 		'S_LOGIN_ACTION'		=> ((!defined('ADMIN_START')) ? append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=login') : append_sid("{$phpbb_admin_path}index.$phpEx", false, true, $user->session_id)),
-		'S_LOGIN_REDIRECT'		=> build_hidden_fields(array('redirect' => build_url())),
+		'S_LOGIN_REDIRECT'		=> build_hidden_fields(array('redirect' => $phpbb_path_helper->remove_web_root_path(build_url()))),
 
 		'S_ENABLE_FEEDS'			=> ($config['feed_enable']) ? true : false,
 		'S_ENABLE_FEEDS_OVERALL'	=> ($config['feed_overall']) ? true : false,

tests/functional/extension_controller_test.php

@@ -112,12 +112,33 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c
 		$this->assertContains('No route found for "GET /does/not/exist"', $crawler->filter('body')->text());
 	}
 
+	/**
+	 * Check the redirect after using the login_box() form
+	 */
+	public function test_login_redirect()
+	{
+		$this->markTestIncomplete('Session table contains incorrect data for controllers on travis,'
+			. 'therefor the redirect fails.');
+
+		$this->phpbb_extension_manager->enable('foo/bar');
+		$crawler = self::request('GET', 'app.php/foo/login_redirect');
+		$this->assertContainsLang('LOGIN', $crawler->filter('h2')->text());
+		$form = $crawler->selectButton('login')->form(array(
+			'username'	=> 'admin',
+			'password'	=> 'adminadmin',
+		));
+		$this->assertStringStartsWith('./app.php/foo/login_redirect', $form->get('redirect')->getValue());
+
+		$crawler = self::submit($form);
+		$this->assertContains("I am a variable", $crawler->filter('#content')->text(), 'Unsuccessful redirect after using login_box()');
+		$this->phpbb_extension_manager->purge('foo/bar');
+	}
+
 	/**
 	* Check the output of a controller using the template system
 	*/
 	public function test_redirect()
 	{
-		$filesystem = new \phpbb\filesystem();
 		$this->phpbb_extension_manager->enable('foo/bar');
 		$crawler = self::request('GET', 'app.php/foo/redirect');
 

tests/functional/fixtures/ext/foo/bar/config/routing.yml

@@ -14,6 +14,10 @@ foo_exception_controller:
     pattern: /foo/exception
     defaults: { _controller: foo_bar.controller:exception }
 
+foo_login_redirect_controller:
+    pattern: /foo/login_redirect
+    defaults: { _controller: foo_bar.controller:login_redirect }
+
 foo_redirect_controller:
     pattern: /foo/redirect
     defaults: { _controller: foo_bar.controller:redirect }

tests/functional/fixtures/ext/foo/bar/config/services.yml

@@ -6,6 +6,7 @@ services:
             - @path_helper
             - @template
             - @config
+            - @user
             - %core.root_path%
             - %core.php_ext%
 

tests/functional/fixtures/ext/foo/bar/controller/controller.php

@@ -10,13 +10,15 @@ class controller
 	protected $helper;
 	protected $path_helper;
 	protected $config;
+	protected $user;
 
-	public function __construct(\phpbb\controller\helper $helper, \phpbb\path_helper $path_helper, \phpbb\template\template $template, \phpbb\config\config $config, $root_path, $php_ext)
+	public function __construct(\phpbb\controller\helper $helper, \phpbb\path_helper $path_helper, \phpbb\template\template $template, \phpbb\config\config $config, \phpbb\user $user, $root_path, $php_ext)
 	{
 		$this->template = $template;
 		$this->helper = $helper;
 		$this->path_helper = $path_helper;
 		$this->config = $config;
+		$this->user = $user;
 		$this->root_path = $root_path;
 		$this->php_ext = $php_ext;
 	}
@@ -43,6 +45,18 @@ class controller
 		throw new \phpbb\controller\exception('Exception thrown from foo/exception route');
 	}
 
+	public function login_redirect()
+	{
+		if (!$this->user->data['is_registered'])
+		{
+			login_box();
+		}
+
+		$this->template->assign_var('A_VARIABLE', 'I am a variable');
+
+		return $this->helper->render('foo_bar_body.html');
+	}
+
 	public function redirect()
 	{
 		$url_root = generate_board_url();