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

Connection failure to the Kubernetes Slack channel #699

Closed
max-arnold opened this issue Jun 11, 2019 · 12 comments
Closed

Connection failure to the Kubernetes Slack channel #699

max-arnold opened this issue Jun 11, 2019 · 12 comments

Comments

@max-arnold
Copy link

max-arnold commented Jun 11, 2019

Latest wee-slack (e00965c) can't connect to the Kubernetes community chat (using a 106-character xoxs-* token from a browser): https://slack.k8s.io/

The connection error message is:

ERROR: Failed connecting to Slack with token starting with xoxs-XXXXXXXXXX: rtm_connect_required

There is a note in the API docs https://api.slack.com/methods/rtm.start:

rtm_connect_required

For large teams please use rtm.connect instead of rtm.start.

WeeChat 2.4

@auscompgeek
Copy link
Contributor

AFAIK, wee-slack currently badly handles large workspaces. rtm_connect_required seems relatively new though.

@trygveaa
Copy link
Member

There was another user on IRC reporting the same issue in late April, so apparently this is an issue for some teams. Not sure which teams it affects, maybe it's caused by the size, or possibly something else.

We would have to do some rewriting to support this. I notice that in addition to not supporting rtm.start, this team doesn't support listing all the users without pagination. So this might be a somewhat large task.

In general (as long as the API endpoints wee-slack uses works for the team), wee-slack handles fairly large teams pretty well in my experience. I am for example on a team with 11000 users and 16000 channels without any particular issues.

@dtantsur
Copy link

Seems still an issue:

17:20 ERROR: Failed connecting to Slack with token xoxs-<...>: rtm_connect_required

