yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.24k stars 6.91k forks source link

User login events not triggered when using \yii\web\HttpBasicAuth #18031

Closed achretien closed 4 years ago

achretien commented 4 years ago

What steps will reproduce the problem?

What is the expected result?

Trigger the event named AFTER LOGIN and show the debug log

What do you get instead?

Nothing show in the logs

Additional info

The HttpBasicAuth filter use $user->switchIdentity($identity) instead of $user->login($identity)

Q A
Yii version 2.0.35
PHP version 7.4
Operating system Linux (Docker)
SOHELAHMED7 commented 4 years ago

Hi Everyone,

I would like to fix this issue.

But I don't have concrete understanding how Http Basic Auth work here. As far as my knowledge about HTTP basic auth is concerned and it is a username & password prompt that we see in browser

browser-http-auth

How can I get generic browser prompt like that in fresh yii2-app-advanced app? I am trying to reproduce the issue.

Above prompt is generated by

<?php
    /*
    ** Define a couple of functions for
    ** starting and ending an HTML document
    */
    function startPage()
    {
        print("<html>\n");
        print("<head>\n");
        print("<title>Listing 24-1</title>\n");
        print("</head>\n");
        print("<body>\n");
    }

    function endPage()
    {
        print("</body>\n");
        print("</html>\n");
    }
    /*
    ** test for username/password
    */
    if( ( isset($_SERVER['PHP_AUTH_USER'] ) && ( $_SERVER['PHP_AUTH_USER'] == "leon" ) ) AND
      ( isset($_SERVER['PHP_AUTH_PW'] ) && ( $_SERVER['PHP_AUTH_PW'] == "secret" )) )
    {
        startPage();

        print("You have logged in successfully!<br>\n");

        endPage();
    }
    else
    {
        //Send headers to cause a browser to request
        //username and password from user
        header("WWW-Authenticate: " .
            "Basic realm=\"Leon's Protected Area\"");
        header("HTTP/1.0 401 Unauthorized");

        //Show failure text, which browsers usually
        //show only after several failed attempts
        print("This page is protected by HTTP " .
            "Authentication.<br>\nUse <b>leon</b> " .
            "for the username, and <b>secret</b> " .
            "for the password.<br>\n");
    }
?>

Reference - https://stackoverflow.com/a/4577953/3794786

I tried to get this prompt by putting HttpBasicAuth filter(https://www.yiiframework.com/doc/api/2.0/yii-filters-auth-httpbasicauth) in the behavior of frontend/SiteController.php but always getting Your request was made with invalid credentials. error.

Second thought about the HTTP basic auth is about REST API where a client sends request to server

E.g.

$ch = curl_init($host);
curl_setopt($ch, CURLOPT_HTTPHEADER, array('Content-Type: application/xml', $additionalHeaders));
curl_setopt($ch, CURLOPT_HEADER, 1);
curl_setopt($ch, CURLOPT_USERPWD, $username . ":" . $password);
curl_setopt($ch, CURLOPT_TIMEOUT, 30);
curl_setopt($ch, CURLOPT_POST, 1);
curl_setopt($ch, CURLOPT_POSTFIELDS, $payloadName);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, TRUE);
$return = curl_exec($ch);
curl_close($ch);

Reference - https://stackoverflow.com/questions/2140419/how-do-i-make-a-request-using-http-basic-authentication-with-php-curl

So this issue is about which case from the above two?

Any guidance/hint appreciated

achretien commented 4 years ago

Hi In the basic template (it should be possible in the advance also)

In the file controller/SiteController.php, add the following behavior in first position:

            'basicAuth' => [
                'class' => \yii\filters\auth\HttpBasicAuth::class,
                'except' => ['error'],
                'auth' => static function ($username, $password) {
                    $user = \app\models\User::findByUsername($username);
                    if ($user && $user->validatePassword($password)) {
                        return $user;
                    }
                    return null;
                },
            ],

In the config/web.php file add this to the 'user' component:

            'on afterLogin' => static function () {
                \Yii::info('Logged IN');
            },

After login with demo/demo, the log doesn't show up in the debugger.

If you change switchIdentity to login in the HttpBasicAuth.php file line 104, the log show up

SOHELAHMED7 commented 4 years ago

Note: this is browser prompt based HTTP basic authentication.

SOHELAHMED7 commented 4 years ago

Also i found another issue:

If I login successfully I cannot logout. @samdark should I create new issue for this?

SOHELAHMED7 commented 4 years ago

Upon viewing similar issues and file history of HttpBasicAuth.php I have multiple thoughts.

yii\filters\auth\HttpBasicAuth should only check/filter username and/or password is true or not. It should not perform login operation. It should be done outside the filter file mostly in controller action like we do in a typical web app (https://github.com/SOHELAHMED7/yii2-app-advanced/blob/master/common/models/LoginForm.php#L59).

However the default implementation of yii\filters\auth\HttpBasicAuth use loginByAccessToken (https://github.com/yiisoft/yii2/blob/master/framework/web/User.php#L294) which perform login (\yii\web\User::login()) operation in it.

Also I cannot completely figure out why switchIdentity is introduced in the commit https://github.com/tvdavid/yii2/commit/39c183c7ff2dcfed06d75c401eb8de99b937d1c9#diff-490915389a2d974a09da94224d4e1d4aR73

So I think as @achretien said we should replace switchIdentity() to login()

achretien commented 4 years ago

Yes to be consistent with login by access token (used in the other auth filters).

rob006 commented 4 years ago

@SOHELAHMED7 AFAIK there is no traditional login/logout event in basic auth. Each request provides login and password in request headers - no session, no identity cookie, every request validates provided login credentials (so each request will trigger EVENT_AFTER_LOGIN after change requested in this issue). Removing username/password validation from HttpBasicAuth may not be a thing that most people will expect from basic auth, and kinda defeats purpose this behavior.

SOHELAHMED7 commented 4 years ago

@rob006

there is no traditional login/logout event in basic auth

There is. When no auth callback provided, yii\filters\auth\HttpBasicAuth will try to login the user by only username and will call loginByAccessToken() (https://github.com/yiisoft/yii2-framework/blob/master/filters/auth/HttpBasicAuth.php#L110) which in turn calls yii\web\User::login() (https://github.com/yiisoft/yii2/blob/master/framework/web/User.php#L299) which will trigger the login event.

Removing username/password validation from HttpBasicAuth may not be a thing that most people will expect from basic auth, and kinda defeats purpose this behavior.

Yes definitely. I agree. And checking the username and/or password is the main purpose of this filter

achretien commented 4 years ago

@rob006 the event is already triggered each time in case of token auth (Bearer for exemple).

As @SOHELAHMED7 explain, the loginByAccessToken method used in every other cases perform the login method and trigger the before and after login events.

I raised the issue because my user logged in to my API through username/password the first time then through a Bearer token provided for every other API calls.

In my case the framework raise no event on 'login' but raise after login event on authentication.

As a developper I expected than a event named AFTER_LOGIN_EVENT is raised when a user login :)

samdark commented 4 years ago

There's, indeed, inconsistency between HttpHeaderAuth that does login on valid token value and HttpBasicAuth that does not. Using login() instead of switchIdentity() makes these equal.

If I login successfully I cannot logout. @samdark should I create new issue for this?

That's expected. Once valid credentials are provided, it's up to the browser to decide when to stop using them. There are workarounds but these are tricky.

samdark commented 4 years ago

Overall, I think we should change switchIdentity() to login() there.