diff --git a/base/server.go b/base/server.go index daf71ebb..cbfba6ac 100644 --- a/base/server.go +++ b/base/server.go @@ -67,6 +67,24 @@ func (s *Server) Name() string { return s.name } +// RunBot runs a bot to completion and reports any fatal error to stdout and, +// when Start has configured chat error reporting, to the error conversation. +func RunBot(s *Server, run func() error) int { + if err := run(); err != nil { + s.ReportFatalError(err) + return 3 + } + return 0 +} + +func (s *Server) ReportFatalError(err error) { + fmt.Printf("error running chat loop: %v\n", err) + if s.DebugOutput == nil { + return + } + s.Report("```fatal error running %s: %v```", s.name, err) +} + func (s *Server) SetBotAdmins(admins []string) { s.botAdmins = admins } diff --git a/canarybot/main.go b/canarybot/main.go index 44e58ff5..c5c05f00 100644 --- a/canarybot/main.go +++ b/canarybot/main.go @@ -101,9 +101,5 @@ func mainInner() int { return 3 } bs := NewBotServer(*opts) - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %s\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) } diff --git a/elastiwatch/main.go b/elastiwatch/main.go index 4ef2347e..70f64b3e 100644 --- a/elastiwatch/main.go +++ b/elastiwatch/main.go @@ -206,9 +206,5 @@ func mainInner() int { opts.AlertConvID = chat1.ConvIDStr(alertConvID) opts.EmailConvID = chat1.ConvIDStr(emailConvID) bs := NewBotServer(*opts) - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %v\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) } diff --git a/gcalbot/main.go b/gcalbot/main.go index fc8dee3f..5eef72eb 100644 --- a/gcalbot/main.go +++ b/gcalbot/main.go @@ -259,10 +259,5 @@ func mainInner() int { return 3 } bs := NewBotServer(*opts) - - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %s\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) } diff --git a/githubbot/main.go b/githubbot/main.go index d86194d0..bbf8654b 100644 --- a/githubbot/main.go +++ b/githubbot/main.go @@ -293,9 +293,5 @@ func mainInner() int { return 3 } bs := NewBotServer(*opts) - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %v\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) } diff --git a/gitlabbot/db.sql b/gitlabbot/db.sql index 7be267cb..e6e06431 100644 --- a/gitlabbot/db.sql +++ b/gitlabbot/db.sql @@ -2,5 +2,6 @@ CREATE TABLE `subscriptions` ( `conv_id` char(64) NOT NULL, `repo` varchar(128) NOT NULL, `oauth_identifier` varchar(128) NOT NULL, + `reauthorization_needed` boolean NOT NULL DEFAULT false, UNIQUE KEY unique_subscription (`conv_id`, `repo`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; diff --git a/gitlabbot/gitlabbot/db.go b/gitlabbot/gitlabbot/db.go index c96c1c14..37b01888 100644 --- a/gitlabbot/gitlabbot/db.go +++ b/gitlabbot/gitlabbot/db.go @@ -3,6 +3,7 @@ package gitlabbot import ( "context" "database/sql" + "fmt" "github.com/keybase/go-keybase-chat-bot/kbchat/types/chat1" @@ -14,6 +15,11 @@ type DB struct { *base.DB } +type SubscribedConv struct { + ConvID chat1.ConvIDStr + ReauthorizationNeeded bool +} + func NewDB(db *sql.DB) *DB { return &DB{ DB: base.NewDB(db), @@ -49,23 +55,22 @@ func (d *DB) DeleteSubscriptionsForRepo(ctx context.Context, convID chat1.ConvID return err } -func (d *DB) GetSubscribedConvs(ctx context.Context, repo string) (res []chat1.ConvIDStr, err error) { +func (d *DB) GetSubscribedConvs(ctx context.Context, repo string) (res []SubscribedConv, err error) { rows, err := d.QueryContext(ctx, ` - SELECT conv_id + SELECT conv_id, reauthorization_needed FROM subscriptions WHERE repo = ? - GROUP BY conv_id `, repo) if err != nil { return res, err } defer rows.Close() for rows.Next() { - var convID chat1.ConvIDStr - if err := rows.Scan(&convID); err != nil { + var subscribedConv SubscribedConv + if err := rows.Scan(&subscribedConv.ConvID, &subscribedConv.ReauthorizationNeeded); err != nil { return res, err } - res = append(res, convID) + res = append(res, subscribedConv) } return res, rows.Err() } @@ -89,22 +94,44 @@ func (d *DB) GetSubscriptionExists(ctx context.Context, convID chat1.ConvIDStr, } } -func (d *DB) GetSubscriptionForRepoExists(ctx context.Context, convID chat1.ConvIDStr, repo string) (exists bool, err error) { +func (d *DB) GetSubscriptionForRepoStatus(ctx context.Context, convID chat1.ConvIDStr, repo string) ( + exists bool, reauthorizationNeeded bool, err error, +) { row := d.QueryRowContext(ctx, ` - SELECT 1 + SELECT reauthorization_needed FROM subscriptions WHERE (conv_id = ? AND repo = ?) `, convID, repo) - var rowRes string - err = row.Scan(&rowRes) + err = row.Scan(&reauthorizationNeeded) switch err { case sql.ErrNoRows: - return false, nil + return false, false, nil case nil: - return true, nil + return true, reauthorizationNeeded, nil default: - return false, err + return false, false, err + } +} + +func (d *DB) CompleteSubscriptionReauthorization( + ctx context.Context, convID chat1.ConvIDStr, repo string, +) error { + res, err := d.ExecContext(ctx, ` + UPDATE subscriptions + SET reauthorization_needed = false + WHERE conv_id = ? AND repo = ? AND reauthorization_needed = true + `, convID, repo) + if err != nil { + return err + } + rowsAffected, err := res.RowsAffected() + if err != nil { + return err + } + if rowsAffected != 1 { + return fmt.Errorf("expected to complete one subscription reauthorization, updated %d", rowsAffected) } + return nil } func (d *DB) GetAllSubscriptionsForConvID(ctx context.Context, convID chat1.ConvIDStr) (res []string, err error) { diff --git a/gitlabbot/gitlabbot/handler.go b/gitlabbot/gitlabbot/handler.go index aeb6da1d..c4663859 100644 --- a/gitlabbot/gitlabbot/handler.go +++ b/gitlabbot/gitlabbot/handler.go @@ -101,13 +101,23 @@ func (h *Handler) handleSubscribe(ctx context.Context, cmd string, msg chat1.Msg return nil } - alreadyExists, err := h.db.GetSubscriptionForRepoExists(ctx, msg.ConvID, repo) + subscriptionFound, reauthorizationNeeded, err := h.db.GetSubscriptionForRepoStatus(ctx, msg.ConvID, repo) if err != nil { return fmt.Errorf("error checking subscription: %s", err) } if create { - if !alreadyExists { + if subscriptionFound && reauthorizationNeeded { + _, err = h.kbc.SendMessageByTlfName(msg.Sender.Username, "%s", formatReauthorizationInstructions(repo, hostedURL, msg, h.httpPrefix, h.secret)) + if err != nil { + return fmt.Errorf("error sending message: %s", err) + } + if !base.IsDirectPrivateMessage(h.kbc.GetUsername(), msg.Sender.Username, msg.Channel) { + h.ChatEcho(msg.ConvID, "OK! I've sent instructions to @%s to reauthorize the webhook.", msg.Sender.Username) + } + return nil + } + if !subscriptionFound { err = h.db.CreateSubscription(ctx, msg.ConvID, repo, base.IdentifierFromMsg(msg)) if err != nil { return fmt.Errorf("error creating subscription: %s", err) @@ -126,7 +136,7 @@ func (h *Handler) handleSubscribe(ctx context.Context, cmd string, msg chat1.Msg return nil } - if alreadyExists { + if subscriptionFound { err = h.db.DeleteSubscriptionsForRepo(ctx, msg.ConvID, repo) if err != nil { return fmt.Errorf("error deleting subscriptions: %s", err) diff --git a/gitlabbot/gitlabbot/http.go b/gitlabbot/gitlabbot/http.go index 2e2c195f..f357340e 100644 --- a/gitlabbot/gitlabbot/http.go +++ b/gitlabbot/gitlabbot/http.go @@ -129,12 +129,18 @@ func (h *HTTPSrv) handleWebhook(_ http.ResponseWriter, r *http.Request) { return } - for _, convID := range convs { - secretToken := base.MakeSecret(repo, convID, h.secret) + for _, conv := range convs { + secretToken := base.MakeSecret(repo, conv.ConvID, h.secret) if !hmac.Equal([]byte(signature), []byte(secretToken)) { - h.Debug("payload signature mismatch for conversation %s", convID) + h.Debug("payload signature mismatch for conversation %s", conv.ConvID) continue } - h.ChatEcho(convID, "%s", message) + if conv.ReauthorizationNeeded { + if err := h.db.CompleteSubscriptionReauthorization(ctx, conv.ConvID, repo); err != nil { + h.Errorf("Error completing webhook reauthorization for conversation %s: %s", conv.ConvID, err) + continue + } + } + h.ChatEcho(conv.ConvID, "%s", message) } } diff --git a/gitlabbot/gitlabbot/util.go b/gitlabbot/gitlabbot/util.go index cb5fdaa9..c53f4c0d 100644 --- a/gitlabbot/gitlabbot/util.go +++ b/gitlabbot/gitlabbot/util.go @@ -75,6 +75,16 @@ Happy coding!`, return message } +func formatReauthorizationInstructions(repo string, hostedURL string, msg chat1.MsgSummary, httpAddress string, secret string) string { + back := "`" + return fmt.Sprintf(` +To reauthorize notifications, go to %s/%s/hooks and edit the existing webhook for %s%s/gitlabbot/webhook%s. +Replace its “Secret Token” with %s%s%s. + +Happy coding!`, + hostedURL, repo, back, httpAddress, back, back, base.MakeSecret(repo, msg.ConvID, secret), back) +} + // parseRepoInput checks if url or form func parseRepoInput(urlOrRepoPath string) (hostedURL string, repo string, err error) { urlOrRepoPath = strings.TrimSuffix(urlOrRepoPath, ".git") diff --git a/gitlabbot/gitlabbot/util_test.go b/gitlabbot/gitlabbot/util_test.go index 70887c5b..1dc7404f 100644 --- a/gitlabbot/gitlabbot/util_test.go +++ b/gitlabbot/gitlabbot/util_test.go @@ -3,9 +3,26 @@ package gitlabbot import ( "testing" + "github.com/keybase/go-keybase-chat-bot/kbchat/types/chat1" + "github.com/keybase/managed-bots/base" "github.com/stretchr/testify/require" ) +func TestFormatReauthorizationInstructions(t *testing.T) { + msg := chat1.MsgSummary{ConvID: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"} + repo := "owner/repo" + hostedURL := "https://gitlab.example.com" + httpAddress := "https://bots.example.com" + secret := "server-secret" + + res := formatReauthorizationInstructions(repo, hostedURL, msg, httpAddress, secret) + + require.Contains(t, res, "https://gitlab.example.com/owner/repo/hooks") + require.Contains(t, res, "edit the existing webhook") + require.Contains(t, res, "https://bots.example.com/gitlabbot/webhook") + require.Contains(t, res, base.MakeSecret(repo, msg.ConvID, secret)) +} + func TestParseRepoInputWithURL(t *testing.T) { httpPrefix := "https://mywebsite.com" urlRepo := "owner/repo" diff --git a/gitlabbot/main.go b/gitlabbot/main.go index 64ed967b..4da82851 100644 --- a/gitlabbot/main.go +++ b/gitlabbot/main.go @@ -192,9 +192,5 @@ func mainInner() int { return 3 } bs := NewBotServer(*opts) - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %v\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) } diff --git a/macrobot/main.go b/macrobot/main.go index 7775658b..41f9b4e0 100644 --- a/macrobot/main.go +++ b/macrobot/main.go @@ -139,9 +139,5 @@ func mainInner() int { return 3 } bs := NewBotServer(*opts) - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %s\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) } diff --git a/meetbot/main.go b/meetbot/main.go index adbcd6e0..a75f6a5b 100644 --- a/meetbot/main.go +++ b/meetbot/main.go @@ -151,9 +151,5 @@ func mainInner() int { } bs := NewBotServer(*opts) - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %s\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) } diff --git a/pollbot/main.go b/pollbot/main.go index 30a78240..df241509 100644 --- a/pollbot/main.go +++ b/pollbot/main.go @@ -156,9 +156,5 @@ func mainInner() int { return 3 } bs := NewBotServer(*opts) - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %v\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) } diff --git a/triviabot/main.go b/triviabot/main.go index f2382e35..7a5831e5 100644 --- a/triviabot/main.go +++ b/triviabot/main.go @@ -116,9 +116,5 @@ func mainInner() int { return 3 } bs := NewBotServer(*opts) - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %s\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) } diff --git a/webhookbot/main.go b/webhookbot/main.go index 400500ab..3a676f17 100644 --- a/webhookbot/main.go +++ b/webhookbot/main.go @@ -154,9 +154,5 @@ func mainInner() int { return 3 } bs := NewBotServer(*opts) - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %s\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) } diff --git a/zoombot/main.go b/zoombot/main.go index 7bd50a99..15b739a8 100644 --- a/zoombot/main.go +++ b/zoombot/main.go @@ -186,9 +186,5 @@ func mainInner() int { } bs := NewBotServer(*opts) - if err := bs.Go(); err != nil { - fmt.Printf("error running chat loop: %s\n", err) - return 3 - } - return 0 + return base.RunBot(bs.Server, bs.Go) }