mirror of
https://github.com/hanzoai/team.git
synced 2026-09-30 01:57:35 +00:00
c9e9a2a43e
* MM-69693: notify token owners when an expired PAT is deleted Send an at-expiry DM from the cleanup_expired_access_tokens job at the moment it deletes an expired personal access token. This complements the pre-expiry warning cascade (MM-69215): for the silent-integration case, where a fire-and-forget caller swallows the rejection error, the owner is now told their token stopped working. The notification fires in the same step as the delete, so no grace period, tombstone, or new schema is needed and re-notification is inherently impossible. It is best-effort: sent before the delete (while the token->owner mapping still exists) and the delete proceeds regardless of the result. Bot-owned tokens and tokens owned by deactivated users are skipped, matching the pre-expiry job. An app-layer notify callback is injected into the worker, mirroring how expirynotify is wired, so cleanupExpired stays unit-testable with a fake notifier. * MM-69693: add app-layer notify tests and isolate notifier panics Add expired_access_token_notify_test.go covering NotifyExpiredAccessTokensDeleted: empty list, happy-path DM, bot-owned skip, deactivated-user skip, and unknown-owner skip with continued processing. Wrap the notifyExpired call in the cleanup worker with its own recover so a panic in the best-effort notifier can't abort execute() before DeleteByIds runs, and add a worker test asserting deletion still happens after a notify panic. * MM-69693: drop notifier panic recover from cleanup worker The job-level HandleJobPanic in execute() is already the panic boundary for the run, and NotifyExpiredAccessTokensDeleted logs and swallows its own errors, so the local recover around notifyExpired added no real protection. Remove it (and its test) along with the now-redundant inline comment; the notify-before-delete rationale stays documented on cleanupExpired. * MM-69693: thread request.CTX into NotifyExpiredAccessTokensDeleted Pass the job's request context from the cleanup worker down through cleanupExpired into the notify callback instead of building a fresh EmptyContext inside the app function, so notification logging is correlated with the job's logger. --------- Co-authored-by: Hanzo Dev <[email protected]>
131 lines
4.1 KiB
Go
131 lines
4.1 KiB
Go
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
|
// See LICENSE.txt for license information.
|
|
|
|
package app
|
|
|
|
import (
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"github.com/mattermost/mattermost/server/public/model"
|
|
)
|
|
|
|
// dmPostsFromSystemBot returns the posts in the DM channel between the system
|
|
// bot and userID, or nil if no such channel exists yet. NotifyExpiredAccessTokensDeleted
|
|
// posts synchronously, so no polling is required.
|
|
func dmPostsFromSystemBot(t *testing.T, th *TestHelper, botUserID, userID string) []*model.Post {
|
|
t.Helper()
|
|
|
|
channel, err := th.App.Srv().Store().Channel().GetByName("", model.GetDMNameFromIds(botUserID, userID), false)
|
|
if err != nil {
|
|
// No DM channel means no notification was ever sent.
|
|
return nil
|
|
}
|
|
|
|
postList, err := th.App.Srv().Store().Post().GetPosts(th.Context, model.GetPostsOptions{ChannelId: channel.Id, Page: 0, PerPage: 10}, false, map[string]bool{})
|
|
require.NoError(t, err)
|
|
|
|
posts := make([]*model.Post, 0, len(postList.Order))
|
|
for _, id := range postList.Order {
|
|
posts = append(posts, postList.Posts[id])
|
|
}
|
|
return posts
|
|
}
|
|
|
|
// notificationPostsFor returns the DM posts authored by the system bot that
|
|
// mention the given token description.
|
|
func notificationPostsFor(t *testing.T, th *TestHelper, botUserID, userID, description string) []*model.Post {
|
|
t.Helper()
|
|
|
|
var matching []*model.Post
|
|
for _, post := range dmPostsFromSystemBot(t, th, botUserID, userID) {
|
|
if post.UserId == botUserID && strings.Contains(post.Message, description) {
|
|
matching = append(matching, post)
|
|
}
|
|
}
|
|
return matching
|
|
}
|
|
|
|
func TestNotifyExpiredAccessTokensDeleted(t *testing.T) {
|
|
mainHelper.Parallel(t)
|
|
|
|
t.Run("empty token list is a no-op", func(t *testing.T) {
|
|
th := Setup(t).InitBasic(t)
|
|
|
|
require.NotPanics(t, func() {
|
|
th.App.NotifyExpiredAccessTokensDeleted(th.Context, nil)
|
|
th.App.NotifyExpiredAccessTokensDeleted(th.Context, []*model.UserAccessToken{})
|
|
})
|
|
})
|
|
|
|
t.Run("happy path DMs the token owner", func(t *testing.T) {
|
|
th := Setup(t).InitBasic(t)
|
|
|
|
bot, appErr := th.App.GetSystemBot(th.Context)
|
|
require.Nil(t, appErr)
|
|
|
|
user := th.CreateUser(t)
|
|
const description = "ci-integration-token"
|
|
|
|
th.App.NotifyExpiredAccessTokensDeleted(th.Context, []*model.UserAccessToken{
|
|
{Id: model.NewId(), UserId: user.Id, Description: description},
|
|
})
|
|
|
|
posts := notificationPostsFor(t, th, bot.UserId, user.Id, description)
|
|
require.Len(t, posts, 1, "the owner should receive exactly one expiry notification")
|
|
require.Equal(t, bot.UserId, posts[0].UserId)
|
|
})
|
|
|
|
t.Run("bot-owned token is skipped", func(t *testing.T) {
|
|
th := Setup(t).InitBasic(t)
|
|
|
|
systemBot, appErr := th.App.GetSystemBot(th.Context)
|
|
require.Nil(t, appErr)
|
|
|
|
bot := th.CreateBot(t)
|
|
|
|
th.App.NotifyExpiredAccessTokensDeleted(th.Context, []*model.UserAccessToken{
|
|
{Id: model.NewId(), UserId: bot.UserId, Description: "bot-token"},
|
|
})
|
|
|
|
require.Empty(t, dmPostsFromSystemBot(t, th, systemBot.UserId, bot.UserId))
|
|
})
|
|
|
|
t.Run("deactivated user token is skipped", func(t *testing.T) {
|
|
th := Setup(t).InitBasic(t)
|
|
|
|
bot, appErr := th.App.GetSystemBot(th.Context)
|
|
require.Nil(t, appErr)
|
|
|
|
user := th.CreateUser(t)
|
|
_, appErr = th.App.UpdateActive(th.Context, user, false)
|
|
require.Nil(t, appErr)
|
|
|
|
th.App.NotifyExpiredAccessTokensDeleted(th.Context, []*model.UserAccessToken{
|
|
{Id: model.NewId(), UserId: user.Id, Description: "deactivated-token"},
|
|
})
|
|
|
|
require.Empty(t, dmPostsFromSystemBot(t, th, bot.UserId, user.Id))
|
|
})
|
|
|
|
t.Run("unknown owner is skipped but processing continues", func(t *testing.T) {
|
|
th := Setup(t).InitBasic(t)
|
|
|
|
bot, appErr := th.App.GetSystemBot(th.Context)
|
|
require.Nil(t, appErr)
|
|
|
|
user := th.CreateUser(t)
|
|
|
|
// First token references a user that does not exist (GetUser fails);
|
|
// the second is a valid active user that must still be notified.
|
|
th.App.NotifyExpiredAccessTokensDeleted(th.Context, []*model.UserAccessToken{
|
|
{Id: model.NewId(), UserId: model.NewId(), Description: "orphan-token"},
|
|
{Id: model.NewId(), UserId: user.Id, Description: "valid-token"},
|
|
})
|
|
|
|
require.Len(t, notificationPostsFor(t, th, bot.UserId, user.Id, "valid-token"), 1)
|
|
})
|
|
}
|