commit 4dda6fb5b103ad0544bc3c00d3214e2fa41f028e
parent 74976fcd857538feb4a49875ddad79ab2a41e75f
Author: Felicitus <felicitus@felicitus.org>
Date: Wed, 4 Nov 2015 17:45:00 +0100
Refactored the auth system to allow changing the user's legacy password, thus creating a new FOSRest user. Added unit tests for authentication
Diffstat:
8 files changed, 203 insertions(+), 31 deletions(-)
diff --git a/src/PartKeepr/AuthBundle/Action/ChangePasswordAction.php b/src/PartKeepr/AuthBundle/Action/ChangePasswordAction.php
@@ -3,17 +3,12 @@ namespace PartKeepr\AuthBundle\Action;
use Dunglas\ApiBundle\Action\ActionUtilTrait;
-use Dunglas\ApiBundle\Api\ResourceInterface;
-use Dunglas\ApiBundle\Exception\RuntimeException;
-use Dunglas\ApiBundle\Model\DataProviderInterface;
use FOS\UserBundle\Model\UserManagerInterface;
use FOS\UserBundle\Util\UserManipulator;
use PartKeepr\AuthBundle\Entity\User;
use PartKeepr\AuthBundle\Services\UserService;
use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\Security\Core\Encoder\EncoderFactory;
-use Symfony\Component\Serializer\SerializerInterface;
class ChangePasswordAction
{
@@ -51,7 +46,8 @@ class ChangePasswordAction
$this->userManager = $userManager;
}
- public function __invoke (Request $request) {
+ public function __invoke(Request $request)
+ {
$user = $this->userService->getUser();
if (!$request->request->has("oldpassword") && !$request->request->has("newpassword")) {
@@ -60,19 +56,31 @@ class ChangePasswordAction
$FOSUser = $this->userManager->findUserByUsername($user->getUsername());
- if ($FOSUser === null) {
- throw new \Exception("Cannot change password for legacy or LDAP users");
- }
+ if ($FOSUser !== null) {
+ $encoder = $this->encoderFactory->getEncoder($FOSUser);
+ $encoded_pass = $encoder->encodePassword($request->request->get("oldpassword"), $FOSUser->getSalt());
- $encoder = $this->encoderFactory->getEncoder($FOSUser);
- $encoded_pass = $encoder->encodePassword($request->request->get("oldpassword"), $FOSUser->getSalt());
+ if ($FOSUser->getPassword() != $encoded_pass) {
+ throw new \Exception("Old password is wrong");
+ }
- if ($FOSUser->getPassword() != $encoded_pass) {
- throw new \Exception("Old password is wrong");
- }
+ $this->userManipulator->changePassword($user->getUsername(), $request->request->get("newpassword"));
+ } else {
+ if ($user->isLegacy()) {
+ if ($user->getPassword() !== md5($request->request->get("oldpassword"))) {
+ throw new \Exception("Old password is wrong");
+ }
- $this->userManipulator->changePassword($user->getUsername(), $request->request->get("newpassword"));
+ $user->setNewPassword($request->request->get("newpassword"));
+
+ $this->userService->syncData($user);
+ } else {
+ throw new \Exception("Cannot change password for LDAP users");
+ }
+ }
+ $user->setPassword("");
+ $user->setNewPassword("");
return $user;
}
diff --git a/src/PartKeepr/AuthBundle/Action/PostUserAction.php b/src/PartKeepr/AuthBundle/Action/PostUserAction.php
@@ -73,6 +73,7 @@ class PostUserAction
$data->setLegacy(false);
$this->userService->syncData($data);
+ $data->setNewPassword("");
$data->setPassword("");
return $data;
diff --git a/src/PartKeepr/AuthBundle/Action/PutUserAction.php b/src/PartKeepr/AuthBundle/Action/PutUserAction.php
@@ -75,6 +75,7 @@ class PutUserAction
);
$this->userService->syncData($data);
+ $data->setNewPassword("");
$data->setPassword("");
$data->setLegacy(false);
diff --git a/src/PartKeepr/AuthBundle/Entity/User.php b/src/PartKeepr/AuthBundle/Entity/User.php
@@ -3,6 +3,7 @@ namespace PartKeepr\AuthBundle\Entity;
use Doctrine\ORM\Mapping as ORM;
use PartKeepr\DoctrineReflectionBundle\Annotation\TargetService;
+use PartKeepr\DoctrineReflectionBundle\Annotation\VirtualField;
use PartKeepr\Util\BaseEntity;
use Symfony\Component\Security\Core\User\EquatableInterface;
use Symfony\Component\Security\Core\User\UserInterface;
@@ -31,9 +32,11 @@ class User extends BaseEntity implements UserInterface, EquatableInterface
private $password;
/**
+ * @Groups({"default"})
+ * @VirtualField(type="string")
* @var string
*/
- private $plainPassword;
+ private $newPassword;
/**
* @Assert\Email()
@@ -203,7 +206,7 @@ class User extends BaseEntity implements UserInterface, EquatableInterface
*/
public function getPassword()
{
- return $this->plainPassword;
+ return $this->password;
}
/**
@@ -213,18 +216,18 @@ class User extends BaseEntity implements UserInterface, EquatableInterface
*/
public function setPassword($password)
{
- $this->plainPassword = $password;
+ $this->password = $password;
}
- public function getPlainPassword () {
- return $this->plainPassword;
+ public function getNewPassword () {
+ return $this->newPassword;
}
/**
- * Sets the plain password. Used for password changes
+ * Sets the new password. Used for password changes
*/
- public function setPlainPassword ($password) {
-
+ public function setNewPassword ($password) {
+ $this->newPassword = $password;
}
/**
diff --git a/src/PartKeepr/AuthBundle/Services/UserService.php b/src/PartKeepr/AuthBundle/Services/UserService.php
@@ -97,14 +97,15 @@ class UserService
if ($FOSUser === null) {
- if ($user->getPlainPassword() == "") {
+ if ($user->getNewPassword() == "") {
throw new \Exception("Password must be set");
}
- $FOSUser = $this->userManipulator->create($user->getUsername(), $user->getPlainPassword(), "", true, false);
+ $FOSUser = $this->userManipulator->create($user->getUsername(), $user->getNewPassword(), "", true, false);
+ $user->setLegacy(false);
}
- if ($user->getPlainPassword() != "") {
- $this->userManipulator->changePassword($user->getUsername(), $user->getPlainPassword());
+ if ($user->getNewPassword() != "") {
+ $this->userManipulator->changePassword($user->getUsername(), $user->getNewPassword());
}
diff --git a/src/PartKeepr/AuthBundle/Tests/LegacyAuthTest.php b/src/PartKeepr/AuthBundle/Tests/LegacyAuthTest.php
@@ -0,0 +1,43 @@
+<?php
+namespace PartKeepr\AuthBundle\Tests;
+
+use PartKeepr\AuthBundle\Entity\User;
+use PartKeepr\CoreBundle\Tests\WebTestCase;
+
+class LegacyAuthTest extends WebTestCase
+{
+ public function setUp()
+ {
+ $this->loadFixtures(array());
+ }
+
+ public function testLegacyAuth () {
+ $user = new User("foobar");
+ $user->setPassword(md5("admin"));
+ $user->setLegacy(true);
+
+ $this->getContainer()->get("doctrine.orm.default_entity_manager")->persist($user);
+ $this->getContainer()->get("doctrine.orm.default_entity_manager")->flush($user);
+
+ $client = static::makeClient(false, array(
+ 'PHP_AUTH_USER' => "foobar",
+ 'PHP_AUTH_PW' => "admin"
+ )
+ );
+
+ $client->request("GET", "/api/system_status");
+
+ $this->assertEquals(200, $client->getResponse()->getStatusCode());
+
+ $client = static::makeClient(false, array(
+ 'PHP_AUTH_USER' => "foobar",
+ 'PHP_AUTH_PW' => "admin2"
+ )
+ );
+
+ $client->request("GET", "/api/system_status");
+
+ $this->assertEquals(401, $client->getResponse()->getStatusCode());
+
+ }
+}
diff --git a/src/PartKeepr/AuthBundle/Tests/UserTest.php b/src/PartKeepr/AuthBundle/Tests/UserTest.php
@@ -0,0 +1,114 @@
+<?php
+namespace PartKeepr\AuthBundle\Tests;
+
+
+use PartKeepr\AuthBundle\Entity\User;
+use PartKeepr\CoreBundle\Tests\WebTestCase;
+
+class UserTest extends WebTestCase
+{
+ public function setUp()
+ {
+ $this->loadFixtures(array());
+ }
+
+ public function testCreateUser () {
+ $client = static::makeClient(true);
+
+ $data = array(
+ "username" => "foobartest",
+ "newPassword" => "1234"
+ );
+
+ $client->request("POST", "/api/users", array(), array(), array(), json_encode($data));
+
+ $response = json_decode($client->getResponse()->getContent());
+
+ $this->assertEquals(201, $client->getResponse()->getStatusCode());
+ $this->assertEquals("foobartest", $response->{"username"});
+ $this->assertEmpty($response->{"password"});
+ $this->assertEmpty($response->{"newPassword"});
+ $this->assertFalse($response->{"legacy"});
+ }
+
+ public function testChangeUserPassword () {
+ $builtinProvider = $this->getContainer()->get("partkeepr.userservice")->getBuiltinProvider();
+
+ $user = new User("bernd");
+ $user->setPassword(md5("admin"));
+ $user->setLegacy(true);
+ $user->setProvider($builtinProvider);
+
+ $this->getContainer()->get("doctrine.orm.default_entity_manager")->persist($user);
+ $this->getContainer()->get("doctrine.orm.default_entity_manager")->flush($user);
+
+ $client = static::makeClient(true);
+
+ $iriConverter = $this->getContainer()->get("api.iri_converter");
+ $iri = $iriConverter->getIriFromItem($user);
+
+ $client->request("GET", $iri);
+
+ $response = json_decode($client->getResponse()->getContent());
+
+ $response->{"newPassword"} = "foobar";
+
+ $client->request("PUT", $iri, array(), array(), array(), json_encode($response));
+
+ $response = json_decode($client->getResponse()->getContent());
+
+ $this->assertEquals(200, $client->getResponse()->getStatusCode());
+ $this->assertEmpty($response->{"password"});
+ $this->assertEmpty($response->{"newPassword"});
+ $this->assertFalse($response->{"legacy"});
+ }
+
+ public function testSelfChangeUserPassword () {
+ $builtinProvider = $this->getContainer()->get("partkeepr.userservice")->getBuiltinProvider();
+
+ $user = new User("bernd2");
+ $user->setPassword(md5("admin"));
+ $user->setLegacy(true);
+ $user->setProvider($builtinProvider);
+
+ $this->getContainer()->get("doctrine.orm.default_entity_manager")->persist($user);
+ $this->getContainer()->get("doctrine.orm.default_entity_manager")->flush($user);
+
+ $client = static::makeClient(false, array(
+ 'PHP_AUTH_USER' => "bernd2",
+ 'PHP_AUTH_PW' => "admin"
+ )
+ );
+
+ $iriConverter = $this->getContainer()->get("api.iri_converter");
+ $iri = $iriConverter->getIriFromItem($user)."/changePassword";
+
+ $parameters = array(
+ "oldpassword" => "admin",
+ "newpassword" => "foobar"
+ );
+
+ $client->request("PUT", $iri, $parameters);
+
+ $response = json_decode($client->getResponse()->getContent());
+
+ $this->assertEquals(200, $client->getResponse()->getStatusCode());
+ $this->assertFalse($response->{"legacy"});
+ $this->assertEmpty($response->{"password"});
+ $this->assertEmpty($response->{"newPassword"});
+
+ $client = static::makeClient(false, array(
+ 'PHP_AUTH_USER' => "bernd2",
+ 'PHP_AUTH_PW' => "foobar"
+ )
+ );
+
+ $client->request("PUT", $iri, $parameters);
+
+ $response = json_decode($client->getResponse()->getContent());
+
+ $this->assertEquals(500, $client->getResponse()->getStatusCode());
+ $this->assertObjectHasAttribute("@type", $response);
+ $this->assertEquals("Error", $response->{"@type"});
+ }
+}
diff --git a/src/PartKeepr/FrontendBundle/Resources/public/js/Components/User/UserEditor.js b/src/PartKeepr/FrontendBundle/Resources/public/js/Components/User/UserEditor.js
@@ -20,7 +20,7 @@ Ext.define('PartKeepr.UserEditor', {
}, {
xtype: 'textfield',
inputType: "password",
- name: 'password',
+ name: 'newPassword',
fieldLabel: i18n("Password")
}, {
xtype: 'displayfield',
@@ -31,14 +31,15 @@ Ext.define('PartKeepr.UserEditor', {
}
];
- this.on("startEdit", this.onEditStart, this, {delay: 200});
-
+ this.on("startEdit", this.toggleLegacyField, this, {delay: 200});
this.callParent();
},
- onEditStart: function ()
+ toggleLegacyField: function ()
{
if (this.record.get("legacy") === true) {
this.down("#legacyField").setVisible(true);
+ } else {
+ this.down("#legacyField").setVisible(false);
}
}
});
