diff --git a/composer.json b/composer.json index 2295cb0..bfa966b 100644 --- a/composer.json +++ b/composer.json @@ -1,36 +1,37 @@ { "name": "battis/user-session", "description": "User session management for Slim Framework", - "license": "MIT", - "autoload": { - "psr-4": { - "Battis\\UserSession\\": "src" - } - }, - "autoload-dev": { - "psr-4": { - "Battis\\UserSession\\Tests\\": "tests" - } - }, "authors": [ { "name": "Seth Battis", "email": "seth@battis.net" } ], + "license": "MIT", "require": { "bryanjhv/slim-session": "^4.0", "slim/http": "^1.2", "slim/php-view": "^3.0", "slim/psr7": "^1.0" }, - "config": { - "sort-packages": true + "autoload": { + "psr-4": { + "Battis\\UserSession\\": "src" + } }, "require-dev": { "battis/data-utilities": "^1.1", + "ext-simplexml": "*", "php-di/php-di": "^6.4", "phpspec/prophecy-phpunit": "^2.0", "phpunit/phpunit": "^9.0" + }, + "autoload-dev": { + "psr-4": { + "Battis\\UserSession\\Tests\\": "tests" + } + }, + "config": { + "sort-packages": true } } diff --git a/phpunit.xml b/phpunit.xml index 93357a1..c1d7df2 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,5 +1,5 @@ - + src @@ -10,4 +10,10 @@ tests + + + + + + diff --git a/src/Actions/HandleLogin.php b/src/Actions/HandleLogin.php index 2d67972..96f709c 100644 --- a/src/Actions/HandleLogin.php +++ b/src/Actions/HandleLogin.php @@ -47,6 +47,8 @@ public function __invoke(ServerRequest $request, Response $response) return $this->renderer->render($response, "login.php", [ "message" => "bad credentials", "message_type" => "error", + "usernameFieldName" => $this->usernameFieldName, + "passwordFieldName" => $this->passwordFieldName ]); } } diff --git a/src/Manager.php b/src/Manager.php index ee7785f..6439a71 100644 --- a/src/Manager.php +++ b/src/Manager.php @@ -10,6 +10,11 @@ class Manager { + public const DEFAULT_LOGIN_PATH = '/auth/login'; + public const DEFAULT_REDIRECT = '/'; + + private const HEADER_LOCATION = 'Location'; + private const USER = "battis.userSession.manager.user"; private const REDIRECT = "battis.userSession.manager.redirect"; @@ -19,8 +24,8 @@ class Manager public function __construct( Session $session, - string $loginPath = "/auth/login", - string $defaultRedirect = "/" + string $loginPath = self::DEFAULT_LOGIN_PATH, + string $defaultRedirect = self::DEFAULT_REDIRECT ) { $this->session = $session; $this->loginPath = $loginPath; @@ -31,7 +36,7 @@ public function startUserLogin(RequestInterface $request): ResponseInterface { $this->session->set(self::REDIRECT, $request->getUri()->getPath()); $response = new Response(); - return $response->withHeader("Location", $this->loginPath)->withStatus(302); // using status 302 forces the redirect to use the GET method + return $response->withHeader(self::HEADER_LOCATION, $this->loginPath)->withStatus(302); // using status 302 forces the redirect to use the GET method } public function startUserSession( @@ -43,7 +48,7 @@ public function startUserSession( $redirect = $this->session->get(self::REDIRECT, $this->defaultRedirect); $this->session->delete(self::REDIRECT); $response = new Response(); - return $response->withHeader("Location", $redirect)->withStatus(302); + return $response->withHeader(self::HEADER_LOCATION, $redirect)->withStatus(302); } public function sessionIsActive(): bool @@ -58,6 +63,6 @@ public function getCurrentUser(): ?UserEntityInterface public function endUserSession(): void { - $this->session->destroy(); + $this->session->clear(); } } diff --git a/src/Middleware/RequireAuthentication.php b/src/Middleware/RequireAuthentication.php index fdf3f6c..4c76597 100644 --- a/src/Middleware/RequireAuthentication.php +++ b/src/Middleware/RequireAuthentication.php @@ -11,9 +11,10 @@ class RequireAuthentication extends Session { private $manager; - public function __construct($settings = [], Manager $manager) + public function __construct($settings = [], Manager $manager = null) { parent::__construct($settings); + assert($manager !== null); $this->manager = $manager; } diff --git a/templates/login.php b/templates/login.php index e40dddb..b6e2ecc 100644 --- a/templates/login.php +++ b/templates/login.php @@ -14,14 +14,14 @@
diff --git a/tests/Actions/HandleLoginTest.php b/tests/Actions/HandleLoginTest.php new file mode 100644 index 0000000..92c7c39 --- /dev/null +++ b/tests/Actions/HandleLoginTest.php @@ -0,0 +1,28 @@ +getAppInstance(); + + $user = new User(); + + /** @var UserRepository $userRepo */ + $userRepo = $app->getContainer()->get(UserRepositoryInterface::class); + $userRepo->addUser($user); + + /** #var Manager $manager */ + $manager = $app->getContainer()->get(Manager::class); + + $response = $app->handle($this->createRequest('POST', Manager::DEFAULT_LOGIN_PATH)); + } +} diff --git a/tests/Actions/ShowLoginViewTest.php b/tests/Actions/ShowLoginViewTest.php new file mode 100644 index 0000000..535c0d4 --- /dev/null +++ b/tests/Actions/ShowLoginViewTest.php @@ -0,0 +1,35 @@ +getAppInstance(); + $response = $app->handle($this->createRequest('GET', Manager::DEFAULT_LOGIN_PATH)); + $payload = (string) $response->getBody(); + $xml = new SimpleXMLElement($payload); + $this->assertNotEmpty($payload); + $this->assertCount(0, $xml->{'non-document-error'}); + $this->assertCount(0, $xml->error); + $this->assertStringContainsString('
', $payload); + } + + public function testLoginViewWithActiveuser() + { + $app = $this->getAppInstance(); + + /** @var Manager */ + $manager = $app->getContainer()->get(Manager::class); + $manager->startUserSession(new User()); + + $response = $app->handle($this->createRequest('GET', Manager::DEFAULT_LOGIN_PATH)); + $this->assertLocationHeader(Manager::DEFAULT_REDIRECT, $response); + } +} diff --git a/tests/Fixtures/ManagerTest/User.php b/tests/Fixtures/ManagerTest/User.php deleted file mode 100644 index 755e623..0000000 --- a/tests/Fixtures/ManagerTest/User.php +++ /dev/null @@ -1,29 +0,0 @@ -id = $id; - $this->password = $password; - $this->hash = password_hash($password, PASSWORD_DEFAULT); - } - - public function getIdentifier() - { - return $this->id; - } - - public function passwordVerify(string $password): bool - { - return password_verify($password, $this->hash); - } -} diff --git a/tests/Fixtures/Reusable/PageRenderer.php b/tests/Fixtures/Reusable/PageRenderer.php new file mode 100644 index 0000000..d245d77 --- /dev/null +++ b/tests/Fixtures/Reusable/PageRenderer.php @@ -0,0 +1,39 @@ +renderer = $renderer; + $this->manager = $manager; + } + + public function __invoke( + ServerRequestInterface $request, + ResponseInterface $response, + array $args = [] + ) { + return $this->renderer->render( + $response, + basename($request->getUri()->getPath()) . ".php", + array_merge($args, $request->getQueryParams(), [ + "user" => $this->manager->getCurrentUser(), + ]) + ); + } +} diff --git a/tests/Fixtures/ManagerTest/Session.php b/tests/Fixtures/Reusable/Session.php similarity index 88% rename from tests/Fixtures/ManagerTest/Session.php rename to tests/Fixtures/Reusable/Session.php index 4d13e55..05c94f0 100644 --- a/tests/Fixtures/ManagerTest/Session.php +++ b/tests/Fixtures/Reusable/Session.php @@ -1,6 +1,6 @@ id = $id ?? random_int(1, 1000); + $this->username = md5(time() . 'username'); + $this->password = $password ?? md5(time() . 'password'); + $this->hash = password_hash($this->password, PASSWORD_DEFAULT); + } + + public function getIdentifier() + { + return $this->id; + } + + public function passwordVerify(string $password): bool + { + return password_verify($password, $this->hash); + } +} diff --git a/tests/Fixtures/Reusable/UserRepository.php b/tests/Fixtures/Reusable/UserRepository.php new file mode 100644 index 0000000..40a4823 --- /dev/null +++ b/tests/Fixtures/Reusable/UserRepository.php @@ -0,0 +1,21 @@ +users[$user->username] = $user; + } + + public function getUserEntityByUsername(string $username): ?UserEntityInterface + { + return $this->users[$username] ?? null; + } +} diff --git a/tests/Fixtures/app/dependencies.inc.php b/tests/Fixtures/app/dependencies.inc.php new file mode 100644 index 0000000..5454603 --- /dev/null +++ b/tests/Fixtures/app/dependencies.inc.php @@ -0,0 +1,8 @@ + fn() => new UserRepository(), +]; diff --git a/tests/Fixtures/app/middleware.inc.php b/tests/Fixtures/app/middleware.inc.php new file mode 100644 index 0000000..1401d10 --- /dev/null +++ b/tests/Fixtures/app/middleware.inc.php @@ -0,0 +1,12 @@ +addBodyParsingMiddleware(); diff --git a/tests/Fixtures/app/routes.inc.php b/tests/Fixtures/app/routes.inc.php new file mode 100644 index 0000000..d2ff4fb --- /dev/null +++ b/tests/Fixtures/app/routes.inc.php @@ -0,0 +1,20 @@ +group(UserSession\Controller::ENDPOINT, UserSession\Controller::class); + +$app + ->get("/home", PageRenderer::class) + ->add(UserSession\Middleware\Session::class); +$app->redirect("/", "/home"); +$app + ->get("/protected", PageRenderer::class) + ->add(UserSession\Middleware\RequireAuthentication::class); diff --git a/tests/Fixtures/app/settings.inc.php b/tests/Fixtures/app/settings.inc.php new file mode 100644 index 0000000..0d43cd8 --- /dev/null +++ b/tests/Fixtures/app/settings.inc.php @@ -0,0 +1,7 @@ + realpath(__DIR__ . "/../../../templates"), +]; diff --git a/tests/ManagerTest.php b/tests/ManagerTest.php index e32469a..4f103ac 100644 --- a/tests/ManagerTest.php +++ b/tests/ManagerTest.php @@ -2,11 +2,9 @@ namespace Battis\UserSession\Tests; -use Battis\UserSession\Entities\UserEntityInterface; use Battis\UserSession\Manager; -use Battis\UserSession\Tests\Fixtures\ManagerTest\Session; -use Battis\UserSession\Tests\Fixtures\ManagerTest\User; -use Psr\Http\Message\ResponseInterface; +use Battis\UserSession\Tests\Fixtures\Reusable\Session; +use Battis\UserSession\Tests\Fixtures\Reusable\User; use ReflectionClass; class ManagerTest extends TestCase @@ -28,21 +26,11 @@ protected function setUp(): void $_SESSION = []; } - private function getUser(): UserEntityInterface - { - return new User(random_int(1,1000), md5(time())); - } - private function getManager(...$args): Manager { return new Manager(new Session(), ...$args); } - private function assertLocationHeader($expectedLocation, ResponseInterface $response) - { - $this->assertEquals(['Location' => [$expectedLocation]], $response->getHeaders()); - } - public function testConstructorDefault() { global $_SESSION; @@ -66,7 +54,7 @@ public function testConstructorCustomDefaultRedirect() { $redirect = 'custom-default-redirect'; $manager = $this->getManager('', $redirect); - $response = $manager->startUserSession($this->getUser()); + $response = $manager->startUserSession(new User()); $this->assertLocationHeader($redirect, $response); } @@ -77,7 +65,7 @@ public function testStartUserLogin() $manager = $this->getManager(); $request = $this->createRequest('GET', $path); $response = $manager->startUserLogin($request); - $this->assertLocationHeader('/auth/login', $response); + $this->assertLocationHeader(Manager::DEFAULT_LOGIN_PATH, $response); $this->assertEquals(302, $response->getStatusCode()); $this->assertFalse(isset($_SESSION[self::$USER])); $this->assertEquals($path, $_SESSION[self::$REDIRECT]); @@ -85,11 +73,11 @@ public function testStartUserLogin() public function testStartUserSession() { - $user = $this->getUser(); + $user = new User(); $manager = $this->getManager(); $response = $manager->startUserSession($user); - $this->assertLocationHeader('/', $response); + $this->assertLocationHeader(Manager::DEFAULT_REDIRECT, $response); $this->assertTrue($manager->sessionIsActive()); $this->assertEquals($user, $manager->getCurrentUser()); } @@ -97,7 +85,7 @@ public function testStartUserSession() public function testEndUserSession() { global $_SESSION; - $user = $this->getUser(); + $user = new User(); $manager = $this->getManager(); $manager->startUserSession($user); diff --git a/tests/TestCase.php b/tests/TestCase.php index e2ac811..d0dd067 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -7,9 +7,11 @@ use DI\ContainerBuilder; use PHPUnit\Framework\TestCase as PHPUnitTestCase; use Prophecy\PhpUnit\ProphecyTrait; +use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Slim\App; use Slim\Factory\AppFactory; +use Slim\Http\ServerRequest; use Slim\Psr7\Factory\StreamFactory; use Slim\Psr7\Headers; use Slim\Psr7\Request; @@ -23,14 +25,14 @@ protected function getAppInstance(): App { $container = (new ContainerBuilder()) ->addDefinitions(Dependencies::definitions()) - ->addDefinitions(include __DIR__ . '/../example/config/settings.inc.php') - ->addDefinitions(include __DIR__ . '/../example/config/dependencies.inc.php') + ->addDefinitions(include __DIR__ . '/Fixtures/app/settings.inc.php') + ->addDefinitions(include __DIR__ . '/Fixtures/app/dependencies.inc.php') ->build(); $app = AppFactory::createFromContainer($container); - include __DIR__ . "/../example/config/middleware.inc.php"; - include __DIR__ . "/../example/config/routes.inc.php"; + include __DIR__ . "/Fixtures/app/middleware.inc.php"; + include __DIR__ . "/Fixtures/app/routes.inc.php"; return $app; } @@ -50,6 +52,19 @@ protected function createRequest( foreach ($headers as $name => $value) { $h->addHeader($name, $value); } - return new Request($method, $uri, $h, $cookies, $serverParams, $stream); + return new ServerRequest(new Request($method, $uri, $h, $cookies, $serverParams, $stream)); } + + protected function assertLocationHeader($expectedLocation, ResponseInterface $response, bool $exact = true) + { + $headers = $response->getHeaders(); + if ($exact) { + $this->assertContainsEquals('Location', array_keys($headers)); + $this->assertContainsEquals($expectedLocation, $headers['Location']); + } else { + $this->assertContains('Location', array_keys($headers)); + $this->assertContains($expectedLocation, $headers['Location']); + } + } + } diff --git a/tests/TestZZZ.php b/tests/TestZZZ.php new file mode 100644 index 0000000..d81ad32 --- /dev/null +++ b/tests/TestZZZ.php @@ -0,0 +1,13 @@ +