Allow CompositeAuth auth methods to use their own user if defined

[mtangoo]

Q	A
Is bugfix?	✔️
New feature?	❌
Breaks BC?	❌
Fixed issues	#20238

[codecov]

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 64.84%. Comparing base (fd866da) to head (e9f7a54).

Files with missing lines	Patch %	Lines
framework/filters/auth/CompositeAuth.php	60.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #20308      +/-   ##
============================================
- Coverage     64.84%   64.84%   -0.01%     
- Complexity    11424    11427       +3     
============================================
  Files           431      431              
  Lines         37172    37177       +5     
============================================
+ Hits          24106    24109       +3     
- Misses        13066    13068       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bizley]

I see this was discussed and proposed to be changed in 2.2. Are we doing this in 2.0 anyway?

[samdark]

@bizley I don't see any compatibility issues with this change. Are there any?

[mtangoo]

I see this was discussed and proposed to be changed in 2.2. Are we doing this in 2.0 anyway?

I think in 2.2 there will be a need to look at the API design itself and if necessary make breaking change. This is a "painkiller" approach to just fix the issue without any breaking changes

[bizley]

With this change user set in the inner auth method will be used for it, now we are using user set in the composite auth.

[mtangoo]

With this change user set in the inner auth method will be used for it, now we are using user set in the composite auth.

Only if is set. Otherwise it works as it used to!

[bizley]

With this change user set in the inner auth method will be used for it, now we are using user set in the composite auth.

Only if is set. Otherwise it works as it used to!

Yes, but now even if user is set in one of the methods it is ignored.

[mtangoo]

With this change user set in the inner auth method will be used for it, now we are using user set in the composite auth.

Only if is set. Otherwise it works as it used to!

Yes, but now even if user is set in one of the methods it is ignored.

Isn't that the purpose of defining user in auth method, that you want it to override anything else defined?

[bizley]

With this change user set in the inner auth method will be used for it, now we are using user set in the composite auth.

Only if is set. Otherwise it works as it used to!

Yes, but now even if user is set in one of the methods it is ignored.

Isn't that the purpose of defining user in auth method, that you want it to override anything else defined?

I agree but this is BC break as was stated in #20238

[mtangoo]

I take it as a bug, because otherwise what is the point of an API allowing devs to pass user object of their choice if it ends up being discarded?

IMO, this is a bug that wasn't uncovered for so long.