ConfigResolver overwriting

[antonkomarev]

Issue / Motivation:

I can't store configuration of provider in services config file.
Because of it I'm receiving MissingConfigException:

There is no services entry for github

I've tried to replace ConfigResolver with my own implementation.
Added to App\Providers\AppServiceProvider:

$this->app->singleton(
    \SocialiteProviders\Manager\Contracts\Helpers\ConfigRetrieverInterface::class,
    \App\CustomConfigRetriever::class
);

But no luck, because ServiceProvider of the package loading after the AppServiceProvider and overwrites my binding.

Proposed solution:

It looks like I have to disable auto-discovery of "socialiteproviders/manager" in composer.json and load service provider manually, and only after that rebind ConfigResolver. Is there any other thoughts how this could be solved?

[m1guelpf]

Can you store a mock configuration and replace it when redirecting?

[antonkomarev]

Yes, but how can I do it if it's automatically tries to resolve config from services?

[antonkomarev]

Disabling discovery and registering provider manually has negative side effects. Then SocialiteWasCalled event will be fired on every application call, even when it wasn't called.
Even with $this->app->registerDeferredProvider() 😢

[antonkomarev]

Nice, we've got unreachable statement. And that's exactly what I need. Fill provider with spoofed config to overwrite it later.

Manager/src/SocialiteWasCalled.php

Lines 150 to 165 in f870574

	protected function getConfig($providerClass, $providerName)
	{
	try {
	return resolve(ConfigRetrieverInterface::class)->fromServices(
	$providerName, $providerClass::additionalConfigKeys()
	);
	} catch (MissingConfigException $e) {
	throw new MissingConfigException($e->getMessage());
	}
	return new Config(
	$this->spoofedConfig['client_id'],
	$this->spoofedConfig['client_secret'],
	$this->spoofedConfig['redirect']
	);
	}

Why do we need to check and fill real config on registering providers in Socialite, and not on the first call? It will check config for each provider and configure each one even if we don't use it. It might be a candidate for refactoring for the next major release.

[m1guelpf]

Honestly, I'm not entirely familiar with the functionality of that trait, I wasn't the one who wrote it, unfortunately.

I've added a deep dive to my todo list. Meanwhile, and fix to this projects that doesn't break existing apps would work for me :)

[antonkomarev]

That's hidden not in the trait. It's event which called when Socialite is required. I've found more issues and described them in other threads. Will try to help you out later.

[antonkomarev]

Additionally I've found some more issues:

1. There is unused class property:

private $configRetriever;

2. resolve helper method not found (that's why tests are failing). I don't know why it was used instead of DI to __construct method which was used before.

3. Catching exception and throw it again is pretty strange thing:

try {
    return resolve(ConfigRetrieverInterface::class)->fromServices(
        $providerName, $providerClass::additionalConfigKeys()
    );
} catch (MissingConfigException $e) {
    throw new MissingConfigException($e->getMessage());
}

Need some more time to investigate all the required changes and will create a PR with it.

[m1guelpf]

Awesome, thanks for all your work here!