From 0afef7760c40d87a492d540e1bb428a094e66cce Mon Sep 17 00:00:00 2001 From: Felipe Martin <812088+fmartingr@users.noreply.github.com> Date: Mon, 11 May 2026 10:09:47 +0200 Subject: [PATCH] Include connection ID in plugin context (#36074) * feat: include connection id in the plugin context * refactor: group ConnectionId next to SessionId in plugin Context Addresses review feedback to keep related identifier fields adjacent. * fix(files): forward Connection-Id on file uploads to plugin hooks The webapp uploadFile XHR didn't attach the Connection-Id header, so FileWillBeUploaded plugin hooks always received an empty ConnectionId. Read it from the websocket selector and set it on the request, matching how drafts and channel bookmarks already do it. Adds a server-side test asserting the connection id propagates through pluginContext. * fix(lint): reorder file_actions imports to satisfy import/order * Document ConnectionId on request.Context --- server/channels/api4/command.go | 1 + server/channels/app/context.go | 1 + server/channels/app/context_test.go | 46 ++++++++++++++++++ server/channels/app/plugin_hooks_test.go | 49 ++++++++++++++++++++ server/channels/app/plugin_requests.go | 1 + server/channels/app/plugin_requests_test.go | 32 +++++++++++++ server/channels/web/handlers.go | 4 ++ server/channels/web/handlers_test.go | 44 ++++++++++++++++++ server/public/model/command_args.go | 18 +++---- server/public/model/command_args_test.go | 39 ++++++++++++++++ server/public/plugin/context.go | 1 + server/public/shared/request/context.go | 20 ++++++++ server/public/shared/request/context_test.go | 26 +++++++++++ webapp/channels/src/actions/file_actions.ts | 7 +++ 14 files changed, 281 insertions(+), 8 deletions(-) create mode 100644 server/channels/app/context_test.go diff --git a/server/channels/api4/command.go b/server/channels/api4/command.go index e50830a1e4e..4588a62c309 100644 --- a/server/channels/api4/command.go +++ b/server/channels/api4/command.go @@ -416,6 +416,7 @@ func executeCommand(c *Context, w http.ResponseWriter, r *http.Request) { commandArgs.UserId = c.AppContext.Session().UserId commandArgs.T = c.AppContext.T commandArgs.SiteURL = c.GetSiteURLHeader() + commandArgs.ConnectionId = r.Header.Get(model.ConnectionId) response, err := c.App.ExecuteCommand(c.AppContext, &commandArgs) if err != nil { diff --git a/server/channels/app/context.go b/server/channels/app/context.go index 83b04e1887a..0d5e1d5a578 100644 --- a/server/channels/app/context.go +++ b/server/channels/app/context.go @@ -37,6 +37,7 @@ func pluginContext(rctx request.CTX) *plugin.Context { IPAddress: rctx.IPAddress(), AcceptLanguage: rctx.AcceptLanguage(), UserAgent: rctx.UserAgent(), + ConnectionId: rctx.ConnectionId(), } return context } diff --git a/server/channels/app/context_test.go b/server/channels/app/context_test.go new file mode 100644 index 00000000000..51da9585a6f --- /dev/null +++ b/server/channels/app/context_test.go @@ -0,0 +1,46 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/request" +) + +func TestPluginContext(t *testing.T) { + t.Run("creates plugin context with all fields from request context", func(t *testing.T) { + rctx := request.TestContext(t) + session := &model.Session{ + Id: "session-id-123", + UserId: "user-id-456", + } + rctx = rctx.WithSession(session).(*request.Context) + rctx = rctx.WithRequestId("request-id-789").(*request.Context) + rctx = rctx.WithIPAddress("192.168.1.1").(*request.Context) + rctx = rctx.WithAcceptLanguage("en-US").(*request.Context) + rctx = rctx.WithUserAgent("TestAgent/1.0").(*request.Context) + rctx = rctx.WithConnectionId("connection-id-abc").(*request.Context) + + ctx := pluginContext(rctx) + + assert.Equal(t, "request-id-789", ctx.RequestId) + assert.Equal(t, "session-id-123", ctx.SessionId) + assert.Equal(t, "192.168.1.1", ctx.IPAddress) + assert.Equal(t, "en-US", ctx.AcceptLanguage) + assert.Equal(t, "TestAgent/1.0", ctx.UserAgent) + assert.Equal(t, "connection-id-abc", ctx.ConnectionId) + }) + + t.Run("creates plugin context with empty connection id when not set", func(t *testing.T) { + rctx := request.TestContext(t) + + ctx := pluginContext(rctx) + + assert.Empty(t, ctx.ConnectionId) + }) +} diff --git a/server/channels/app/plugin_hooks_test.go b/server/channels/app/plugin_hooks_test.go index 0a595480c3b..82562d734df 100644 --- a/server/channels/app/plugin_hooks_test.go +++ b/server/channels/app/plugin_hooks_test.go @@ -712,6 +712,55 @@ func TestHookFileWillBeUploaded(t *testing.T) { require.NoError(t, err) assert.Equal(t, "changedtext", resultBuf.String()) }) + + t.Run("connection id propagated to plugin context", func(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + const connectionID = "test-connection-id-xyz" + + var mockAPI plugintest.API + mockAPI.On("LoadPluginConfiguration", mock.Anything).Return(nil) + mockAPI.On("LogDebug", "testhook.txt").Return(nil) + mockAPI.On("LogDebug", "inputfile").Return(nil) + mockAPI.On("LogDebug", "connection_id="+connectionID).Return(nil) + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{ + ` + package main + + import ( + "io" + "github.com/mattermost/mattermost/server/public/plugin" + "github.com/mattermost/mattermost/server/public/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) FileWillBeUploaded(c *plugin.Context, info *model.FileInfo, file io.Reader, output io.Writer) (*model.FileInfo, string) { + p.API.LogDebug("connection_id=" + c.ConnectionId) + return nil, "" + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + `, + }, th.App, func(*model.Manifest) plugin.API { return &mockAPI }) + defer tearDown() + + rctx := th.Context.WithConnectionId(connectionID) + + _, appErr := th.App.UploadFile(rctx, + []byte("inputfile"), + th.BasicChannel.Id, + "testhook.txt", + ) + require.Nil(t, appErr) + + mockAPI.AssertCalled(t, "LogDebug", "connection_id="+connectionID) + }) } func TestUserWillLogIn_Blocked(t *testing.T) { diff --git a/server/channels/app/plugin_requests.go b/server/channels/app/plugin_requests.go index 52e25f17aea..ef033f4888d 100644 --- a/server/channels/app/plugin_requests.go +++ b/server/channels/app/plugin_requests.go @@ -164,6 +164,7 @@ func (ch *Channels) servePluginRequest(w http.ResponseWriter, r *http.Request, h IPAddress: utils.GetIPAddress(r, ch.cfgSvc.Config().ServiceSettings.TrustedProxyIPHeader), AcceptLanguage: r.Header.Get("Accept-Language"), UserAgent: r.UserAgent(), + ConnectionId: r.Header.Get(model.ConnectionId), } pluginID := mux.Vars(r)["plugin_id"] diff --git a/server/channels/app/plugin_requests_test.go b/server/channels/app/plugin_requests_test.go index 87920a1b940..44f688ab9ea 100644 --- a/server/channels/app/plugin_requests_test.go +++ b/server/channels/app/plugin_requests_test.go @@ -451,6 +451,38 @@ func TestServePluginRequest(t *testing.T) { require.True(t, handlerCalled) }) + t.Run("connection id passed to plugin context", func(t *testing.T) { + connectionId := "test-connection-id-abc123" + req := httptest.NewRequest(http.MethodGet, "/plugins/testplugin/endpoint", nil) + req = mux.SetURLVars(req, map[string]string{"plugin_id": "testplugin"}) + req.Header.Set(model.ConnectionId, connectionId) + rr := httptest.NewRecorder() + + handlerCalled := false + mockHandler := func(ctx *plugin.Context, w http.ResponseWriter, r *http.Request) { + handlerCalled = true + assert.Equal(t, connectionId, ctx.ConnectionId) + } + + th.App.ch.servePluginRequest(rr, req, mockHandler) + require.True(t, handlerCalled) + }) + + t.Run("empty connection id when header not present", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/plugins/testplugin/endpoint", nil) + req = mux.SetURLVars(req, map[string]string{"plugin_id": "testplugin"}) + rr := httptest.NewRecorder() + + handlerCalled := false + mockHandler := func(ctx *plugin.Context, w http.ResponseWriter, r *http.Request) { + handlerCalled = true + assert.Empty(t, ctx.ConnectionId) + } + + th.App.ch.servePluginRequest(rr, req, mockHandler) + require.True(t, handlerCalled) + }) + t.Run("subpath handling", func(t *testing.T) { // Set up with subpath th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "http://localhost:8065/subpath" }) diff --git a/server/channels/web/handlers.go b/server/channels/web/handlers.go index 1ec64fb611e..8a7f1723594 100644 --- a/server/channels/web/handlers.go +++ b/server/channels/web/handlers.go @@ -191,6 +191,10 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { t, ) + if connectionId := r.Header.Get(model.ConnectionId); connectionId != "" { + c.AppContext = c.AppContext.WithConnectionId(connectionId) + } + c.Params = ParamsFromRequest(r) c.Logger = c.App.Log() diff --git a/server/channels/web/handlers_test.go b/server/channels/web/handlers_test.go index 456c3f7c14b..47d05b7cb1a 100644 --- a/server/channels/web/handlers_test.go +++ b/server/channels/web/handlers_test.go @@ -1122,6 +1122,50 @@ func TestHandlerServeHTTPRequestPayloadLimit(t *testing.T) { }) } +func TestHandlerConnectionIdHeader(t *testing.T) { + t.Run("should set connection id from header on request context", func(t *testing.T) { + th := SetupWithStoreMock(t) + + connectionId := "test-connection-id-12345" + var capturedConnectionId string + + handlerFunc := func(c *Context, w http.ResponseWriter, r *http.Request) { + capturedConnectionId = c.AppContext.ConnectionId() + } + + web := New(th.Server) + handler := web.NewHandler(handlerFunc) + + request := httptest.NewRequest("GET", "/api/v4/test", nil) + request.Header.Set(model.ConnectionId, connectionId) + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + + assert.Equal(t, http.StatusOK, response.Code) + assert.Equal(t, connectionId, capturedConnectionId) + }) + + t.Run("should have empty connection id when header not present", func(t *testing.T) { + th := SetupWithStoreMock(t) + + var capturedConnectionId string + + handlerFunc := func(c *Context, w http.ResponseWriter, r *http.Request) { + capturedConnectionId = c.AppContext.ConnectionId() + } + + web := New(th.Server) + handler := web.NewHandler(handlerFunc) + + request := httptest.NewRequest("GET", "/api/v4/test", nil) + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + + assert.Equal(t, http.StatusOK, response.Code) + assert.Empty(t, capturedConnectionId) + }) +} + func TestHandleContextErrorZeroStatusCode(t *testing.T) { t.Run("should set StatusCode to 500 when AppError has zero StatusCode", func(t *testing.T) { th := SetupWithStoreMock(t) diff --git a/server/public/model/command_args.go b/server/public/model/command_args.go index 24781c98c0c..136bd0205cb 100644 --- a/server/public/model/command_args.go +++ b/server/public/model/command_args.go @@ -14,6 +14,7 @@ type CommandArgs struct { RootId string `json:"root_id"` ParentId string `json:"parent_id"` TriggerId string `json:"trigger_id,omitempty"` + ConnectionId string `json:"connection_id,omitempty"` Command string `json:"command"` SiteURL string `json:"-"` T i18n.TranslateFunc `json:"-"` @@ -23,14 +24,15 @@ type CommandArgs struct { func (o *CommandArgs) Auditable() map[string]any { return map[string]any{ - "user_id": o.UserId, - "channel_id": o.ChannelId, - "team_id": o.TeamId, - "root_id": o.RootId, - "parent_id": o.ParentId, - "trigger_id": o.TriggerId, - "command": o.Command, - "site_url": o.SiteURL, + "user_id": o.UserId, + "channel_id": o.ChannelId, + "team_id": o.TeamId, + "root_id": o.RootId, + "parent_id": o.ParentId, + "trigger_id": o.TriggerId, + "connection_id": o.ConnectionId, + "command": o.Command, + "site_url": o.SiteURL, } } diff --git a/server/public/model/command_args_test.go b/server/public/model/command_args_test.go index 7f4613eb29e..97059718999 100644 --- a/server/public/model/command_args_test.go +++ b/server/public/model/command_args_test.go @@ -9,6 +9,45 @@ import ( "github.com/stretchr/testify/require" ) +func TestCommandArgs_Auditable(t *testing.T) { + t.Run("includes connection_id in auditable output", func(t *testing.T) { + args := CommandArgs{ + UserId: "user-id", + ChannelId: "channel-id", + TeamId: "team-id", + RootId: "root-id", + ParentId: "parent-id", + TriggerId: "trigger-id", + ConnectionId: "connection-id-123", + Command: "/test command", + SiteURL: "http://localhost:8065", + } + + auditable := args.Auditable() + + require.Equal(t, "user-id", auditable["user_id"]) + require.Equal(t, "channel-id", auditable["channel_id"]) + require.Equal(t, "team-id", auditable["team_id"]) + require.Equal(t, "root-id", auditable["root_id"]) + require.Equal(t, "parent-id", auditable["parent_id"]) + require.Equal(t, "trigger-id", auditable["trigger_id"]) + require.Equal(t, "connection-id-123", auditable["connection_id"]) + require.Equal(t, "/test command", auditable["command"]) + require.Equal(t, "http://localhost:8065", auditable["site_url"]) + }) + + t.Run("includes empty connection_id when not set", func(t *testing.T) { + args := CommandArgs{ + UserId: "user-id", + Command: "/test", + } + + auditable := args.Auditable() + + require.Equal(t, "", auditable["connection_id"]) + }) +} + func TestCommandArgs_AddUserMention(t *testing.T) { fixture := []struct { args CommandArgs diff --git a/server/public/plugin/context.go b/server/public/plugin/context.go index bfd3c2e5405..33150e5f8fe 100644 --- a/server/public/plugin/context.go +++ b/server/public/plugin/context.go @@ -8,6 +8,7 @@ package plugin // For hooks, app.PluginContext() is called. type Context struct { SessionId string + ConnectionId string RequestId string IPAddress string AcceptLanguage string diff --git a/server/public/shared/request/context.go b/server/public/shared/request/context.go index ab96e180a1d..c8e65963dbb 100644 --- a/server/public/shared/request/context.go +++ b/server/public/shared/request/context.go @@ -22,6 +22,7 @@ type Context struct { path string userAgent string acceptLanguage string + connectionId string logger mlog.LoggerIFace context context.Context } @@ -96,6 +97,17 @@ func (c *Context) AcceptLanguage() string { return c.acceptLanguage } +// ConnectionId returns the identifier of the WebSocket connection associated +// with the request, when present. It is populated from the "Connection-Id" +// HTTP header that authenticated clients set when they have an active +// WebSocket connection, allowing handlers and plugins to correlate an HTTP +// request with its originating WebSocket connection. Returns an empty string +// when the header is absent (e.g., requests from clients without an active +// WebSocket connection or from non-WebSocket integrations). +func (c *Context) ConnectionId() string { + return c.connectionId +} + func (c *Context) Logger() mlog.LoggerIFace { return c.logger } @@ -156,6 +168,12 @@ func (c *Context) WithAcceptLanguage(s string) CTX { return rctx } +func (c *Context) WithConnectionId(s string) CTX { + rctx := c.clone() + rctx.connectionId = s + return rctx +} + func (c *Context) WithContext(ctx context.Context) CTX { rctx := c.clone() rctx.context = ctx @@ -188,6 +206,7 @@ type CTX interface { Path() string UserAgent() string AcceptLanguage() string + ConnectionId() string Logger() mlog.LoggerIFace Context() context.Context WithT(i18n.TranslateFunc) CTX @@ -198,6 +217,7 @@ type CTX interface { WithPath(string) CTX WithUserAgent(string) CTX WithAcceptLanguage(string) CTX + WithConnectionId(string) CTX WithLogger(mlog.LoggerIFace) CTX WithLogFields(fields ...mlog.Field) CTX WithContext(ctx context.Context) CTX diff --git a/server/public/shared/request/context_test.go b/server/public/shared/request/context_test.go index 40cdd3ba6e4..53fc9fd29a7 100644 --- a/server/public/shared/request/context_test.go +++ b/server/public/shared/request/context_test.go @@ -11,6 +11,32 @@ import ( "github.com/stretchr/testify/require" ) +func TestContext_WithConnectionId(t *testing.T) { + t.Run("returns new context with connection id", func(t *testing.T) { + originalCtx := TestContext(t) + connectionId := "test-connection-id-123" + + newCtx := originalCtx.WithConnectionId(connectionId) + + require.NotNil(t, newCtx) + assert.NotSame(t, originalCtx, newCtx, "should return a new context instance") + assert.Equal(t, connectionId, newCtx.ConnectionId()) + assert.Empty(t, originalCtx.ConnectionId(), "original context should remain unchanged") + }) + + t.Run("returns new context with empty connection id", func(t *testing.T) { + originalCtx := TestContext(t) + originalCtx = originalCtx.WithConnectionId("existing-id").(*Context) + + newCtx := originalCtx.WithConnectionId("") + + require.NotNil(t, newCtx) + assert.NotSame(t, originalCtx, newCtx, "should return a new context instance") + assert.Empty(t, newCtx.ConnectionId()) + assert.Equal(t, "existing-id", originalCtx.ConnectionId(), "original context should remain unchanged") + }) +} + func TestContext_WithSession(t *testing.T) { t.Run("returns new context with empty session when session is nil", func(t *testing.T) { originalCtx := TestContext(t) diff --git a/webapp/channels/src/actions/file_actions.ts b/webapp/channels/src/actions/file_actions.ts index 05cf454d0c2..85f1b8aa1a8 100644 --- a/webapp/channels/src/actions/file_actions.ts +++ b/webapp/channels/src/actions/file_actions.ts @@ -11,6 +11,8 @@ import {getLogErrorAction} from 'mattermost-redux/actions/errors'; import {forceLogoutIfNecessary} from 'mattermost-redux/actions/helpers'; import {Client4} from 'mattermost-redux/client'; +import {getConnectionId} from 'selectors/general'; + import type {FilePreviewInfo} from 'components/file_preview/file_preview'; import {localizeMessage} from 'utils/utils'; @@ -52,6 +54,11 @@ export function uploadFile({file, name, type, rootId, channelId, clientId, onPro xhr.setRequestHeader('Accept', 'application/json'); + const connectionId = getConnectionId(getState()); + if (connectionId) { + xhr.setRequestHeader('Connection-Id', connectionId); + } + const formData = new FormData(); formData.append('channel_id', channelId); formData.append('client_ids', clientId);