eliseuamaro/gnu-social
1
0
Fork 0
forked from GNUsocial/gnu-social
Code Issues Pull Requests Releases Wiki Activity

[TESTS] Fix SecurityTest

Browse Source 
This test was broken by changes in the routing and in the templates.
However, this revealead a potential open redirect and duplicated code
in the Reply and Favourite plugins
This commit is contained in:
Hugo Sales 2021-11-11 12:24:45 +00:00
parent dea9aa4dcf
commit f667b558f7
Signed by untrusted user: someonewithpc
GPG Key ID: 7D0C7EAFC9D835A0
4 changed files with 42 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
plugins/Favourite/Favourite.php
View File

@ -37,7 +37,6 @@ use App\Util\Exception\NotFoundException;
use App\Util\Exception\RedirectException; use App\Util\Exception\RedirectException;
use App\Util\Formatting; use App\Util\Formatting;
use App\Util\Nickname; use App\Util\Nickname;
use phpDocumentor\Reflection\PseudoTypes\NumericString;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
class Favourite extends NoteHandlerPlugin class Favourite extends NoteHandlerPlugin
@ -54,29 +53,30 @@ class Favourite extends NoteHandlerPlugin
*/ */
public function onAddNoteActions(Request $request, Note $note, array &$actions): bool public function onAddNoteActions(Request $request, Note $note, array &$actions): bool
{ {
if (is_null($user = Common::user())) { if (\is_null($user = Common::user())) {
return Event::next; return Event::next;
} }
// If note is favourite, "is_favourite" is 1 // If note is favourite, "is_favourite" is 1
$opts = ['note_id' => $note->getId(), 'actor_id' => $user->getId()]; $opts = ['note_id' => $note->getId(), 'actor_id' => $user->getId()];
$is_favourite = DB::find('favourite', $opts) !== null; $is_favourite = DB::find('favourite', $opts) !== null;
// Generating URL for favourite action route // Generating URL for favourite action route
$args = ['id' => $note->getId()]; $args = ['id' => $note->getId()];
$type = Router::ABSOLUTE_PATH; $type = Router::ABSOLUTE_PATH;
$favourite_action_url = $is_favourite ? $favourite_action_url = $is_favourite
Router::url('favourite_remove', $args, $type) : ? Router::url('favourite_remove', $args, $type)
Router::url('favourite_add', $args, $type); : Router::url('favourite_add', $args, $type);
$query_string = $request->getQueryString();
// Concatenating get parameter to redirect the user to where he came from // Concatenating get parameter to redirect the user to where he came from
$favourite_action_url .= '?from=' . substr($request->getQueryString(), 2); $favourite_action_url .= !\is_null($query_string) ? '?from=' . mb_substr($query_string, 2) : '';
$extra_classes = $is_favourite ? "note-actions-set" : "note-actions-unset"; $extra_classes = $is_favourite ? 'note-actions-set' : 'note-actions-unset';
$favourite_action = [ $favourite_action = [
"url" => $favourite_action_url, 'url' => $favourite_action_url,
"classes" => "button-container favourite-button-container $extra_classes", 'classes' => "button-container favourite-button-container {$extra_classes}",
"id" => "favourite-button-container-" . $note->getId() 'id' => 'favourite-button-container-' . $note->getId(),
]; ];
$actions[] = $favourite_action; $actions[] = $favourite_action;

34
plugins/Repeat/Repeat.php
View File

@ -23,23 +23,21 @@ namespace Plugin\Repeat;
use App\Core\DB\DB; use App\Core\DB\DB;
use App\Core\Event; use App\Core\Event;
use App\Core\Modules\NoteHandlerPlugin;
use App\Core\Router\RouteLoader; use App\Core\Router\RouteLoader;
use App\Core\Router\Router; use App\Core\Router\Router;
use App\Entity\Actor; use App\Entity\Actor;
use App\Entity\Note;
use App\Util\Common;
use App\Util\Exception\DuplicateFoundException; use App\Util\Exception\DuplicateFoundException;
use App\Util\Exception\InvalidFormException; use App\Util\Exception\InvalidFormException;
use App\Util\Exception\NoSuchNoteException; use App\Util\Exception\NoSuchNoteException;
use App\Core\Modules\NoteHandlerPlugin;
use App\Entity\Note;
use App\Util\Common;
use App\Util\Exception\NotFoundException; use App\Util\Exception\NotFoundException;
use App\Util\Exception\RedirectException; use App\Util\Exception\RedirectException;
use App\Util\Formatting;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
class Repeat extends NoteHandlerPlugin class Repeat extends NoteHandlerPlugin
{ {
/** /**
* HTML rendering event that adds the repeat form as a note * HTML rendering event that adds the repeat form as a note
* action, if a user is logged in * action, if a user is logged in
@ -52,7 +50,7 @@ class Repeat extends NoteHandlerPlugin
*/ */
public function onAddNoteActions(Request $request, Note $note, array &$actions): bool public function onAddNoteActions(Request $request, Note $note, array &$actions): bool
{ {
if (is_null($user = Common::user())) { if (\is_null($user = Common::user())) {
return Event::next; return Event::next;
} }
@ -68,22 +66,24 @@ class Repeat extends NoteHandlerPlugin
$is_repeat = DB::count('note_repeat', ['id' => $note->getId()]) >= 1; $is_repeat = DB::count('note_repeat', ['id' => $note->getId()]) >= 1;
// Generating URL for repeat action route // Generating URL for repeat action route
$args = ['id' => $note->getId()]; $args = ['id' => $note->getId()];
$type = Router::ABSOLUTE_PATH; $type = Router::ABSOLUTE_PATH;
$repeat_action_url = $is_repeat ? $repeat_action_url = $is_repeat
Router::url('repeat_remove', $args, $type) : ? Router::url('repeat_remove', $args, $type)
Router::url('repeat_add', $args, $type); : Router::url('repeat_add', $args, $type);
// TODO clean this up
// SECURITY: open redirect?
$query_string = $request->getQueryString();
// Concatenating get parameter to redirect the user to where he came from // Concatenating get parameter to redirect the user to where he came from
$repeat_action_url .= '?from=' . substr($request->getQueryString(), 2); $repeat_action_url .= !\is_null($query_string) ? '?from=' . mb_substr($query_string, 2) : '';
$extra_classes = $is_repeat ? "note-actions-set" : "note-actions-unset"; $extra_classes = $is_repeat ? 'note-actions-set' : 'note-actions-unset';
$repeat_action = [ $repeat_action = [
"url" => $repeat_action_url, 'url' => $repeat_action_url,
"classes" => "button-container repeat-button-container $extra_classes", 'classes' => "button-container repeat-button-container {$extra_classes}",
"id" => "repeat-button-container-" . $note->getId() 'id' => 'repeat-button-container-' . $note->getId(),
]; ];
$actions[] = $repeat_action; $actions[] = $repeat_action;

6
src/Controller/Security.php
View File

@ -12,9 +12,9 @@ use function App\Core\I18n\_m;
use App\Core\Log; use App\Core\Log;
use App\Core\VisibilityScope; use App\Core\VisibilityScope;
use App\Entity\Actor; use App\Entity\Actor;
use App\Entity\Subscription;
use App\Entity\LocalUser; use App\Entity\LocalUser;
use App\Entity\Note; use App\Entity\Note;
use App\Entity\Subscription;
use App\Security\Authenticator; use App\Security\Authenticator;
use App\Security\EmailVerifier; use App\Security\EmailVerifier;
use App\Util\Common; use App\Util\Common;
@ -61,7 +61,6 @@ class Security extends Controller
'_template' => 'security/login.html.twig', '_template' => 'security/login.html.twig',
'last_login_id' => $last_login_id, 'last_login_id' => $last_login_id,
'error' => $error, 'error' => $error,
'notes_fn' => fn () => Note::getAllNotes(VisibilityScope::$instance_scope),
]; ];
} }
@ -92,8 +91,7 @@ class Security extends Controller
Request $request, Request $request,
GuardAuthenticatorHandler $guard_handler, GuardAuthenticatorHandler $guard_handler,
Authenticator $authenticator, Authenticator $authenticator,
): array|Response|null ): array|Response|null {
{
$form = Form::create([ $form = Form::create([
['nickname', TextType::class, [ ['nickname', TextType::class, [
'label' => _m('Nickname'), 'label' => _m('Nickname'),

18
tests/Controller/SecurityTest.php
View File

@ -33,12 +33,12 @@ class SecurityTest extends GNUsocialTestCase
{ {
// This calls static::bootKernel(), and creates a "client" that is acting as the browser // This calls static::bootKernel(), and creates a "client" that is acting as the browser
$client = static::createClient(); $client = static::createClient();
$crawler = $client->request('GET', '/login'); $crawler = $client->request('GET', '/main/login');
$this->assertResponseIsSuccessful(); $this->assertResponseIsSuccessful();
// $form = $crawler->selectButton('Sign in')->form(); // $form = $crawler->selectButton('Sign in')->form();
$crawler = $client->submitForm('Sign in', [ $crawler = $client->submitForm('Sign in', [
'nickname' => $nickname, 'nickname_or_email' => $nickname,
'password' => $password, 'password' => $password,
]); ]);
$this->assertResponseStatusCodeSame(302); $this->assertResponseStatusCodeSame(302);
$crawler = $client->followRedirect(); $crawler = $client->followRedirect();
@ -51,13 +51,13 @@ class SecurityTest extends GNUsocialTestCase
$this->assertResponseIsSuccessful(); $this->assertResponseIsSuccessful();
$this->assertSelectorNotExists('.alert'); $this->assertSelectorNotExists('.alert');
$this->assertRouteSame('main_all'); $this->assertRouteSame('main_all');
$this->assertSelectorTextContains('#user-nickname', $nickname); $this->assertSelectorTextContains('.profile-info .profile-info-nickname', $nickname);
} }
public function testLoginAttemptAlreadyLoggedIn() public function testLoginAttemptAlreadyLoggedIn()
{ {
[$client] = self::testLogin('taken_user', 'foobar'); // Normal login [$client] = self::testLogin('taken_user', 'foobar'); // Normal login
$crawler = $client->request('GET', '/login'); // attempt to login again $crawler = $client->request('GET', '/main/login'); // attempt to login again
$client->followRedirect(); $client->followRedirect();
$this->assertRouteSame('main_all'); $this->assertRouteSame('main_all');
} }
@ -77,7 +77,7 @@ class SecurityTest extends GNUsocialTestCase
$this->assertResponseIsSuccessful(); $this->assertResponseIsSuccessful();
$this->assertSelectorNotExists('.alert'); $this->assertSelectorNotExists('.alert');
$this->assertRouteSame('main_all'); $this->assertRouteSame('main_all');
$this->assertSelectorTextContains('#user-nickname', 'taken_user'); $this->assertSelectorTextContains('.profile-info .profile-info-nickname', 'taken_user');
} }
// --------- Register -------------- // --------- Register --------------
@ -85,7 +85,7 @@ class SecurityTest extends GNUsocialTestCase
private function testRegister(string $nickname, string $email, string $password) private function testRegister(string $nickname, string $email, string $password)
{ {
$client = static::createClient(); $client = static::createClient();
$crawler = $client->request('GET', '/register'); $crawler = $client->request('GET', '/main/register');
$this->assertResponseIsSuccessful(); $this->assertResponseIsSuccessful();
$crawler = $client->submitForm('Register', [ $crawler = $client->submitForm('Register', [
'register[nickname]' => $nickname, 'register[nickname]' => $nickname,
@ -104,13 +104,13 @@ class SecurityTest extends GNUsocialTestCase
$this->assertResponseIsSuccessful(); $this->assertResponseIsSuccessful();
$this->assertSelectorNotExists('.alert'); $this->assertSelectorNotExists('.alert');
$this->assertRouteSame('main_all'); $this->assertRouteSame('main_all');
$this->assertSelectorTextContains('#user-nickname', 'new_nickname'); $this->assertSelectorTextContains('.profile-info .profile-info-nickname', 'new_nickname');
} }
public function testRegisterDifferentPassword() public function testRegisterDifferentPassword()
{ {
$client = static::createClient(); $client = static::createClient();
$crawler = $client->request('GET', '/register'); $crawler = $client->request('GET', '/main/register');
$this->assertResponseIsSuccessful(); $this->assertResponseIsSuccessful();
$crawler = $client->submitForm('Register', [ $crawler = $client->submitForm('Register', [
'register[nickname]' => 'new_user', 'register[nickname]' => 'new_user',