From 7b428641f04766ea8711cb47e76bbe2b52638abe Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 14 Nov 2019 21:32:50 +0100 Subject: [PATCH 1/3] [ticket/16211] Prevent skipping COPPA via URL parameter PHPBB3-16211 --- phpBB/includes/ucp/ucp_register.php | 12 +++++- tests/functional/registration_test.php | 55 ++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index 54e418d58c..29829c2e68 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -40,6 +40,7 @@ class ucp_register } $coppa = $request->is_set('coppa') ? (int) $request->variable('coppa', false) : false; + $token = $request->variable('hash', ''); $agreed = $request->variable('agreed', false); $submit = $request->is_set_post('submit'); $change_lang = $request->variable('change_lang', ''); @@ -50,6 +51,11 @@ class ucp_register $agreed = false; } + if ($coppa !== false && !check_link_hash($token, 'coppa') && !check_form_key('ucp_register')) + { + $coppa = false; + } + /** * Add UCP register data before they are assigned to the template or submitted * @@ -164,13 +170,15 @@ class ucp_register ->format($user->lang['DATE_FORMAT'], true); unset($now); + $coppa_link_hash = '&hash=' . generate_link_hash('coppa'); + $template_vars = array( 'S_LANG_OPTIONS' => (count($lang_row) > 1) ? language_select($user_lang) : '', 'L_COPPA_NO' => sprintf($user->lang['UCP_COPPA_BEFORE'], $coppa_birthday), 'L_COPPA_YES' => sprintf($user->lang['UCP_COPPA_ON_AFTER'], $coppa_birthday), - 'U_COPPA_NO' => append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=register&coppa=0'), - 'U_COPPA_YES' => append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=register&coppa=1'), + 'U_COPPA_NO' => append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=register&coppa=0' . $coppa_link_hash), + 'U_COPPA_YES' => append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=register&coppa=1' . $coppa_link_hash), 'S_SHOW_COPPA' => true, 'S_HIDDEN_FIELDS' => build_hidden_fields($s_hidden_fields), diff --git a/tests/functional/registration_test.php b/tests/functional/registration_test.php index 690f4ae9f2..be6dd50203 100644 --- a/tests/functional/registration_test.php +++ b/tests/functional/registration_test.php @@ -36,6 +36,10 @@ class phpbb_functional_registration_test extends phpbb_functional_test_case { $this->add_lang('ucp'); + // Check that we can't skip + self::request('GET', 'ucp.php?mode=register&agreed=1'); + $this->assertContainsLang('AGREE', $this->get_content()); + $crawler = self::request('GET', 'ucp.php?mode=register'); $this->assertContainsLang('REGISTRATION', $crawler->filter('div.content h2')->text()); @@ -64,4 +68,55 @@ class phpbb_functional_registration_test extends phpbb_functional_test_case $this->assert_checkbox_is_checked($crawler, 'notification.type.post_notification.method.email'); $this->assert_checkbox_is_checked($crawler, 'notification.type.topic_notification.method.email'); } + + /** + * @depends test_disable_captcha_on_registration + */ + public function test_register_coppa_account() + { + $this->login(); + $this->admin_login(); + + $crawler = self::request('GET', "adm/index.php?i=acp_board&mode=registration&sid={$this->sid}"); + $form = $crawler->selectButton('Submit')->form(); + $form['config[coppa_enable]']->setValue('1'); + $crawler = self::submit($form); + + $this->assertContainsLang('CONFIG_UPDATED', $crawler->filter('#main .successbox')->text()); + $this->logout(); + + $this->add_lang('ucp'); + + // Check that we can't skip + $crawler = self::request('GET', 'ucp.php?mode=register&coppa=1'); + $this->assertContainsLang('COPPA_BIRTHDAY', $crawler->html()); + + $agreement_url = $crawler->filter('#agreement')->filter('a')->links()[0]->getUri(); + preg_match('/(&hash=\w+)/', $agreement_url, $matches); + $crawler = self::request('GET', 'ucp.php?mode=register&coppa=1' . $matches[1]); + + $this->assertContainsLang('REGISTRATION', $crawler->filter('div.content h2')->text()); + + $form = $crawler->selectButton('I agree to these terms')->form(); + $crawler = self::submit($form); + + $form = $crawler->selectButton('Submit')->form(array( + 'username' => 'user-coppa-test', + 'email' => 'user-coppa-test@phpbb.com', + 'new_password' => 'user-coppa-testuser-coppa-test', + 'password_confirm' => 'user-coppa-testuser-coppa-test', + )); + $form['tz']->select('Europe/Berlin'); + $crawler = self::submit($form); + + $this->assertContainsLang('ACCOUNT_COPPA', $crawler->filter('#message')->text()); + + $this->login(); + $this->admin_login(); + + $crawler = self::request('GET', "adm/index.php?i=acp_board&mode=registration&sid={$this->sid}"); + $form = $crawler->selectButton('Submit')->form(); + $form['config[coppa_enable]']->setValue('0'); + $crawler = self::submit($form); + } } From 417271f5738b0a73bc3dba28f516675ca3d146ea Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 27 Nov 2019 22:01:26 +0100 Subject: [PATCH 2/3] [ticket/16211] Use form to ensure link data is passed on coppa registration PHPBB3-16211 --- phpBB/includes/ucp/ucp_register.php | 15 +++++---------- .../styles/prosilver/template/ucp_agreement.html | 3 ++- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index 29829c2e68..03ac63b12b 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -39,8 +39,8 @@ class ucp_register trigger_error('UCP_REGISTER_DISABLE'); } - $coppa = $request->is_set('coppa') ? (int) $request->variable('coppa', false) : false; - $token = $request->variable('hash', ''); + $coppa = $request->is_set('coppa_yes') ? 1 : ($request->is_set('coppa_no') ? 0 : false); + $coppa = $request->is_set('coppa') ? $request->variable('coppa', 0) : $coppa; $agreed = $request->variable('agreed', false); $submit = $request->is_set_post('submit'); $change_lang = $request->variable('change_lang', ''); @@ -51,7 +51,7 @@ class ucp_register $agreed = false; } - if ($coppa !== false && !check_link_hash($token, 'coppa') && !check_form_key('ucp_register')) + if ($coppa !== false && !check_form_key('ucp_register')) { $coppa = false; } @@ -170,15 +170,10 @@ class ucp_register ->format($user->lang['DATE_FORMAT'], true); unset($now); - $coppa_link_hash = '&hash=' . generate_link_hash('coppa'); - $template_vars = array( 'S_LANG_OPTIONS' => (count($lang_row) > 1) ? language_select($user_lang) : '', - 'L_COPPA_NO' => sprintf($user->lang['UCP_COPPA_BEFORE'], $coppa_birthday), - 'L_COPPA_YES' => sprintf($user->lang['UCP_COPPA_ON_AFTER'], $coppa_birthday), - - 'U_COPPA_NO' => append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=register&coppa=0' . $coppa_link_hash), - 'U_COPPA_YES' => append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=register&coppa=1' . $coppa_link_hash), + 'L_COPPA_NO' => $user->lang('UCP_COPPA_BEFORE', $coppa_birthday), + 'L_COPPA_YES' => $user->lang('UCP_COPPA_ON_AFTER', $coppa_birthday), 'S_SHOW_COPPA' => true, 'S_HIDDEN_FIELDS' => build_hidden_fields($s_hidden_fields), diff --git a/phpBB/styles/prosilver/template/ucp_agreement.html b/phpBB/styles/prosilver/template/ucp_agreement.html index d4fef9f0a5..7959925d30 100644 --- a/phpBB/styles/prosilver/template/ucp_agreement.html +++ b/phpBB/styles/prosilver/template/ucp_agreement.html @@ -43,7 +43,8 @@
- {L_COPPA_NO}  {L_COPPA_YES} + +   From c7ed162a0627a0cfcf000631f236fbb7f6722ba3 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 28 Nov 2019 07:39:51 +0100 Subject: [PATCH 3/3] [ticket/16211] Fix coppa registration test PHPBB3-16211 --- tests/functional/registration_test.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/functional/registration_test.php b/tests/functional/registration_test.php index be6dd50203..48982edc8c 100644 --- a/tests/functional/registration_test.php +++ b/tests/functional/registration_test.php @@ -91,9 +91,8 @@ class phpbb_functional_registration_test extends phpbb_functional_test_case $crawler = self::request('GET', 'ucp.php?mode=register&coppa=1'); $this->assertContainsLang('COPPA_BIRTHDAY', $crawler->html()); - $agreement_url = $crawler->filter('#agreement')->filter('a')->links()[0]->getUri(); - preg_match('/(&hash=\w+)/', $agreement_url, $matches); - $crawler = self::request('GET', 'ucp.php?mode=register&coppa=1' . $matches[1]); + $form = $crawler->selectButton('coppa_yes')->form(); + $crawler = self::submit($form); $this->assertContainsLang('REGISTRATION', $crawler->filter('div.content h2')->text());