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

auth: link slack user #2564

Merged
merged 39 commits into from
Sep 6, 2022
Merged

auth: link slack user #2564

merged 39 commits into from
Sep 6, 2022

Conversation

tony-tvu
Copy link
Collaborator

  • Identified the issue which this PR solves.
  • Read the CONTRIBUTING document.
  • Code builds clean without any errors or warnings.
  • Added appropriate tests for any new functionality.
  • All new and existing tests passed.
  • Added comments in the code, where necessary.
  • Ran 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:

  1. Alert sent to unlinked slack user

Screen Shot 2022-08-17 at 10 49 34 AM

  1. Clicking 'Link Account' button routes them to GoAlert's profile page to confirm linking

Screen Shot 2022-08-17 at 10 50 40 AM

  1. Once linking is confirmed, it'll automatically acknowledge or close the alert based on the selection at stage 1

Screen Shot 2022-08-17 at 10 51 01 AM

Screen Shot 2022-08-17 at 10 51 48 AM

  1. Linked user can now Close the alert from the slack UI

Screen Shot 2022-08-17 at 10 52 00 AM

mastercactapus and others added 30 commits June 22, 2022 09:39
Co-authored-by: Forfold <[email protected]>
Co-authored-by: Forfold <[email protected]>
Co-authored-by: mastercactapus <[email protected]>
Copy link
Member

@mastercactapus mastercactapus left a 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

@mastercactapus
Copy link
Member

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.

mastercactapus
mastercactapus previously approved these changes Aug 29, 2022
Copy link
Member

@mastercactapus mastercactapus left a 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

@tony-tvu tony-tvu merged commit 31bdea6 into master Sep 6, 2022
@tony-tvu tony-tvu deleted the slack-easy-link branch September 6, 2022 21:21
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

Successfully merging this pull request may close these issues.

3 participants