Implemented stateless login for Auth #1169

Merged
merged 1 commit into from Mar 13, 2013

Projects

None yet

3 participants

@ADmad
Member
ADmad commented Mar 5, 2013

I think I have finally managed to get stateless auth working in a sensible way with minimal hacking of the AuthComponent.

With this patch when using only BasicAuthenticate there is no redirection to login action nor session starting.

When using Basic and Form authenticators together (order matters) if required http headers are passed and valid user is found it behaves as mentioned above else continues with regular form basic authentication.

For now I only modified existing tests to pass. More tests need to be added after wiser men confirm what I have done is a good idea 😄

@markstory markstory commented on the diff Mar 5, 2013
...Cake/Controller/Component/Auth/DigestAuthenticate.php
@@ -97,9 +97,6 @@ class DigestAuthenticate extends BaseAuthenticate {
*/
public function __construct(ComponentCollection $collection, $settings) {
parent::__construct($collection, $settings);
- if (empty($this->settings['realm'])) {
- $this->settings['realm'] = env('SERVER_NAME');
- }
@markstory
markstory Mar 5, 2013 Member

Are you sure we don't need the realm anymore? I thought it was required to properly generate digest hashes.

@ADmad
ADmad Mar 5, 2013 Member

It is required but now DigestAuthenticate extends BasicAuthenticate and its constructor already sets the realm.

@markstory markstory commented on an outdated diff Mar 5, 2013
lib/Cake/Controller/Component/AuthComponent.php
$user = CakeSession::read(self::$sessionKey);
+ } else {
+ return null;
}
@markstory
markstory Mar 5, 2013 Member

This is a nice way of preventing the sessions from being started 👍

@markstory markstory commented on the diff Mar 5, 2013
...e/Controller/Component/Auth/BasicAuthenticateTest.php
@@ -113,10 +108,6 @@ public function testAuthenticateNoPassword() {
$_SERVER['PHP_AUTH_USER'] = 'mariano';
$_SERVER['PHP_AUTH_PW'] = null;
- $this->response->expects($this->once())
- ->method('header')
- ->with('WWW-Authenticate: Basic realm="localhost"');
@markstory
markstory Mar 5, 2013 Member

Why don't these get set anymore?

@ADmad
ADmad Mar 5, 2013 Member

They are no longer set when calling authenticate() but rather when unathenticated() is called. AuthComponent::_getUser() is now called for all requests which in turn calls getUser() of all athenticators. If none of the authenticators return a user record then AuthComponent::_unauthenticate() calls unauthenticated() which is what would trigger the headers if basic/digest is the last authenticator in the list.

@ADmad
ADmad Mar 5, 2013 Member

BasicAuthenticate::authenticate() is redundant now. Because if required credentials were passed BasicAuthenticate::getUser() would already return the user and if not BasicAuthenticate::unauthenticated() would send the headers asking for credentials.

@markstory
markstory Mar 6, 2013 Member

Ah ok. Thanks for explaining it.

@lorenzo lorenzo commented on the diff Mar 6, 2013
lib/Cake/Controller/Component/Auth/BaseAuthenticate.php
@@ -155,4 +155,15 @@ public function getUser(CakeRequest $request) {
return false;
}
+/**
+ * Handle unauthenticated access attempt.
@lorenzo
lorenzo Mar 6, 2013 Member

Can you somehow specify that only the last one in the chain is called?

@ADmad
ADmad Mar 6, 2013 Member

Yup I will review and improve the docblocks wherever needed and add more tests.

@lorenzo
Member
lorenzo commented Mar 6, 2013

This looks good to me, just a bit concerned about the clarity on how unauthenticated() works

@ADmad
Member
ADmad commented Mar 8, 2013

@markstory @lorenzo Updated docblocks and added new tests. Although session start was prevented when correct credentials were passed for basic/digest auth, I had to add one more tweak to ensure session is not started when no or incorrect credentials are passed (check third commit).

@markstory markstory commented on an outdated diff Mar 8, 2013
.../Test/Case/Controller/Component/AuthComponentTest.php
@@ -1348,4 +1348,79 @@ public function testUser() {
$result = $this->Auth->user('is_admin');
$this->assertFalse($result);
}
+
+/**
+ * testStatelessAuthNoRedirect method
+ *
+ * @return void
+ */
+ public function testStatelessAuthNoRedirect() {
+ if (CakeSession::id()) {
+ session_destroy();
+ debug(session_id());
@markstory
markstory Mar 8, 2013 Member

Left over debug()

@ADmad
Member
ADmad commented Mar 13, 2013

Does anyone have anymore feedback on this or should I get the manual updates ready and merge?

@markstory
Member

I think it looks good.

@lorenzo lorenzo merged commit abe6511 into cakephp:2.4 Mar 13, 2013
@lorenzo
Member
lorenzo commented Mar 13, 2013

Don't forget to document in the book! :)

@ADmad
Member
ADmad commented Mar 13, 2013

I thought my previous message would be hint enough that I intend to. :)

@lorenzo
Member
lorenzo commented Mar 13, 2013

I was not enough :trollface:

@ADmad ADmad deleted the ADmad:2.4-auth-take-2 branch Mar 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment