mirror of
https://github.com/mattermost/mattermost.git
synced 2026-08-26 21:27:40 -05:00
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
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
}
|
||||
@@ -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) {
|
||||
|
||||
@@ -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"]
|
||||
|
||||
@@ -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" })
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user