Skip to content
This repository has been archived by the owner on Dec 7, 2020. It is now read-only.

Adding config option to skip tls verify for OpenID provider #147

Conversation

johanneslanger
Copy link
Contributor

This is in response to issue #146

Did this fix today so we are unblocked. Would love to get this in. Let me know what you think!

The config param defaults to false as this could be a security issue if set to true in production.
Did not find a quick way to write test for this though :(

Let me know what you think!

Copy link
Contributor

@gambol99 gambol99 left a comment

Choose a reason for hiding this comment

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

Other than the one change ... LGTM

@@ -115,6 +115,8 @@ type Config struct {
RedirectionURL string `json:"redirection-url" yaml:"redirection-url"`
// RevocationEndpoint is the token revocation endpoint to revoke refresh tokens
RevocationEndpoint string `json:"revocation-url" yaml:"revocation-url"`
// skips the tls verification for openid provider communication
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs the variable name added to the comment ...as it's exported (go-vet will complain) .. i.e.
// SkipOpenIDProviderTLSVerify skip ...

@johanneslanger johanneslanger force-pushed the feature/skip_openid_provider_tls_verify branch from 3b0f2e3 to 86fdd1d Compare November 8, 2016 13:00
@johanneslanger
Copy link
Contributor Author

Ok, updated the comment to include the variable name!

@gambol99 gambol99 merged commit df3a932 into louketo:master Nov 9, 2016
@gambol99
Copy link
Contributor

gambol99 commented Nov 9, 2016

thank you for the fix @johanneslanger :-)

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

Successfully merging this pull request may close these issues.

3 participants