-
Notifications
You must be signed in to change notification settings - Fork 253
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
auth: link slack user #2564
auth: link slack user #2564
Conversation
Co-authored-by: Forfold <[email protected]>
Co-authored-by: Forfold <[email protected]>
Co-authored-by: Forfold <[email protected]>
Co-authored-by: Forfold <[email protected]> Co-authored-by: mastercactapus <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's what I think we have left:
- Update the messaging to be a little more informative, like:
Slack user <User.Name> (@<User.Username>) from <Team.Domain>.slack.com
- We should show errors in the dialog before closing (e.g., network issue)
- Unknown/invalid/expired tokens should have consistent messaging
- We should probably add the details in the token, or at least a signature -- it's too easy to make a URL that will link a user in a way they didn't expect; we can have a query to validate the token before the user confirms
I moved the metadata to a table column, that way, to get any info, you need a valid GoAlert session AND the token/url, and also handles ensuring the messaging the user gets is also validated and isn't subject to a malicious/misleading link. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving integration test out-of-scope; we'll add that as a test when the mockslack server gains a UI
make check
to catch common errors. Fixed any that came up.Description:
This PR facilitates linking slack users to their GoAlert accounts, which would allow them to acknowledge or close an alert from the Slack UI once linked.
Out of Scope:
Devtool's mock slack server has been updated to account for block elements
Screenshots: