setConfig overrides constructor.

[mathieutu]

Hi,
as you can see in the following lines, the config is passed twice. Firstly to the constructor of the provider, and then with a setConfig method.

Manager/src/SocialiteWasCalled.php

Lines 129 to 140 in 1de3f3d

	protected function buildOAuth2Provider(SocialiteManager $socialite, $providerClass, $providerName)
	{
	$this->classExtends($providerClass, \Laravel\Socialite\Two\AbstractProvider::class);
	$config = $this->getConfig($providerClass, $providerName);
	$provider = $socialite->buildProvider($providerClass, $config->get());
	$provider->setConfig($config);
	return $provider;
	}

The setConfig method is just about filling the config with the passed parameters, and so, totally overriding the previous work from builProvider and provider constructor (which could be overridden by the dev)

Manager/src/ConfigTrait.php

Lines 17 to 27 in 1de3f3d

	public function setConfig(ConfigInterface $config)
	{
	$config = $config->get();
	$this->config = $config;
	$this->clientId = $config['client_id'];
	$this->clientSecret = $config['client_secret'];
	$this->redirectUrl = $config['redirect'];
	return $this;
	}

It can for example lead to this issue: #110
(actually, I've found that problem, because I had the same)

In my opinion we have to purely and simply remove the re-setting of attributes in setConfig.

I can make a quick PR to do that if you want.

Matt'

[faustbrian]

Yeah, that one has been bugging me for some time together with with the env config https://github.com/SocialiteProviders/Manager/tree/remove-env-config but I am short on time to spend more hours on it.

A PR to solve this would be very welcome.

[mathieutu]

ok, I'll do it as I'll be able to find some time.
I didn't understand what you mean by env config. The link you wrote is broken.

Do you trust in your tests? Should I consider the behaviour ok if the tests pass?

[faustbrian]

The link works fine for me. The env config is a feature that you can have your config either in the .env file or config/services.php but the issue is that reading it from the .env file makes caching of the impossible so I am going to drop that feature.

AndyWendt wrote the initial tests that are currently used, would probably be a good idea to revise them at some point but for now just make your changes and I will manually test ontop of the tests.

[xutl]

I also encountered the same problem.

[m1guelpf]

Could anyone PR this?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.