diff --git a/base/oauth.go b/base/oauth.go index 60818e8c..519c5660 100644 --- a/base/oauth.go +++ b/base/oauth.go @@ -22,6 +22,19 @@ func (e OAuthRequiredError) Error() string { return "OAuth is required for this, permission requested." } +// ShouldRetryAuth checks if an error indicates OAuth credentials have failed +// and should be deleted to trigger re-authentication. This consolidates the +// retry logic used across meetbot, zoombot, and gcalbot. +func ShouldRetryAuth(err error) bool { + if err == nil { + return false + } + errMsg := err.Error() + return strings.Contains(errMsg, "cannot fetch token") || + strings.Contains(errMsg, "invalid_grant") || + strings.Contains(errMsg, "token expired and refresh token is not set") +} + type OAuthStorage interface { GetToken(ctx context.Context, identifier string) (*oauth2.Token, error) PutToken(ctx context.Context, identifier string, token *oauth2.Token) error diff --git a/gcalbot/gcalbot/account.go b/gcalbot/gcalbot/account.go index a60dce23..54057be9 100644 --- a/gcalbot/gcalbot/account.go +++ b/gcalbot/gcalbot/account.go @@ -11,6 +11,7 @@ import ( "google.golang.org/api/googleapi" "github.com/keybase/go-keybase-chat-bot/kbchat/types/chat1" + "github.com/keybase/managed-bots/base" "google.golang.org/api/calendar/v3" "google.golang.org/api/option" ) @@ -89,32 +90,37 @@ func (h *Handler) deleteAccount(ctx context.Context, keybaseUsername, accountNic return fmt.Errorf("error getting account: %s", err) } - srv, err := GetCalendarService(ctx, account, h.oauth, h.db) - if err != nil { - return err - } - - channels, err := h.db.GetChannelListByAccount(ctx, account) - if err != nil { - return err - } + srv, err := h.GetCalendarServiceWithRetry(ctx, account) + if err == nil { + // Successfully got service, stop all channels before deleting + channels, err := h.db.GetChannelListByAccount(ctx, account) + if err != nil { + return err + } - for _, channel := range channels { - err := srv.Channels.Stop(&calendar.Channel{ - Id: channel.ChannelID, - ResourceId: channel.ResourceID, - }).Do() - switch err := err.(type) { - case nil: - case *googleapi.Error: - if err.Code == 404 { - // if the channel wasn't found, continue - continue + for _, channel := range channels { + err := srv.Channels.Stop(&calendar.Channel{ + Id: channel.ChannelID, + ResourceId: channel.ResourceID, + }).Do() + switch err := err.(type) { + case nil: + case *googleapi.Error: + if err.Code == 404 { + // if the channel wasn't found, continue + continue + } + return err + default: + return err } - return err - default: - return err } + } else if _, ok := err.(AccountAuthError); ok { + // Auth already failed, can't stop channels but continue with deletion + h.Debug("skipping channel cleanup for %s/%s due to auth failure", keybaseUsername, accountNickname) + } else { + // Unexpected error + return err } // cascading delete of account, oauth, subscriptions, channels and invites @@ -129,14 +135,48 @@ func GetCalendarService(ctx context.Context, account *Account, config *oauth2.Co if err != nil { return nil, err } - if newToken.AccessToken != account.Token.AccessToken { - account.Token = *newToken - err = db.InsertAccount(ctx, *account) - if err != nil { - return nil, fmt.Errorf("unable to update account token: %s", err) - } + account.Token = *newToken + err = db.InsertAccount(ctx, *account) + if err != nil { + return nil, fmt.Errorf("unable to update account token: %s", err) } } client := config.Client(ctx, &account.Token) return calendar.NewService(ctx, option.WithHTTPClient(client)) } + +// GetCalendarServiceWithRetry wraps GetCalendarService and handles auth failures +// by deleting invalid credentials. Returns AccountAuthError if credentials were deleted. +func (h *Handler) GetCalendarServiceWithRetry(ctx context.Context, account *Account) (*calendar.Service, error) { + srv, err := GetCalendarService(ctx, account, h.oauth, h.db) + if err != nil && base.ShouldRetryAuth(err) { + h.Errorf("auth failed for %s/%s, deleting credentials: %v", account.KeybaseUsername, account.AccountNickname, err) + if delErr := h.db.DeleteAccount(ctx, account.KeybaseUsername, account.AccountNickname); delErr != nil { + h.Errorf("failed to delete account after auth error: %v", delErr) + } + return nil, AccountAuthError{ + Username: account.KeybaseUsername, + Nickname: account.AccountNickname, + } + } + return srv, err +} + +// handleAuthError sends a reconnection message to the user for AccountAuthError +func (h *Handler) handleAuthError(err error, accountNickname string, convID chat1.ConvIDStr) error { + if _, ok := err.(AccountAuthError); !ok { + return err + } + h.ChatEcho(convID, "Your account '%s' needs to be reconnected. Please run `!gcal accounts connect %s` again.", accountNickname, accountNickname) + return nil +} + +// handleAuthErrorDM sends a reconnection message via DM for AccountAuthError +func (h *Handler) handleAuthErrorDM(err error, account *Account) error { + if _, ok := err.(AccountAuthError); !ok { + return err + } + _, sendErr := h.kbc.SendMessageByTlfName(account.KeybaseUsername, + "Your account '%s' needs to be reconnected. Please run `!gcal accounts connect %s` again.", account.AccountNickname, account.AccountNickname) + return sendErr +} diff --git a/gcalbot/gcalbot/calendar.go b/gcalbot/gcalbot/calendar.go index f2e9e990..c5d2cf34 100644 --- a/gcalbot/gcalbot/calendar.go +++ b/gcalbot/gcalbot/calendar.go @@ -25,9 +25,9 @@ func (h *Handler) handleCalendarsList(ctx context.Context, msg chat1.MsgSummary, return nil } - srv, err := GetCalendarService(ctx, account, h.oauth, h.db) + srv, err := h.GetCalendarServiceWithRetry(ctx, account) if err != nil { - return err + return h.handleAuthError(err, accountNickname, msg.ConvID) } calendarList, err := getCalendarList(ctx, srv) diff --git a/gcalbot/gcalbot/http.go b/gcalbot/gcalbot/http.go index 4e0105bd..1ca6c9f6 100644 --- a/gcalbot/gcalbot/http.go +++ b/gcalbot/gcalbot/http.go @@ -201,8 +201,15 @@ func (h *HTTPSrv) configHandler(w http.ResponseWriter, r *http.Request) { return } - srv, err := GetCalendarService(ctx, selectedAccount, h.oauth, h.db) + srv, err := h.handler.GetCalendarServiceWithRetry(ctx, selectedAccount) if err != nil { + switch err.(type) { + case AccountAuthError: + h.Errorf("account auth failed for web UI: %v", err) + h.showConfigError(w) + default: + h.Errorf("error getting calendar service: %v", err) + } return } diff --git a/gcalbot/gcalbot/invite.go b/gcalbot/gcalbot/invite.go index ce951389..59b7143e 100644 --- a/gcalbot/gcalbot/invite.go +++ b/gcalbot/gcalbot/invite.go @@ -41,9 +41,9 @@ Awaiting your response. *Are you going?*` eventType = "a recurring event" } - srv, err := GetCalendarService(ctx, account, h.oauth, h.db) + srv, err := h.GetCalendarServiceWithRetry(ctx, account) if err != nil { - return err + return h.handleAuthErrorDM(err, account) } timezone, err := GetUserTimezone(srv) if err != nil { @@ -107,9 +107,9 @@ func (h *Handler) updateEventResponseStatus(ctx context.Context, invite *Invite, return nil } - srv, err := GetCalendarService(ctx, account, h.oauth, h.db) + srv, err := h.GetCalendarServiceWithRetry(ctx, account) if err != nil { - return err + return h.handleAuthErrorDM(err, account) } // fetch event diff --git a/gcalbot/gcalbot/schedulescheduler/send.go b/gcalbot/gcalbot/schedulescheduler/send.go index 5718dbb5..7d6ab029 100644 --- a/gcalbot/gcalbot/schedulescheduler/send.go +++ b/gcalbot/gcalbot/schedulescheduler/send.go @@ -13,6 +13,7 @@ import ( "google.golang.org/api/calendar/v3" "google.golang.org/api/googleapi" + "github.com/keybase/managed-bots/base" "github.com/keybase/managed-bots/gcalbot/gcalbot" ) @@ -109,6 +110,10 @@ func (s *ScheduleScheduler) SendDailyScheduleMessage(sendMinute time.Time, subsc s.Debug("error retrieving token: %s", err) return default: + if base.ShouldRetryAuth(err) { + s.Debug("auth error in scheduler (will not auto-delete): %s", err) + return + } s.Errorf("unable to get calendar service: %s", err) return } diff --git a/gcalbot/gcalbot/type.go b/gcalbot/gcalbot/type.go index cde8afa3..0842a170 100644 --- a/gcalbot/gcalbot/type.go +++ b/gcalbot/gcalbot/type.go @@ -1,6 +1,7 @@ package gcalbot import ( + "fmt" "time" "github.com/keybase/go-keybase-chat-bot/kbchat/types/chat1" @@ -103,3 +104,13 @@ type AggregatedDailyScheduleSubscription struct { NotificationTime time.Duration Account Account } + +// AccountAuthError indicates that account credentials are invalid and were deleted +type AccountAuthError struct { + Username string + Nickname string +} + +func (e AccountAuthError) Error() string { + return fmt.Sprintf("account '%s' for user '%s' requires re-authentication", e.Nickname, e.Username) +} diff --git a/gcalbot/gcalbot/webhook.go b/gcalbot/gcalbot/webhook.go index 0e62711a..91a74f62 100644 --- a/gcalbot/gcalbot/webhook.go +++ b/gcalbot/gcalbot/webhook.go @@ -70,6 +70,11 @@ func (h *HTTPSrv) handleEventUpdateWebhook(w http.ResponseWriter, r *http.Reques err = nil // clear error return default: + if base.ShouldRetryAuth(err) { + h.Debug("auth error in webhook (will not auto-delete): %s", err) + err = nil // clear error + return + } return } @@ -254,21 +259,28 @@ func (h *Handler) removeSubscription( if channel != nil { srv, err := GetCalendarService(ctx, account, h.oauth, h.db) if err != nil { - return err - } - err = srv.Channels.Stop(&calendar.Channel{ - Id: channel.ChannelID, - ResourceId: channel.ResourceID, - }).Do() - switch err := err.(type) { - case nil: - case *googleapi.Error: - if err.Code != 404 { + if base.ShouldRetryAuth(err) { + h.Debug("auth error stopping channel, skipping channel.Stop: %s", err) + // Still delete from DB even if we can't stop the channel + } else { + return err + } + } else { + // Only try to stop if we got the service successfully + err = srv.Channels.Stop(&calendar.Channel{ + Id: channel.ChannelID, + ResourceId: channel.ResourceID, + }).Do() + switch err := err.(type) { + case nil: + case *googleapi.Error: + if err.Code != 404 { + return err + } + // if the channel wasn't found, don't return + default: return err } - // if the channel wasn't found, don't return - default: - return err } err = h.db.DeleteChannelByChannelID(ctx, channel.ChannelID) @@ -284,6 +296,9 @@ func (h *Handler) removeSubscription( func (h *Handler) createEventChannel(ctx context.Context, account *Account, calendarID string) error { srv, err := GetCalendarService(ctx, account, h.oauth, h.db) if err != nil { + if base.ShouldRetryAuth(err) { + h.Debug("auth error creating channel, cannot create webhook: %s", err) + } return err } exists, err := h.db.ExistsChannelByAccountAndCalendar(ctx, account, calendarID) diff --git a/meetbot/meetbot/handler.go b/meetbot/meetbot/handler.go index b6e7ecd0..357ada58 100644 --- a/meetbot/meetbot/handler.go +++ b/meetbot/meetbot/handler.go @@ -79,8 +79,7 @@ func (h *Handler) meetHandler(ctx context.Context, msg chat1.MsgSummary) error { h.Errorf("unable to get service %v, deleting credentials and retrying", err) return retry() default: - if strings.Contains(err.Error(), "cannot fetch token") || - strings.Contains(err.Error(), "invalid_grant") { + if base.ShouldRetryAuth(err) { h.Errorf("unable to get service %v, deleting credentials and retrying", err) return retry() } diff --git a/zoombot/zoombot/handler.go b/zoombot/zoombot/handler.go index bea588de..84482c60 100644 --- a/zoombot/zoombot/handler.go +++ b/zoombot/zoombot/handler.go @@ -91,8 +91,7 @@ func (h *Handler) zoomHandler(ctx context.Context, msg chat1.MsgSummary, attempt } return err default: - if strings.Contains(err.Error(), "cannot fetch token") || - strings.Contains(err.Error(), "invalid_grant") { + if base.ShouldRetryAuth(err) { h.Errorf("unable to get service %v, deleting credentials and retrying", err) return retry() }