From 43337d58c0be097ca510f2abd1497f13f25bda5b Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sat, 2 Jul 2016 17:44:45 -0400 Subject: Preserve role for existing users when using ReverseProxy authentication --- ChangeLog | 1 + app/Auth/ReverseProxyAuth.php | 3 +- app/User/ReverseProxyUserProvider.php | 21 +++++- tests/units/Auth/ReverseProxyAuthTest.php | 111 ++++++++++++++++++++++++++++++ 4 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 tests/units/Auth/ReverseProxyAuthTest.php diff --git a/ChangeLog b/ChangeLog index 000400ae..3d2b0aa7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,7 @@ New features: Improvements: +* Preserve role for existing users when using ReverseProxy authentication * Handle priority for task and project duplication * Expose task reference field to the user interface * Improve ICal export diff --git a/app/Auth/ReverseProxyAuth.php b/app/Auth/ReverseProxyAuth.php index b9730c5c..fdf936b1 100644 --- a/app/Auth/ReverseProxyAuth.php +++ b/app/Auth/ReverseProxyAuth.php @@ -45,7 +45,8 @@ class ReverseProxyAuth extends Base implements PreAuthenticationProviderInterfac $username = $this->request->getRemoteUser(); if (! empty($username)) { - $this->userInfo = new ReverseProxyUserProvider($username); + $userProfile = $this->userModel->getByUsername($username); + $this->userInfo = new ReverseProxyUserProvider($username, $userProfile ?: array()); return true; } diff --git a/app/User/ReverseProxyUserProvider.php b/app/User/ReverseProxyUserProvider.php index 723b8155..34d2187d 100644 --- a/app/User/ReverseProxyUserProvider.php +++ b/app/User/ReverseProxyUserProvider.php @@ -21,15 +21,24 @@ class ReverseProxyUserProvider implements UserProviderInterface */ protected $username = ''; + /** + * User profile if the user already exists + * + * @access protected + * @var array + */ + private $userProfile = array(); + /** * Constructor * * @access public * @param string $username */ - public function __construct($username) + public function __construct($username, array $userProfile = array()) { $this->username = $username; + $this->userProfile = $userProfile; } /** @@ -84,7 +93,15 @@ class ReverseProxyUserProvider implements UserProviderInterface */ public function getRole() { - return REVERSE_PROXY_DEFAULT_ADMIN === $this->username ? Role::APP_ADMIN : Role::APP_USER; + if (REVERSE_PROXY_DEFAULT_ADMIN === $this->username) { + return Role::APP_ADMIN; + } + + if (isset($this->userProfile['role'])) { + return $this->userProfile['role']; + } + + return Role::APP_USER; } /** diff --git a/tests/units/Auth/ReverseProxyAuthTest.php b/tests/units/Auth/ReverseProxyAuthTest.php new file mode 100644 index 00000000..cdbc247d --- /dev/null +++ b/tests/units/Auth/ReverseProxyAuthTest.php @@ -0,0 +1,111 @@ +container['request'] = $this + ->getMockBuilder('\Kanboard\Core\Http\Request') + ->setConstructorArgs(array($this->container)) + ->setMethods(array('getRemoteUser')) + ->getMock(); + } + + public function testGetName() + { + $provider = new ReverseProxyAuth($this->container); + $this->assertEquals('ReverseProxy', $provider->getName()); + } + + public function testAuthenticateSuccess() + { + $this->container['request'] + ->expects($this->once()) + ->method('getRemoteUser') + ->will($this->returnValue('admin')); + + $provider = new ReverseProxyAuth($this->container); + $this->assertTrue($provider->authenticate()); + } + + public function testAuthenticateFailure() + { + $this->container['request'] + ->expects($this->once()) + ->method('getRemoteUser') + ->will($this->returnValue('')); + + $provider = new ReverseProxyAuth($this->container); + $this->assertFalse($provider->authenticate()); + } + + public function testValidSession() + { + $this->container['request'] + ->expects($this->once()) + ->method('getRemoteUser') + ->will($this->returnValue('admin')); + + $this->container['sessionStorage']->user = array( + 'username' => 'admin' + ); + + $provider = new ReverseProxyAuth($this->container); + $this->assertTrue($provider->isValidSession()); + } + + public function testInvalidSession() + { + $this->container['request'] + ->expects($this->once()) + ->method('getRemoteUser') + ->will($this->returnValue('foobar')); + + $this->container['sessionStorage']->user = array( + 'username' => 'admin' + ); + + $provider = new ReverseProxyAuth($this->container); + $this->assertFalse($provider->isValidSession()); + } + + public function testRoleForNewUser() + { + $this->container['request'] + ->expects($this->once()) + ->method('getRemoteUser') + ->will($this->returnValue('someone')); + + $provider = new ReverseProxyAuth($this->container); + $this->assertTrue($provider->authenticate()); + + $user = $provider->getUser(); + $this->assertEquals(Role::APP_USER, $user->getRole()); + } + + public function testRoleIsPreservedForExistingUser() + { + $this->container['request'] + ->expects($this->once()) + ->method('getRemoteUser') + ->will($this->returnValue('someone')); + + $provider = new ReverseProxyAuth($this->container); + $userModel = new UserModel($this->container); + + $this->assertEquals(2, $userModel->create(array('username' => 'someone', 'role' => Role::APP_MANAGER))); + + $this->assertTrue($provider->authenticate()); + + $user = $provider->getUser(); + $this->assertEquals(Role::APP_MANAGER, $user->getRole()); + } +} -- cgit v1.2.3