Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guzzle configuration: verify #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

DrSchimke
Copy link

I want to be able to disable the certification verification for guzzle client

I want to be able to disable the certification verification for guzzle client
@greg0ire
Copy link
Contributor

greg0ire commented Sep 2, 2016

Shouldn't you be using this value somewhere, like maybe in #32 ? Apparently it should be enough indeed

@@ -54,6 +54,9 @@ public function configure(ArrayNodeDefinition $builder)
->scalarNode('base_url')
->defaultValue('http://localhost')
->end()
->booleanNode('verify')
->defaultValue(true)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we are supposed to test our own API, which is sometimes self-signed in dev, would it be a bad practice to set ->defaultValue(false) ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think disabling the SSL cert check must be done on purpose, not by default

Copy link

@Tartare2240 Tartare2240 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple question about general good practices

@jcornide
Copy link

It would be great if this is merged, I'm using a workaround and this could be quite helpful

@forceedge01
Copy link

Can the maintainer please accept this PR? Its been open since 2016.. not cool to be honest.

@forceedge01
Copy link

For now overriding config with reflection - but honestly when the maintainer wakes up - please merge PR.

@forceedge01
Copy link

For now resorting to this:

        $api = $this->getExternalContext(WebApiContext::class);

        $reflection = new ReflectionObject($api);
        $reflectionMethod = $reflection->getMethod('getClient');
        $reflectionMethod->setAccessible(true);
        $client = $reflectionMethod->invoke($api);

        $reflection = new ReflectionObject($client);
        $reflectionProperty = $reflection->getProperty('config');
        $reflectionProperty->setAccessible(true);
        $config = $reflectionProperty->getValue($client);
        $config['verify'] = false;

        $reflectionProperty->setValue($client, $config);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants