[Pkg-owncloud-commits] [php-sabredav] 57/64: Authentication backends can now specify the reason for failure.
David Prévot
taffit at moszumanska.debian.org
Thu Dec 11 15:13:27 UTC 2014
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to tag 2.2.0alpha1
in repository php-sabredav.
commit 21cc1b3ebbcf5d7792f73fd6ad5f1bd9dbb00a71
Author: Evert Pot <me at evertpot.com>
Date: Sat Dec 6 22:47:52 2014 -0500
Authentication backends can now specify the reason for failure.
---
lib/DAV/Auth/Backend/AbstractBasic.php | 17 ++++++++---------
lib/DAV/Auth/Backend/AbstractDigest.php | 19 +++++++++----------
lib/DAV/Auth/Backend/Apache.php | 15 +++++++--------
lib/DAV/Auth/Backend/BackendInterface.php | 11 +++++------
lib/DAV/Auth/Plugin.php | 16 +++++++++-------
tests/Sabre/DAV/Auth/Backend/AbstractBasicTest.php | 10 +++++-----
tests/Sabre/DAV/Auth/Backend/AbstractDigestTest.php | 21 +++++++++------------
tests/Sabre/DAV/Auth/Backend/ApacheTest.php | 8 ++++----
tests/Sabre/DAV/Auth/Backend/BasicCallBackTest.php | 2 +-
tests/Sabre/DAV/Auth/Backend/Mock.php | 15 +++++++--------
10 files changed, 64 insertions(+), 70 deletions(-)
diff --git a/lib/DAV/Auth/Backend/AbstractBasic.php b/lib/DAV/Auth/Backend/AbstractBasic.php
index 5222a25..7c73bf4 100644
--- a/lib/DAV/Auth/Backend/AbstractBasic.php
+++ b/lib/DAV/Auth/Backend/AbstractBasic.php
@@ -55,8 +55,10 @@ abstract class AbstractBasic implements BackendInterface {
* When this method is called, the backend must check if authentication was
* successful.
*
- * This method should simply return null if authentication was not
- * successful.
+ * The returned value must be one of the following
+ *
+ * [true, "principals/username"]
+ * [false, "reason for failure"]
*
* If authentication was successful, it's expected that the authentication
* backend returns a so-called principal url.
@@ -73,12 +75,9 @@ abstract class AbstractBasic implements BackendInterface {
*
* principals/users/[username]
*
- * But literally any non-null value will be accepted as a 'succesful
- * authentication'.
- *
* @param RequestInterface $request
* @param ResponseInterface $response
- * @return null|string
+ * @return array
*/
function check(RequestInterface $request, ResponseInterface $response) {
@@ -90,12 +89,12 @@ abstract class AbstractBasic implements BackendInterface {
$userpass = $auth->getCredentials($request);
if (!$userpass) {
- return null;
+ return [false, "No 'Authorization: Basic' header found. Either the client didn't send one, or the server is mis-configured"];
}
if (!$this->validateUserPass($userpass[0],$userpass[1])) {
- return null;
+ return [false, "Username or password was incorrect"];
}
- return $this->principalPrefix . $userpass[0];
+ return [true, $this->principalPrefix . $userpass[0]];
}
diff --git a/lib/DAV/Auth/Backend/AbstractDigest.php b/lib/DAV/Auth/Backend/AbstractDigest.php
index bb423b7..d55b4e5 100644
--- a/lib/DAV/Auth/Backend/AbstractDigest.php
+++ b/lib/DAV/Auth/Backend/AbstractDigest.php
@@ -53,8 +53,10 @@ abstract class AbstractDigest implements BackendInterface {
* When this method is called, the backend must check if authentication was
* successful.
*
- * This method should simply return null if authentication was not
- * successful.
+ * The returned value must be one of the following
+ *
+ * [true, "principals/username"]
+ * [false, "reason for failure"]
*
* If authentication was successful, it's expected that the authentication
* backend returns a so-called principal url.
@@ -71,12 +73,9 @@ abstract class AbstractDigest implements BackendInterface {
*
* principals/users/[username]
*
- * But literally any non-null value will be accepted as a 'succesful
- * authentication'.
- *
* @param RequestInterface $request
* @param ResponseInterface $response
- * @return null|string
+ * @return array
*/
function check(RequestInterface $request, ResponseInterface $response) {
@@ -91,13 +90,13 @@ abstract class AbstractDigest implements BackendInterface {
// No username was given
if (!$username) {
- return null;
+ return [false, "No 'Authorization: Digest' header found. Either the client didn't send one, or the server is mis-configured"];
}
$hash = $this->getDigestHash($this->realm, $username);
// If this was false, the user account didn't exist
if ($hash===false || is_null($hash)) {
- return null;
+ return [false, "Username or password was incorrect"];
}
if (!is_string($hash)) {
throw new DAV\Exception('The returned value from getDigestHash must be a string or null');
@@ -105,10 +104,10 @@ abstract class AbstractDigest implements BackendInterface {
// If this was false, the password or part of the hash was incorrect.
if (!$digest->validateA1($hash)) {
- return null;
+ return [false, "Username or password was incorrect"];
}
- return $this->principalPrefix . $username;
+ return [true, $this->principalPrefix . $username];
}
diff --git a/lib/DAV/Auth/Backend/Apache.php b/lib/DAV/Auth/Backend/Apache.php
index b7c01f4..f46ffc6 100644
--- a/lib/DAV/Auth/Backend/Apache.php
+++ b/lib/DAV/Auth/Backend/Apache.php
@@ -32,8 +32,10 @@ class Apache implements BackendInterface {
* When this method is called, the backend must check if authentication was
* successful.
*
- * This method should simply return null if authentication was not
- * successful.
+ * The returned value must be one of the following
+ *
+ * [true, "principals/username"]
+ * [false, "reason for failure"]
*
* If authentication was successful, it's expected that the authentication
* backend returns a so-called principal url.
@@ -50,12 +52,9 @@ class Apache implements BackendInterface {
*
* principals/users/[username]
*
- * But literally any non-null value will be accepted as a 'succesful
- * authentication'.
- *
* @param RequestInterface $request
* @param ResponseInterface $response
- * @return null|string
+ * @return array
*/
function check(RequestInterface $request, ResponseInterface $response) {
@@ -64,10 +63,10 @@ class Apache implements BackendInterface {
$remoteUser = $request->getRawServerValue('REDIRECT_REMOTE_USER');
}
if (is_null($remoteUser)) {
- return null;
+ return [false, 'No REMOTE_USER property was found in the PHP $_SERVER super-global. This likely means your server is not configured correctly'];
}
- return $this->principalPrefix . $remoteUser;
+ return [true, $this->principalPrefix . $remoteUser];
}
diff --git a/lib/DAV/Auth/Backend/BackendInterface.php b/lib/DAV/Auth/Backend/BackendInterface.php
index 954a9ed..fe3d183 100644
--- a/lib/DAV/Auth/Backend/BackendInterface.php
+++ b/lib/DAV/Auth/Backend/BackendInterface.php
@@ -19,8 +19,10 @@ interface BackendInterface {
* When this method is called, the backend must check if authentication was
* successful.
*
- * This method should simply return null if authentication was not
- * successful.
+ * The returned value must be one of the following
+ *
+ * [true, "principals/username"]
+ * [false, "reason for failure"]
*
* If authentication was successful, it's expected that the authentication
* backend returns a so-called principal url.
@@ -37,12 +39,9 @@ interface BackendInterface {
*
* principals/users/[username]
*
- * But literally any non-null value will be accepted as a 'succesful
- * authentication'.
- *
* @param RequestInterface $request
* @param ResponseInterface $response
- * @return null|string
+ * @return array
*/
function check(RequestInterface $request, ResponseInterface $response);
diff --git a/lib/DAV/Auth/Plugin.php b/lib/DAV/Auth/Plugin.php
index 5ce2633..8617d8c 100644
--- a/lib/DAV/Auth/Plugin.php
+++ b/lib/DAV/Auth/Plugin.php
@@ -130,18 +130,20 @@ class Plugin extends ServerPlugin {
*/
function beforeMethod(RequestInterface $request, ResponseInterface $response) {
- $this->currentPrincipal = $this->authBackend->check(
+ $result = $this->authBackend->check(
$request,
$response
);
- if (!$this->currentPrincipal) {
+ if (!is_array($result) || count($result)!==2 || !is_bool($result[0]) || !is_string($result[1])) {
+ throw new \Sabre\DAV\Exception('The authentication backend did not return a correct value from the check() method.');
+ }
+ if ($result[0]) {
+ $this->currentPrincipal = $result[1];
+ } else {
+ $this->currentPrincipal = null;
$this->authBackend->requireAuth($request, $response);
- if (!$request->hasHeader('Authorization')) {
- throw new NotAuthenticated('Authentication failed. We didn\'t see an Authorization header, this means that the client didn\'t try to authenticate, or that the server is mis-configured');
- } else {
- throw new NotAuthenticated('Authentication failed.');
- }
+ throw new NotAuthenticated('Authentication failed. Reason: ' . $result[1]);
}
}
diff --git a/tests/Sabre/DAV/Auth/Backend/AbstractBasicTest.php b/tests/Sabre/DAV/Auth/Backend/AbstractBasicTest.php
index f156bfb..8c7dbd4 100644
--- a/tests/Sabre/DAV/Auth/Backend/AbstractBasicTest.php
+++ b/tests/Sabre/DAV/Auth/Backend/AbstractBasicTest.php
@@ -16,8 +16,8 @@ class AbstractBasicTest extends \PHPUnit_Framework_TestCase {
$backend = new AbstractBasicMock();
- $this->assertNull(
- $backend->check($request, $response)
+ $this->assertFalse(
+ $backend->check($request, $response)[0]
);
}
@@ -32,8 +32,8 @@ class AbstractBasicTest extends \PHPUnit_Framework_TestCase {
$backend = new AbstractBasicMock();
- $this->assertNull(
- $backend->check($request, $response)
+ $this->assertFalse(
+ $backend->check($request, $response)[0]
);
}
@@ -48,7 +48,7 @@ class AbstractBasicTest extends \PHPUnit_Framework_TestCase {
$backend = new AbstractBasicMock();
$this->assertEquals(
- 'principals/username',
+ [true, 'principals/username'],
$backend->check($request, $response)
);
diff --git a/tests/Sabre/DAV/Auth/Backend/AbstractDigestTest.php b/tests/Sabre/DAV/Auth/Backend/AbstractDigestTest.php
index dff5f9b..3a98b2b 100644
--- a/tests/Sabre/DAV/Auth/Backend/AbstractDigestTest.php
+++ b/tests/Sabre/DAV/Auth/Backend/AbstractDigestTest.php
@@ -13,8 +13,8 @@ class AbstractDigestTest extends \PHPUnit_Framework_TestCase {
$response = new HTTP\Response();
$backend = new AbstractDigestMock();
- $this->assertNull(
- $backend->check($request, $response)
+ $this->assertFalse(
+ $backend->check($request, $response)[0]
);
}
@@ -28,13 +28,10 @@ class AbstractDigestTest extends \PHPUnit_Framework_TestCase {
$response = new HTTP\Response();
$backend = new AbstractDigestMock();
- $this->assertNull(
- $backend->check($request, $response)
+ $this->assertFalse(
+ $backend->check($request, $response)[0]
);
- $backend = new AbstractDigestMock();
- $backend->check($request, $response);
-
}
/**
@@ -69,8 +66,8 @@ class AbstractDigestTest extends \PHPUnit_Framework_TestCase {
$response = new HTTP\Response();
$backend = new AbstractDigestMock();
- $this->assertNull(
- $backend->check($request, $response)
+ $this->assertFalse(
+ $backend->check($request, $response)[0]
);
}
@@ -86,8 +83,8 @@ class AbstractDigestTest extends \PHPUnit_Framework_TestCase {
$response = new HTTP\Response();
$backend = new AbstractDigestMock();
- $this->assertNull(
- $backend->check($request, $response)
+ $this->assertFalse(
+ $backend->check($request, $response)[0]
);
}
@@ -106,7 +103,7 @@ class AbstractDigestTest extends \PHPUnit_Framework_TestCase {
$backend = new AbstractDigestMock();
$this->assertEquals(
- 'principals/user',
+ [true, 'principals/user'],
$backend->check($request, $response)
);
diff --git a/tests/Sabre/DAV/Auth/Backend/ApacheTest.php b/tests/Sabre/DAV/Auth/Backend/ApacheTest.php
index 6532169..20be0ed 100644
--- a/tests/Sabre/DAV/Auth/Backend/ApacheTest.php
+++ b/tests/Sabre/DAV/Auth/Backend/ApacheTest.php
@@ -20,8 +20,8 @@ class ApacheTest extends \PHPUnit_Framework_TestCase {
$response = new HTTP\Response();
$backend = new Apache();
- $this->assertNull(
- $backend->check($request, $response)
+ $this->assertFalse(
+ $backend->check($request, $response)[0]
);
}
@@ -35,7 +35,7 @@ class ApacheTest extends \PHPUnit_Framework_TestCase {
$backend = new Apache();
$this->assertEquals(
- 'principals/username',
+ [true, 'principals/username'],
$backend->check($request, $response)
);
@@ -50,7 +50,7 @@ class ApacheTest extends \PHPUnit_Framework_TestCase {
$backend = new Apache();
$this->assertEquals(
- 'principals/username',
+ [true, 'principals/username'],
$backend->check($request, $response)
);
diff --git a/tests/Sabre/DAV/Auth/Backend/BasicCallBackTest.php b/tests/Sabre/DAV/Auth/Backend/BasicCallBackTest.php
index 74d59e8..5275a52 100644
--- a/tests/Sabre/DAV/Auth/Backend/BasicCallBackTest.php
+++ b/tests/Sabre/DAV/Auth/Backend/BasicCallBackTest.php
@@ -27,7 +27,7 @@ class BasicCallBackTest extends \PHPUnit_Framework_TestCase {
$response = new Response();
$this->assertEquals(
- 'principals/foo',
+ [true, 'principals/foo'],
$backend->check($request, $response)
);
diff --git a/tests/Sabre/DAV/Auth/Backend/Mock.php b/tests/Sabre/DAV/Auth/Backend/Mock.php
index ffb0d4b..127aac4 100644
--- a/tests/Sabre/DAV/Auth/Backend/Mock.php
+++ b/tests/Sabre/DAV/Auth/Backend/Mock.php
@@ -24,8 +24,10 @@ class Mock implements BackendInterface {
* When this method is called, the backend must check if authentication was
* successful.
*
- * This method should simply return null if authentication was not
- * successful.
+ * The returned value must be one of the following
+ *
+ * [true, "principals/username"]
+ * [false, "reason for failure"]
*
* If authentication was successful, it's expected that the authentication
* backend returns a so-called principal url.
@@ -42,20 +44,17 @@ class Mock implements BackendInterface {
*
* principals/users/[username]
*
- * But literally any non-null value will be accepted as a 'succesful
- * authentication'.
- *
* @param RequestInterface $request
* @param ResponseInterface $response
- * @return null|string
+ * @return array
*/
function check(RequestInterface $request, ResponseInterface $response) {
if ($this->fail) {
- return null;
+ return [false, "fail!"];
}
$this->principal = $this->defaultPrincipal;
- return $this->principal;
+ return [true, $this->principal];
}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/php-sabredav.git
More information about the Pkg-owncloud-commits
mailing list