which is quite sad because I really hate their web UI :(

@trygveaa
Copy link
Member

trygveaa commented Aug 2, 2020

@dtantsur: Yes, that's why this issue is still open, it hasn't been fixed.

Unfortunately, I think there is some info we can't get from any other API calls than rtm.start, so I'm not sure if this is possible to fix without some loss of functionality. I will have to investigate it further. I'm sorry to say that this is not a priority for me at the moment. If anyone else wants to check out what would be the required changes and if it's possible to fix it without any loss of functionality, and comment here or make a PR, it would be very welcome.

@sudoforge
Copy link
Contributor

The k8s slack org restricts application installation and the admins reject requests for wee-slack due to the exposure of email addresses by the /whois command. These are not visible in user's profiles in the kubernetes slack, and this is the reason given to me by two different admins in a thread a while ago, when I last asked.

trygveaa added a commit that referenced this issue Jan 30, 2022
rtm.start is deprecated and will stop working on September 20, 2022.
This patch replaces it with several other API endpoints to get the info
we need.

The old code to use rtm.start is still kept because xoxs tokens doesn't
work with the users.conversations API endpoint, but it will probably be
removed soon (it's not possible to get new xoxs tokens anyways).

This is a necessary step for #699 and #844
trygveaa added a commit that referenced this issue Jan 30, 2022
rtm.start is deprecated and will stop working on September 20, 2022.
This patch replaces it with several other API endpoints to get the info
we need.

The old code to use rtm.start is still kept because xoxs tokens doesn't
work with the users.conversations API endpoint, but it will probably be
removed soon (it's not possible to get new xoxs tokens anyways).

This is a necessary step for #699 and #844
trygveaa added a commit that referenced this issue Jan 30, 2022
rtm.start is deprecated and will stop working on September 20, 2022.
This patch replaces it with several other API endpoints to get the info
we need.

The handle_rtmstart method is kept for the test setup to work, but this
setup should also be replaced with the new API endpoints.

This is a necessary step for #699 and #844
trygveaa added a commit that referenced this issue Jan 31, 2022
rtm.start is deprecated and will stop working on September 20, 2022.
This patch replaces it with several other API endpoints to get the info
we need.

The handle_rtmstart method is kept for the test setup to work, but this
setup should also be replaced with the new API endpoints.

This is a necessary step for #699 and #844
trygveaa added a commit that referenced this issue Sep 17, 2022
rtm.start is deprecated and will stop working on September 20, 2022.
This patch replaces it with several other API endpoints to get the info
we need.

The handle_rtmstart method is kept for the test setup to work, but this
setup should also be replaced with the new API endpoints.

This is a necessary step for #699 and #844
@trygveaa
Copy link
Member

trygveaa commented Oct 4, 2022

This has been fixed with the v2.9.0 release. Note that you have to use a session token since the wee-slack app is not approved.

The startup is very slow though because the way wee-slack currently works it requires all users to be loaded on start and this workspace has a ton of users. I tested now and it took 8 minutes for me. This is something I'm working on addressing, but it will probably take a while.

The k8s slack org restricts application installation and the admins reject requests for wee-slack due to the exposure of email addresses by the /whois command. These are not visible in user's profiles in the kubernetes slack, and this is the reason given to me by two different admins in a thread a while ago, when I last asked.

This doesn't make any sense. wee-slack only exposes what's already available from the Slack APIs of course. And I tested now, and the email address is not shown by the /whois command for the users I tested.

@trygveaa trygveaa closed this as completed Oct 4, 2022
@sudoforge
Copy link
Contributor

This doesn't make any sense. wee-slack only exposes what's already available from the Slack APIs of course.

I agree with the logic here; that's just the reasoning I was given by the admins of that particular workspace when requesting the installation of wee-slack back in 2020.

And I tested now, and the email address is not shown by the /whois command for the users I tested.

Perhaps Slack has removed this info from the endpoint that's called? I don't follow their API changes, but I do know that when I made that comment, I was able to view user's email addresses in other workspaces where wee-slack had been installed and a session token wasn't being used.

@trygveaa
Copy link
Member

trygveaa commented Oct 4, 2022

I agree with the logic here; that's just the reasoning I was given by the admins of that particular workspace when requesting the installation of wee-slack back in 2020.

Yes, of course, the comment wasn't directed at you, just at the statement. Sorry if it came across a bit crass.

Perhaps Slack has removed this info from the endpoint that's called? I don't follow their API changes, but I do know that when I made that comment, I was able to view user's email addresses in other workspaces where wee-slack had been installed and a session token wasn't being used.

Yes, email addresses are normally shown by /whois, no matter what token type you use. However, since Kubernetes has restricted viewing of it, it's not available from the API when used against that workspace, so therefore not shown by wee-slack either. I would be surprised if this changes if the wee-slack app is approved.

@sudoforge
Copy link
Contributor

Crass? Not what I took your comment as, at all :)

However, since Kubernetes has restricted viewing of it, it's not available from the API when used against that workspace

That must be a new administration setting. I've moved the communities I managed away from Slack some time ago, so I haven't had administrative access to a workspace for quite some time, but I certainly don't remember that being configurable for a workspace.

@trygveaa
Copy link
Member

trygveaa commented Oct 4, 2022

That must be a new administration setting. I've moved the communities I managed away from Slack some time ago, so I haven't had administrative access to a workspace for quite some time, but I certainly don't remember that being configurable for a workspace.

No idea if it's new or not, but this is the setting:

image

But huh, I tested this setting and I see that with it set to "No one" the email address is not available from the API anymore when using session tokens, but it's still available when using OAuth tokens. That was surprising to me, but I see that they actually warn about it there. Seems like a weird choice by Slack to me.

So then what the admins said that email addresses will be shown in /whois if wee-slack is approved is actually correct.

Anyways, since you can use session tokens, it shouldn't really be an issue if the app isn't and won't be approved.

@trygveaa
Copy link
Member

trygveaa commented Oct 4, 2022

But huh, I tested this setting and I see that with it set to "No one" the email address is not available from the API anymore when using session tokens, but it's still available when using OAuth tokens. That was surprising to me, but I see that they actually warn about it there. Seems like a weird choice by Slack to me.

So then what the admins said that email addresses will be shown in /whois if wee-slack is approved is actually correct.

Strike that, there must have been a delay or some caching. When testing now, email addresses are not available using OAuth tokens anymore. So then it should be okay to approve the app.

Edit: See below.

@trygveaa
Copy link
Member

trygveaa commented Oct 4, 2022

Strike that, there must have been a delay or some caching. When testing now, email addresses are not available using OAuth tokens anymore. So then it should be okay to approve the app.

Eh, sorry, I mixed up the responses, the email addresses are still available using OAuth tokens, so see the original comment...

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

No branches or pull requests

5 participants