CompositeAuth does not respect the $user of authMehods

[spmaxinc]

What steps will reproduce the problem?

When developing authorization mechanism for REST API there is an option to use CompositeAuth to support several authorization mechanisms. In my case, I have two user components in the app and I would like to authenticate with either one of the methods. I would like to use ONE user for HttpHeaderAuth and ANOTHER (default) user for JwtHttpBearerAuth methods. However, compositeAuth does not respect the user set in the configs of authMethds. This is due to CompositeAuth using it's own $user property (which is set by default to Yii::$app->user).

'compositeAuth' => [
    'class' => CompositeAuth::class,
    'authMethods' => [
       [
         'class' => HttpHeaderAuth::class,
         'user' => Yii::$app->terminal,
          'header' => 'terminal',
        ],
        JwtHttpBearerAuth::class,
     ],
],

The problem is in authenticate(..) function of CompositeAuth class. It passes CompositeAuth::$user object instead of passing respected authMethod $user object.

//Current code
 $identity = $auth->authenticate($user, $request, $response);

//Proposed fix
 $identity = $auth->authenticate($auth->user, $request, $response);

Additionally, $request and $response should also be probably taken from the config of the $auth object and not from CompositeAuth properties.

What is the expected result?

What do you get instead?

Additional info

Q	A
Yii version	2.0.?
PHP version
Operating system

[bizley]

Agree. Do you have time for PR with this?

[schmunk42]

But the first param $user would be obsolete then, see

yii2/framework/filters/auth/CompositeAuth.php

Lines 65 to 95 in 9fc8a2d

	public function authenticate($user, $request, $response)
	{
	foreach ($this->authMethods as $i => $auth) {
	if (!$auth instanceof AuthInterface) {
	$this->authMethods[$i] = $auth = Yii::createObject($auth);
	if (!$auth instanceof AuthInterface) {
	throw new InvalidConfigException(get_class($auth) . ' must implement yii\filters\auth\AuthInterface');
	}
	}
	if (
	$this->owner instanceof Controller
	&& (
	!isset($this->owner->action)
	|| (
	$auth instanceof ActionFilter
	&& !$auth->isActive($this->owner->action)
	)
	)
	) {
	continue;
	}
	$identity = $auth->authenticate($user, $request, $response);
	if ($identity !== null) {
	return $identity;
	}
	}
	return null;
	}

It only used in line 88 again. This would actually be an API change.

This change needs to be "triple" checked, looks to me like there could be very strange side effects.

PS: If changed, this should go into 2.2.

CC: @handcode @eluhr

[bizley]

I would rather use the CompositeAuth user when user in auth method is not set. The same for request and response.
Not sure about 2.2 though but definitely it needs to be checked throughout.

[terabytesoftw]

If it is a bug, it should be fixed.

[spmaxinc]

It is a change, but also kind of a bug. My expectation is that $user of authMethods should be used and if null (not set), then CompositeAuth::$user should be used and if that one null (not set) then Yii::$app->user should be used.

This way it will maintain compatibility with existing/deployed code.

[spmaxinc]

Whatever fix is implemented, it should also address CompositeAuth::challenge(...) function. It has the same problem as authenticate(...).

[spmaxinc]

The more I think about it the more I don't understand how this whole authenticate function is supposed to be used. On one hand it is supposed to be implemented as part of the interface with $user, etc being passed to it by some external code. But on the other, $user is a member of the class implementing this same interface. So which $user is supposed to be used? One being passed by external code or the one defined in the class itself??

That is why in the current design $user is being silently substituted by CompositeAuth::authenticate(...) function and no longer reflects the $user defined in the AuthMethod class.

Which also means that @schmunk42 is right and it is a design issue. Once you change $user to $auth->user in CompositeAuth::authenticate, the $user argument passed into the function is no longer used and none of those passed arguments are used really.

[bizley]

I did some meditation on this and I don't think this is a bug anymore. CompositeAuth should be used in order to provide several ways to authenticate a user based on one specific model. If one of the methods is using different model it should use a different entry point for this (action). Overall I think the only thing missing here is a doc statement that configuring additional parameters for auth methods in CompositeAuth is pointless.

[spmaxinc]

What would be a realistic use case for this function then? When would you develop two different authentication methods for the same front end? Why would you ever develop two different front ends for the same app that would use two different authentication strategies??

Your proposed solution of using a different controller/action will result in code duplication in my case because the code is exactly the same for two different users.

One user is a human logging into a front end to manage sales record and another is a terminal generating these sales records. They both access orders REST API endpoint expecting the same behavior.

What I will have to do is a hack. Subclass a HttpHeaderAuth and overwrite its authentication function where I would substitute $user object. Then I will continue to use CompositeAuth. I cannot add two separate AuthMethod behaviors because in that case they will have to be both satisfied at the same time in order to pass the filter.

[bizley]

Just because this is your use case it does not mean that it is the general way of doing things. I changed my mind about this being a bug because I believe that what I described was a principle behind the design here - that is why I recommend separate action for your case in 2.0 (DRY is not something we must obey no matter what) and we should change the way it behaves but in 2.2 as @schmunk42 said, to be more user friendly.

[spmaxinc]

But maybe you are right and the Yii documentation needs to be updated and say that there is only ONE user application component that is allowed. If you want to have a different user component, you need to develop a new app.

[spmaxinc]

Very interesting. I was not aware of Yii 2.2. I thought everyone was working on Yii3. This will make sense. After you elaborated on your previous response, explaining the idea of re working this in 2.2 it makes sense to me.