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

Socket_mode internals uses its own SSL context and not the one provided by the client #1175

Closed
charlch opened this issue Feb 16, 2022 · 3 comments
Assignees
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented socket-mode Version: 3x
Milestone

Comments

@charlch
Copy link
Contributor

charlch commented Feb 16, 2022

SocketModeHandler doesn't seem to use the ssl_context provided to it by the client passed to it.
I think it should use the context provided if available and fall back to the default context if not.

I'm work on a fix here: charlch@afd7cf0

Reproducible in:

pip freeze | grep slack
python --version
sw_vers && uname -v # or `ver`

The Slack SDK version

slack-sdk==3.14.1

Python runtime version

Python 3.6.5 |Man Group Plc| (default, Jul  4 2019, 14:15:57) 
[GCC 7.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.

OS info

LSB Version:	:core-4.1-amd64:core-4.1-noarch
Distributor ID:	CentOS
Description:	CentOS Linux release 7.5.1804 (Core) 
Release:	7.5.1804

Expected result:

I expect to be able to provide an SSL context for the websockets to use.
I can provide one to the WebClient:

WebClient(token=BOT_TOKEN, ssl=ssl_context)

and I want to use the same context for the websocket.

Actual result:

When the SocketModeHandler constructs a Connection it uses ssl.create_default_context() for the context:
image
and not the one I provide.

Requirements

For general questions/issues about Slack API platform or its server-side, could you submit questions at https://my.slack.com/help/requests/new instead. 🙇

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

@seratch seratch added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented socket-mode Version: 3x labels Feb 16, 2022
@seratch seratch self-assigned this Feb 16, 2022
@seratch seratch added this to the 3.15.0 milestone Feb 16, 2022
@seratch
Copy link
Member

seratch commented Feb 16, 2022

Hi @charlch, thanks a lot for taking the time to report this issue and the suggestion to fix it!

Your changes look great to me 👍 If you don't mind, could you send a pull request to this repo?

@charlch
Copy link
Contributor Author

charlch commented Feb 16, 2022

Hi @seratch, I just need get the validation, codegen, black and lint steps run and fixed before I raise the pull request.
(unless you're happy with the change here anyway: charlch@afd7cf0)
Should have some time to run those and raise the PR within the next day

@seratch
Copy link
Member

seratch commented Feb 16, 2022

Fixed by #1177 - @charlch Thanks a lot for fixing this quickly! As for the next release, #1176 is the last remaining task. Once that PR is merged, we'll ship a new minor version shortly!

@seratch seratch closed this as completed Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented socket-mode Version: 3x
Projects
None yet
Development

No branches or pull requests

2 participants