mirror of
https://github.com/outline/outline.git
synced 2026-06-13 11:25:03 +03:00
perf: Avoid correlated subquery in Slack hooks user lookup (#12432)
* perf: Avoid correlated subquery in Slack hooks user lookup Query UserAuthentication directly by indexed providerId and load the associated User and Team, instead of driving from User.findOne with a required hasMany include — which Sequelize translates into a correlated subquery that scans the users table. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: Scope Slack fallback user lookup to matching AuthenticationProvider The fallback in findUserForRequest matched any UserAuthentication with the same providerId, which is only unique per (providerId, userId). A colliding external user id from another workspace or provider could resolve a user from the wrong team. Constrain via the AuthenticationProvider join (name = "slack", providerId = serviceTeamId). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -34,7 +34,7 @@ describe("#hooks.unfurl", () => {
|
||||
const res = await server.post("/api/hooks.unfurl", {
|
||||
body: {
|
||||
token: env.SLACK_VERIFICATION_TOKEN,
|
||||
team_id: `T${randomString(8)}`,
|
||||
team_id: user.team.authenticationProviders[0].providerId,
|
||||
api_app_id: `A${randomString(8)}`,
|
||||
event: {
|
||||
type: "link_shared",
|
||||
|
||||
@@ -403,28 +403,41 @@ async function findUserForRequest(
|
||||
return integration.user;
|
||||
}
|
||||
|
||||
// Fallback to authentication provider if the user has Slack sign-in
|
||||
const user = await User.findOne({
|
||||
// Fallback to authentication provider if the user has Slack sign-in.
|
||||
// Scoped via AuthenticationProvider to the matching Slack workspace so a
|
||||
// colliding providerId from another team/provider cannot resolve.
|
||||
const authentication = await UserAuthentication.findOne({
|
||||
where: {
|
||||
providerId: serviceUserId,
|
||||
},
|
||||
order: [["createdAt", "DESC"]],
|
||||
include: [
|
||||
{
|
||||
where: {
|
||||
providerId: serviceUserId,
|
||||
},
|
||||
order: [["createdAt", "DESC"]],
|
||||
model: UserAuthentication,
|
||||
as: "authentications",
|
||||
model: AuthenticationProvider,
|
||||
as: "authenticationProvider",
|
||||
required: true,
|
||||
where: {
|
||||
name: "slack",
|
||||
providerId: serviceTeamId,
|
||||
},
|
||||
},
|
||||
{
|
||||
model: Team,
|
||||
as: "team",
|
||||
model: User,
|
||||
as: "user",
|
||||
required: true,
|
||||
include: [
|
||||
{
|
||||
model: Team,
|
||||
as: "team",
|
||||
required: true,
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
if (user) {
|
||||
return user;
|
||||
if (authentication?.user) {
|
||||
return authentication.user;
|
||||
}
|
||||
|
||||
return;
|
||||
|
||||
Reference in New Issue
Block a user