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

Add support for corporate internet proxy #6469

Closed
tjuerge opened this Issue May 20, 2017 · 7 comments

Comments

Projects
None yet
2 participants
@tjuerge
Copy link
Contributor

tjuerge commented May 20, 2017

Summary

Running MM behind a corporate firewall is not possible due to missing / incomplete internet proxy support. In a forum entry this issue is mentioned as well.

Steps to reproduce

MM bundled with GitLab CE 9.1.4

Expected behavior

MM should pickup internet proxy URL from environment ("http_proxy" OR "HTTP_PROXY") and use this for initialization of all http.Client connections. Currently the creation of http.Client connections is implemented inconsistently. Only in post.go a proxy defined via "HTTP_PROXY" is recognised.

Observed behavior

Here the following functions are affected:

  • usage tracking via segment
    2017-05-19_12:22:21.74144 segment 2017/05/19 12:22:21 error sending request: Post https://api.segment.io/v1/batch: dial tcp 35.163.223.46:443: i/o timeout
    
  • token request during OAuth signup
    [2017/05/17 13:27:00 UTC] [EROR] /signup/gitlab/complete:AuthorizeOAuthUser code=500 rid=j51tb4tjkp87zy9sc3mjg6zdoh uid= ip=x.x.x.x, x.x.x.x Token request failed [details: Post https://<yourdomain>/oauth/token: dial tcp x.x.x.x:443: getsockopt: connection timed out]
    
  • mobile push notifcation
    [2017/05/19 12:11:29 UTC] [EROR] Device push reported as error for UserId=5xn17iuzupdubjssnz4dp1drqh SessionId=88okheurwirezxwimbf4pxpd7r message=Post http://push-test.mattermost.com/api/v1/send_push: dial tcp 54.152.221.224:80: getsockopt: connection timed out
    

Possible fixes

All source code where a http.Client instance is created (e.g. oauth.go or notification.go) should reuse a common implementation (like the one used in post.go).

@tjuerge

This comment has been minimized.

Copy link
Contributor Author

tjuerge commented May 21, 2017

I'm not used to go (lang) yet, but from my understanding of client.go and transport.go this can be fixed by adding a single line to the initialisation of Transport:

Proxy: http.ProxyFromEnvironment,

So the modified version of notification.go would look like this:

tr := &http.Transport{
	TLSClientConfig:   &tls.Config{InsecureSkipVerify: utils.Cfg.ServiceSettings.EnableInsecureOutgoingConnections},
	DisableKeepAlives: true,
	Proxy:             http.ProxyFromEnvironment,
}
httpClient := &http.Client{Transport: tr}
@enahum

This comment has been minimized.

Copy link
Member

enahum commented May 22, 2017

Hi @tjuerge thanks for bringing this up, we have already a ticket for implementing this everywhere, you can track it here https://mattermost.atlassian.net/browse/PLT-5705

@tjuerge

This comment has been minimized.

Copy link
Contributor Author

tjuerge commented May 22, 2017

@enahum This is good to know.
Is there any ETA for implementing PLT-5705? Right now MM is not usable within a corporate environment (firewall) due to this issue.

@tjuerge

This comment has been minimized.

Copy link
Contributor Author

tjuerge commented May 22, 2017

@enahum And please make sure that PLT-5705 leverages net.ProxyFromEnvironment (instead of handling only a subset of the different proxy-related env vars as done in post.go).

@enahum

This comment has been minimized.

Copy link
Member

enahum commented May 25, 2017

Sure we'll take it into consideration.

Wondering if you would like to work on this and make a pull request?

@tjuerge

This comment has been minimized.

Copy link
Contributor Author

tjuerge commented May 26, 2017

@enahum Created PR #6503 with implementation for PLT-5705

@tjuerge

This comment has been minimized.

Copy link
Contributor Author

tjuerge commented Jun 2, 2017

Fixed via merge of PR #6503

@tjuerge tjuerge closed this Jun 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment