diff --git a/e2e-tests/cypress/tests/integration/channels/files_and_attachments/file_preview_image_spec.js b/e2e-tests/cypress/tests/integration/channels/files_and_attachments/file_preview_image_spec.js index 2f66beeaf1b..fa461f76aa8 100644 --- a/e2e-tests/cypress/tests/integration/channels/files_and_attachments/file_preview_image_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/files_and_attachments/file_preview_image_spec.js @@ -104,6 +104,61 @@ describe('Upload Files - Image', () => { testImage(properties); }); + + it('MM-69026 - Image preview supports zoom controls and keyboard shortcut', () => { + // Use a unique filename so we can find the thumbnail unambiguously after + // earlier tests in this spec have already uploaded PNG.png/JPG.jpg/etc. + const fileName = 'mm-69026-zoom.png'; + const properties = { + filePath: 'mm_file_testing/Images/PNG.png', + fileName, + originalWidth: 400, + originalHeight: 479, + mimeType: 'image/png', + }; + + // # Post any message + cy.postMessage(fileName); + + // # Post an image in center channel + attachFile(properties); + + // # Wait until file upload is complete then submit + waitUntilUploadComplete(); + cy.uiGetPostTextBox().clear().type('{enter}'); + cy.wait(TIMEOUTS.FIVE_SEC); + + // # Open file preview + cy.uiGetFileThumbnail(fileName).click(); + + // * Verify preview modal opened + cy.uiGetFilePreviewModal().as('filePreviewModal'); + + // * Zoom controls are visible for images + cy.get('@filePreviewModal').find('.file-preview-modal__zoom-bar .modal-zoom-btn').should('have.length.at.least', 1); + + // # Click zoom-in + cy.get('@filePreviewModal').find('.modal-zoom-btn .icon-plus').click(); + + // * Image is scaled up — match only scale values strictly greater than 1 + // (e.g. scale(1.25), scale(2)), so a transform of scale(1) or no + // scale at all would fail this assertion. + cy.get('@filePreviewModal').find('[data-testid="imagePreview"]'). + should('have.attr', 'style'). + and('match', /scale\((?:1\.\d+|[2-9]\d*(?:\.\d+)?)\)/); + + // # Press '0' to reset + cy.get('body').type('0'); + + // * Image transform clears (no scale applied at default 1.0) + cy.get('@filePreviewModal').find('[data-testid="imagePreview"]').then(($img) => { + const style = $img.attr('style') || ''; + expect(style).to.not.match(/scale\([0-9.]+\)/); + }); + + // # Close modal + cy.uiCloseFilePreviewModal(); + }); }); function testImage(properties) { diff --git a/server/build/notice-file/config.yaml b/server/build/notice-file/config.yaml index 72cbbbe2b02..83838ac586a 100644 --- a/server/build/notice-file/config.yaml +++ b/server/build/notice-file/config.yaml @@ -7,6 +7,5 @@ search: - "webapp/package.json" - "webapp/channels/package.json" ignoreDependencies: - - "@mattermost/dynamic-virtualized-list" - "@mattermost/shared" includeDevDependencies: false diff --git a/server/channels/api4/bot_test.go b/server/channels/api4/bot_test.go index 109f53ea241..a65eebb7785 100644 --- a/server/channels/api4/bot_test.go +++ b/server/channels/api4/bot_test.go @@ -129,7 +129,7 @@ func TestCreateBot(t *testing.T) { _, appErr = th.App.UpdateUserRoles(th.Context, bot.UserId, model.TeamUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) assert.Nil(t, appErr) - rtoken, _, err := th.Client.CreateUserAccessToken(context.Background(), bot.UserId, "test token") + rtoken, _, err := th.Client.CreateUserAccessToken(context.Background(), bot.UserId, "test token", 0) require.NoError(t, err) th.Client.AuthToken = rtoken.Token diff --git a/server/channels/api4/user.go b/server/channels/api4/user.go index 5df095ff0a8..fa26c7b7fae 100644 --- a/server/channels/api4/user.go +++ b/server/channels/api4/user.go @@ -2912,10 +2912,6 @@ func createUserAccessToken(c *Context, w http.ResponseWriter, r *http.Request) { accessToken.UserId = c.Params.UserId accessToken.Token = "" - // TODO: remove once the API officially supports setting expires_at; until - // then, strip any client-supplied value so that JSON-decoded requests cannot - // set an arbitrary (or zero) expiry through the create-token endpoint. - accessToken.ExpiresAt = 0 token, err := c.App.CreateUserAccessToken(c.AppContext, &accessToken) if err != nil { diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go index a0f64f046f0..c85f74c880d 100644 --- a/server/channels/api4/user_test.go +++ b/server/channels/api4/user_test.go @@ -5696,7 +5696,7 @@ func TestCreateUserAccessToken(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) - _, resp, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + _, resp, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.Error(t, err) CheckForbiddenStatus(t, resp) }) @@ -5708,7 +5708,7 @@ func TestCreateUserAccessToken(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { - rtoken, _, err := client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + rtoken, _, err := client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.NoError(t, err) assert.Equal(t, th.BasicUser.Id, rtoken.UserId, "wrong user id") @@ -5727,7 +5727,7 @@ func TestCreateUserAccessToken(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { - _, resp, err := client.CreateUserAccessToken(context.Background(), "notarealuserid", "test token") + _, resp, err := client.CreateUserAccessToken(context.Background(), "notarealuserid", "test token", 0) require.Error(t, err) CheckBadRequestStatus(t, resp) }) @@ -5740,7 +5740,7 @@ func TestCreateUserAccessToken(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { - _, resp, err := client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "") + _, resp, err := client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "", 0) require.Error(t, err) CheckBadRequestStatus(t, resp) }) @@ -5755,7 +5755,7 @@ func TestCreateUserAccessToken(t *testing.T) { require.Nil(t, appErr) th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { - _, resp, err := client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + _, resp, err := client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.Error(t, err) CheckNotImplementedStatus(t, resp) }) @@ -5769,7 +5769,7 @@ func TestCreateUserAccessToken(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - rtoken, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + rtoken, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.NoError(t, err) assert.Equal(t, th.BasicUser.Id, rtoken.UserId, "wrong user id") @@ -5787,7 +5787,7 @@ func TestCreateUserAccessToken(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) - _, resp, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser2.Id, "test token") + _, resp, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser2.Id, "test token", 0) require.Error(t, err) CheckForbiddenStatus(t, resp) }) @@ -5801,7 +5801,7 @@ func TestCreateUserAccessToken(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserManagerRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - rtoken, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser2.Id, "test token") + rtoken, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser2.Id, "test token", 0) require.NoError(t, err) assert.Equal(t, th.BasicUser2.Id, rtoken.UserId) @@ -5820,7 +5820,7 @@ func TestCreateUserAccessToken(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserManagerRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - _, resp, err := th.Client.CreateUserAccessToken(context.Background(), th.SystemAdminUser.Id, "test token") + _, resp, err := th.Client.CreateUserAccessToken(context.Background(), th.SystemAdminUser.Id, "test token", 0) require.Error(t, err) CheckForbiddenStatus(t, resp) }) @@ -5831,7 +5831,7 @@ func TestCreateUserAccessToken(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) - rtoken, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + rtoken, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.NoError(t, err) assert.Equal(t, th.BasicUser.Id, rtoken.UserId) @@ -5856,7 +5856,7 @@ func TestCreateUserAccessToken(t *testing.T) { }) require.Nil(t, appErr) - _, resp, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), remoteUser.Id, "test token") + _, resp, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), remoteUser.Id, "test token", 0) require.Error(t, err) CheckForbiddenStatus(t, resp) // remote users are not allowed to have access tokens }) @@ -5871,7 +5871,7 @@ func TestCreateUserAccessToken(t *testing.T) { session.IsOAuth = true th.App.AddSessionToCache(session) - _, resp, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + _, resp, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.Error(t, err) CheckForbiddenStatus(t, resp) }) @@ -5907,7 +5907,7 @@ func TestCreateUserAccessToken(t *testing.T) { t.Run("without MANAGE_BOT permission", func(t *testing.T) { th.RemovePermissionFromRole(t, model.PermissionManageBots.Id, model.TeamUserRoleId) - _, resp, err = th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + _, resp, err = th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.Error(t, err) CheckForbiddenStatus(t, resp) }) @@ -5915,7 +5915,7 @@ func TestCreateUserAccessToken(t *testing.T) { t.Run("with MANAGE_BOTS permission", func(t *testing.T) { th.AddPermissionToRole(t, model.PermissionManageBots.Id, model.TeamUserRoleId) - token, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + token, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.NoError(t, err) assert.Equal(t, createdBot.UserId, token.UserId) assertToken(t, th, token, createdBot.UserId) @@ -5952,7 +5952,7 @@ func TestCreateUserAccessToken(t *testing.T) { }() t.Run("only having MANAGE_BOTS permission", func(t *testing.T) { - _, resp, err = th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + _, resp, err = th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.Error(t, err) CheckForbiddenStatus(t, resp) }) @@ -5960,13 +5960,131 @@ func TestCreateUserAccessToken(t *testing.T) { t.Run("with MANAGE_OTHERS_BOTS permission", func(t *testing.T) { th.AddPermissionToRole(t, model.PermissionManageOthersBots.Id, model.TeamUserRoleId) - rtoken, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + rtoken, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.NoError(t, err) assert.Equal(t, createdBot.UserId, rtoken.UserId) assertToken(t, th, rtoken, createdBot.UserId) }) }) + + t.Run("create token with a future expires_at", func(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) + + expiresAt := model.GetMillis() + 7*24*60*60*1000 + rtoken, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", expiresAt) + require.NoError(t, err) + assert.Equal(t, expiresAt, rtoken.ExpiresAt) + assert.True(t, rtoken.IsActive) + assertToken(t, th, rtoken, th.BasicUser.Id) + }) + + t.Run("create token with expires_at in the past is rejected", func(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) + + _, resp, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", model.GetMillis()-1000) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + CheckErrorID(t, err, "app.user_access_token.expires_at_in_past.app_error") + }) + + t.Run("create token without expires_at is allowed when no maximum lifetime is set", func(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + // MaximumPersonalAccessTokenLifetimeDays defaults to 0 (no policy), so a + // never-expiring token is accepted. + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) + + rtoken, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) + require.NoError(t, err) + assert.Equal(t, int64(0), rtoken.ExpiresAt) + assert.True(t, rtoken.IsActive) + }) + + t.Run("create token without expires_at is rejected when a maximum lifetime is set", func(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableUserAccessTokens = true + *cfg.ServiceSettings.MaximumPersonalAccessTokenLifetimeDays = 30 + }) + + _, resp, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + CheckErrorID(t, err, "app.user_access_token.expires_at_required.app_error") + }) + + t.Run("create token with expires_at within the maximum lifetime is allowed", func(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableUserAccessTokens = true + *cfg.ServiceSettings.MaximumPersonalAccessTokenLifetimeDays = 30 + }) + + expiresAt := model.GetMillis() + 24*60*60*1000 + rtoken, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", expiresAt) + require.NoError(t, err) + assert.Equal(t, expiresAt, rtoken.ExpiresAt) + }) + + t.Run("create token beyond maximum lifetime is rejected", func(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableUserAccessTokens = true + *cfg.ServiceSettings.MaximumPersonalAccessTokenLifetimeDays = 30 + }) + + // Request expiry 60 days out, well past the 30-day cap. + expiresAt := model.GetMillis() + 60*24*60*60*1000 + _, resp, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", expiresAt) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + CheckErrorID(t, err, "app.user_access_token.expires_at_too_far.app_error") + }) + + t.Run("bot tokens are exempt from expiry enforcement", func(t *testing.T) { + mainHelper.Parallel(t) + th := Setup(t).InitBasic(t) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableUserAccessTokens = true + *cfg.ServiceSettings.EnableBotAccountCreation = true + *cfg.ServiceSettings.MaximumPersonalAccessTokenLifetimeDays = 30 + }) + + createdBot, resp, err := th.SystemAdminClient.CreateBot(context.Background(), &model.Bot{ + Username: GenerateTestUsername(), + DisplayName: "a bot", + Description: "bot", + }) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + defer func() { + appErr := th.App.PermanentDeleteBot(th.Context, createdBot.UserId) + require.Nil(t, appErr) + }() + + // Bot is allowed a non-expiring token even though a max lifetime is + // configured — bots are programmatic clients and bypass the PAT expiry + // policy, matching the existing EnableUserAccessTokens bypass. + rtoken, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), createdBot.UserId, "test bot token", 0) + require.NoError(t, err) + assert.Equal(t, int64(0), rtoken.ExpiresAt) + assert.True(t, rtoken.IsActive) + }) } func TestGetUserAccessToken(t *testing.T) { @@ -6002,7 +6120,7 @@ func TestGetUserAccessToken(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.NoError(t, err) rtoken, _, err := th.Client.GetUserAccessToken(context.Background(), token.Id) @@ -6023,7 +6141,7 @@ func TestGetUserAccessToken(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.NoError(t, err) rtoken, _, err := th.SystemAdminClient.GetUserAccessToken(context.Background(), token.Id) @@ -6065,7 +6183,7 @@ func TestGetUserAccessToken(t *testing.T) { require.Nil(t, appErr) }() - token, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + token, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.NoError(t, err) t.Run("without MANAGE_BOTS permission", func(t *testing.T) { @@ -6118,7 +6236,7 @@ func TestGetUserAccessToken(t *testing.T) { require.Nil(t, appErr) }() - token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.NoError(t, err) t.Run("only having MANAGE_BOTS permission", func(t *testing.T) { @@ -6152,10 +6270,10 @@ func TestGetUserAccessTokensForUser(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - _, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + _, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.NoError(t, err) - _, _, err = th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2") + _, _, err = th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2", 0) require.NoError(t, err) th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { @@ -6178,10 +6296,10 @@ func TestGetUserAccessTokensForUser(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - _, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + _, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.NoError(t, err) - _, _, err = th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2") + _, _, err = th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2", 0) require.NoError(t, err) th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { @@ -6222,10 +6340,10 @@ func TestGetUserAccessTokens(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - _, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2") + _, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2", 0) require.NoError(t, err) - _, _, err = th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2") + _, _, err = th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2", 0) require.NoError(t, err) rtokens, _, err := th.SystemAdminClient.GetUserAccessTokens(context.Background(), 1, 1) @@ -6243,10 +6361,10 @@ func TestGetUserAccessTokens(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - _, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2") + _, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2", 0) require.NoError(t, err) - _, _, err = th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2") + _, _, err = th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token 2", 0) require.NoError(t, err) rtokens, _, err := th.SystemAdminClient.GetUserAccessTokens(context.Background(), 0, 2) @@ -6267,7 +6385,7 @@ func TestSearchUserAccessToken(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, testDescription) + token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, testDescription, 0) require.NoError(t, err) _, resp, err := th.Client.SearchUserAccessTokens(context.Background(), &model.UserAccessTokenSearch{Term: token.Id}) @@ -6307,7 +6425,7 @@ func TestRevokeUserAccessToken(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { - token, _, err := client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + token, _, err := client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.NoError(t, err) assertToken(t, th, token, th.BasicUser.Id) @@ -6324,7 +6442,7 @@ func TestRevokeUserAccessToken(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) - token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser2.Id, "test token") + token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser2.Id, "test token", 0) require.NoError(t, err) resp, err := th.Client.RevokeUserAccessToken(context.Background(), token.Id) @@ -6362,7 +6480,7 @@ func TestRevokeUserAccessToken(t *testing.T) { require.Nil(t, appErr) }() - token, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + token, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.NoError(t, err) t.Run("without MANAGE_BOTS permission", func(t *testing.T) { @@ -6411,7 +6529,7 @@ func TestRevokeUserAccessToken(t *testing.T) { require.Nil(t, appErr) }() - token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.NoError(t, err) t.Run("only having MANAGE_BOTS permission", func(t *testing.T) { @@ -6440,7 +6558,7 @@ func TestDisableUserAccessToken(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.NoError(t, err) assertToken(t, th, token, th.BasicUser.Id) @@ -6456,7 +6574,7 @@ func TestDisableUserAccessToken(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) - token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser2.Id, "test token") + token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser2.Id, "test token", 0) require.NoError(t, err) resp, err := th.Client.DisableUserAccessToken(context.Background(), token.Id) @@ -6494,7 +6612,7 @@ func TestDisableUserAccessToken(t *testing.T) { require.Nil(t, appErr) }() - token, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + token, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.NoError(t, err) t.Run("without MANAGE_BOTS permission", func(t *testing.T) { @@ -6542,7 +6660,7 @@ func TestDisableUserAccessToken(t *testing.T) { require.Nil(t, appErr) }() - token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.NoError(t, err) t.Run("only having MANAGE_BOTS permission", func(t *testing.T) { @@ -6570,7 +6688,7 @@ func TestEnableUserAccessToken(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token") + token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0) require.NoError(t, err) assertToken(t, th, token, th.BasicUser.Id) @@ -6591,7 +6709,7 @@ func TestEnableUserAccessToken(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true }) - token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser2.Id, "test token") + token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), th.BasicUser2.Id, "test token", 0) require.NoError(t, err) _, err = th.SystemAdminClient.DisableUserAccessToken(context.Background(), token.Id) @@ -6632,7 +6750,7 @@ func TestEnableUserAccessToken(t *testing.T) { require.Nil(t, appErr) }() - token, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + token, _, err := th.Client.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.NoError(t, err) _, err = th.Client.DisableUserAccessToken(context.Background(), token.Id) @@ -6684,7 +6802,7 @@ func TestEnableUserAccessToken(t *testing.T) { require.Nil(t, appErr) }() - token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token") + token, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), createdBot.UserId, "test token", 0) require.NoError(t, err) _, err = th.SystemAdminClient.DisableUserAccessToken(context.Background(), token.Id) @@ -6716,7 +6834,7 @@ func TestUserAccessTokenInactiveUser(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, testDescription) + token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, testDescription, 0) require.NoError(t, err) th.Client.AuthToken = token.Token @@ -6742,7 +6860,7 @@ func TestUserAccessTokenDisableConfig(t *testing.T) { _, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false) require.Nil(t, appErr) - token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, testDescription) + token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, testDescription, 0) require.NoError(t, err) oldSessionToken := th.Client.AuthToken @@ -6779,7 +6897,7 @@ func TestUserAccessTokenDisableConfigBotsExcluded(t *testing.T) { require.NoError(t, err) CheckCreatedStatus(t, resp) - rtoken, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), bot.UserId, "test token") + rtoken, _, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), bot.UserId, "test token", 0) th.Client.AuthToken = rtoken.Token require.NoError(t, err) diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index 10bc89acb81..769b2a8de40 100644 --- a/server/channels/app/channel.go +++ b/server/channels/app/channel.go @@ -4563,15 +4563,21 @@ func (a *App) addChannelToDefaultCategory(rctx request.CTX, userID string, chann // Find the original category if the channel is already in a category var originalCategory *model.SidebarCategoryWithChannels for _, category := range categories.Categories { - if category.Type == model.SidebarCategoryCustom && category.Channels != nil && slices.Contains(category.Channels, channel.Id) { + if category.Type != model.SidebarCategoryDirectMessages && slices.Contains(category.Channels, channel.Id) { originalCategory = category break } } + // The channel is already in the target category, so there's nothing to move. + if originalCategory != nil && originalCategory == targetCategory { + return + } + var categoriesToUpdate []*model.SidebarCategoryWithChannels if originalCategory != nil { - originalCategory.Channels = slices.Delete(originalCategory.Channels, slices.Index(originalCategory.Channels, channel.Id), 1) + idx := slices.Index(originalCategory.Channels, channel.Id) + originalCategory.Channels = slices.Delete(originalCategory.Channels, idx, idx+1) categoriesToUpdate = append(categoriesToUpdate, originalCategory) } diff --git a/server/channels/app/channel_test.go b/server/channels/app/channel_test.go index dce2f6e298c..b845a272620 100644 --- a/server/channels/app/channel_test.go +++ b/server/channels/app/channel_test.go @@ -3996,3 +3996,99 @@ func TestPatchChannelWithCategorySorting(t *testing.T) { require.Equal(t, "Disabled Category/Channel Name", patchedChannel.DisplayName) require.Equal(t, "New Category", patchedChannel.DefaultCategoryName) } + +func TestPatchChannelDefaultCategoryMovesOutOfChannels(t *testing.T) { + th := Setup(t).InitBasic(t) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.TeamSettings.EnableChannelCategorySorting = true + }) + + // Create several channels so the target sits at an index > 0 in the default Channels category, + // exercising the path that previously panicked when removing the channel from its original category. + var channels []*model.Channel + for range 3 { + c := th.createChannel(t, th.BasicTeam, model.ChannelTypeOpen) + channels = append(channels, c) + } + + categories, appErr := th.App.GetSidebarCategoriesForTeamForUser(th.Context, th.BasicUser.Id, th.BasicTeam.Id) + require.Nil(t, appErr) + + var channelsCategory *model.SidebarCategoryWithChannels + for _, category := range categories.Categories { + if category.Type == model.SidebarCategoryChannels { + channelsCategory = category + break + } + } + require.NotNil(t, channelsCategory) + + created := make(map[string]*model.Channel, len(channels)) + for _, c := range channels { + created[c.Id] = c + } + + // Pick the created channel sitting at the highest index in the default Channels category so the + // removal path operates on an index > 1, which previously triggered an out-of-range panic. + var target *model.Channel + targetIndex := -1 + for i, channelID := range channelsCategory.Channels { + if c, ok := created[channelID]; ok && i > targetIndex { + target = c + targetIndex = i + } + } + require.NotNil(t, target) + require.Greater(t, targetIndex, 1, "expected a created channel beyond index 1 to exercise the removal path") + + patch := &model.ChannelPatch{DefaultCategoryName: new("Moved Category")} + _, appErr = th.App.PatchChannel(th.Context, target, patch, th.BasicUser.Id) + require.Nil(t, appErr) + + categories, appErr = th.App.GetSidebarCategoriesForTeamForUser(th.Context, th.BasicUser.Id, th.BasicTeam.Id) + require.Nil(t, appErr) + + for _, category := range categories.Categories { + switch { + case category.DisplayName == "Moved Category": + assert.Contains(t, category.Channels, target.Id, "channel should be in the new default category") + case category.Type == model.SidebarCategoryChannels: + assert.NotContains(t, category.Channels, target.Id, "channel should no longer be in the default Channels category") + } + } +} + +func TestPatchChannelDefaultCategoryReapplyIsIdempotent(t *testing.T) { + th := Setup(t).InitBasic(t) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.TeamSettings.EnableChannelCategorySorting = true + }) + + channel := th.createChannel(t, th.BasicTeam, model.ChannelTypeOpen) + + patch := &model.ChannelPatch{DefaultCategoryName: new("Reapply Category")} + channel, appErr := th.App.PatchChannel(th.Context, channel, patch, th.BasicUser.Id) + require.Nil(t, appErr) + + // Patching again re-runs the default category logic while the channel is already in the target + // category. This must not error or list the channel twice in the category. + _, appErr = th.App.PatchChannel(th.Context, channel, &model.ChannelPatch{Header: new("updated header")}, th.BasicUser.Id) + require.Nil(t, appErr) + + categories, appErr := th.App.GetSidebarCategoriesForTeamForUser(th.Context, th.BasicUser.Id, th.BasicTeam.Id) + require.Nil(t, appErr) + + for _, category := range categories.Categories { + if category.DisplayName == "Reapply Category" { + count := 0 + for _, channelID := range category.Channels { + if channelID == channel.Id { + count++ + } + } + assert.Equal(t, 1, count, "channel should appear exactly once in the default category") + } + } +} diff --git a/server/channels/app/session.go b/server/channels/app/session.go index 0068cd3e49e..815cb364c2b 100644 --- a/server/channels/app/session.go +++ b/server/channels/app/session.go @@ -401,6 +401,49 @@ func (a *App) SetSessionExpireInHours(session *model.Session, hours int) { a.ch.srv.platform.SetSessionExpireInHours(session, hours) } +// validateUserAccessTokenExpiry checks a token's ExpiresAt against +// ServiceSettings.MaximumPersonalAccessTokenLifetimeDays. 0 means no policy: +// never-expiring tokens are allowed. A value > 0 requires the token to expire +// within that many days, so ExpiresAt == 0 or an expiry beyond the cap is +// rejected. Only newly created tokens are checked; existing tokens are not +// re-validated. +func (a *App) validateUserAccessTokenExpiry(token *model.UserAccessToken) *model.AppError { + cfg := a.Config().ServiceSettings + + maxDays := int64(0) + if cfg.MaximumPersonalAccessTokenLifetimeDays != nil { + maxDays = int64(*cfg.MaximumPersonalAccessTokenLifetimeDays) + } + + if token.ExpiresAt == 0 { + // A configured maximum lifetime implies tokens must expire; never-expiring + // tokens are only allowed when no maximum is set. + if maxDays > 0 { + return model.NewAppError("CreateUserAccessToken", "app.user_access_token.expires_at_required.app_error", nil, "", http.StatusBadRequest) + } + return nil + } + + if token.ExpiresAt <= model.GetMillis() { + return model.NewAppError("CreateUserAccessToken", "app.user_access_token.expires_at_in_past.app_error", nil, "", http.StatusBadRequest) + } + + if maxDays > 0 { + maxExpiry := model.GetMillis() + maxDays*24*60*60*1000 + if token.ExpiresAt > maxExpiry { + return model.NewAppError( + "CreateUserAccessToken", + "app.user_access_token.expires_at_too_far.app_error", + map[string]any{"Days": maxDays}, + "", + http.StatusBadRequest, + ) + } + } + + return nil +} + func (a *App) CreateUserAccessToken(rctx request.CTX, token *model.UserAccessToken) (*model.UserAccessToken, *model.AppError) { user, nErr := a.ch.srv.userService.GetUser(token.UserId) if nErr != nil { @@ -417,6 +460,16 @@ func (a *App) CreateUserAccessToken(rctx request.CTX, token *model.UserAccessTok return nil, model.NewAppError("CreateUserAccessToken", "app.user_access_token.disabled", nil, "", http.StatusNotImplemented) } + // Bot accounts are exempt from the PAT expiry policy, matching the existing + // EnableUserAccessTokens bypass above: bots are programmatic clients that + // typically need long-lived credentials, and integrations that provision + // them would otherwise break the moment an admin enables enforcement. + if !user.IsBot { + if err := a.validateUserAccessTokenExpiry(token); err != nil { + return nil, err + } + } + token.Token = model.NewId() token, nErr = a.Srv().Store().UserAccessToken().Save(token) diff --git a/server/cmd/mmctl/client/client.go b/server/cmd/mmctl/client/client.go index 4f157884068..1b8df0bea34 100644 --- a/server/cmd/mmctl/client/client.go +++ b/server/cmd/mmctl/client/client.go @@ -82,7 +82,7 @@ type Client interface { UpdateUserMfa(ctx context.Context, userID, code string, activate bool) (*model.Response, error) UpdateUserPassword(ctx context.Context, userID, currentPassword, newPassword string) (*model.Response, error) UpdateUserHashedPassword(ctx context.Context, userID, newHashedPassword string) (*model.Response, error) - CreateUserAccessToken(ctx context.Context, userID, description string) (*model.UserAccessToken, *model.Response, error) + CreateUserAccessToken(ctx context.Context, userID, description string, expiresAt int64) (*model.UserAccessToken, *model.Response, error) RevokeUserAccessToken(ctx context.Context, tokenID string) (*model.Response, error) GetUserAccessTokensForUser(ctx context.Context, userID string, page, perPage int) ([]*model.UserAccessToken, *model.Response, error) ConvertUserToBot(ctx context.Context, userID string) (*model.Bot, *model.Response, error) diff --git a/server/cmd/mmctl/commands/bot_test.go b/server/cmd/mmctl/commands/bot_test.go index 14bb4fb56e4..4eeb086e9be 100644 --- a/server/cmd/mmctl/commands/bot_test.go +++ b/server/cmd/mmctl/commands/bot_test.go @@ -64,7 +64,7 @@ func (s *MmctlUnitTestSuite) TestBotCreateCmd() { s.client. EXPECT(). - CreateUserAccessToken(context.TODO(), mockBot.UserId, "autogenerated"). + CreateUserAccessToken(context.TODO(), mockBot.UserId, "autogenerated", int64(0)). Return(&mockToken, &model.Response{}, nil). Times(1) diff --git a/server/cmd/mmctl/commands/token.go b/server/cmd/mmctl/commands/token.go index 8f80c62fb9a..b8d4c6adfe4 100644 --- a/server/cmd/mmctl/commands/token.go +++ b/server/cmd/mmctl/commands/token.go @@ -6,7 +6,11 @@ package commands import ( "context" "net/http" + "strconv" + "strings" + "time" + "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/v8/cmd/mmctl/client" "github.com/mattermost/mattermost/server/v8/cmd/mmctl/printer" @@ -20,12 +24,15 @@ var TokenCmd = &cobra.Command{ } var GenerateUserTokenCmd = &cobra.Command{ - Use: "generate [user] [description]", - Short: "Generate token for a user", - Long: "Generate token for a user", - Example: " generate testuser test-token", - RunE: withClient(generateTokenForAUserCmdF), - Args: cobra.ExactArgs(2), + Use: "generate [user] [description]", + Short: "Generate token for a user", + Long: "Generate token for a user. Use --expires-in to set an expiry, which may be required by " + + "the server's MaximumPersonalAccessTokenLifetimeDays setting.", + Example: ` generate testuser test-token + generate testuser ci-token --expires-in 90d + generate testuser short-lived --expires-in 12h`, + RunE: withClient(generateTokenForAUserCmdF), + Args: cobra.ExactArgs(2), } var RevokeUserTokenCmd = &cobra.Command{ @@ -47,6 +54,8 @@ var ListUserTokensCmd = &cobra.Command{ } func init() { + GenerateUserTokenCmd.Flags().String("expires-in", "", "Duration after which the token expires (e.g. 90d, 12h, 30m). Accepts the standard Go duration syntax plus a 'd' (days) suffix. If empty, the token does not expire.") + ListUserTokensCmd.Flags().Int("page", 0, "Page number to fetch for the list of users") ListUserTokensCmd.Flags().Int("per-page", DefaultPageSize, "Number of users to be fetched") ListUserTokensCmd.Flags().Bool("all", false, "Fetch all tokens. --page flag will be ignore if provided") @@ -65,13 +74,18 @@ func init() { } func generateTokenForAUserCmdF(c client.Client, command *cobra.Command, args []string) error { + expiresAt, err := resolveTokenExpiry(command) + if err != nil { + return err + } + userArg := args[0] user := getUserFromUserArg(c, userArg) if user == nil { return errors.Errorf("could not retrieve user information of %q", userArg) } - token, _, err := c.CreateUserAccessToken(context.TODO(), user.Id, args[1]) + token, _, err := c.CreateUserAccessToken(context.TODO(), user.Id, args[1], expiresAt) if err != nil { return errors.Errorf("could not create token for %q: %s", userArg, err.Error()) } @@ -80,6 +94,46 @@ func generateTokenForAUserCmdF(c client.Client, command *cobra.Command, args []s return nil } +// resolveTokenExpiry converts the --expires-in flag into a Unix-millis timestamp +// suitable for UserAccessToken.ExpiresAt. Returns 0 when the flag is empty, +// meaning the token does not expire (subject to server policy). +func resolveTokenExpiry(command *cobra.Command) (int64, error) { + raw, _ := command.Flags().GetString("expires-in") + raw = strings.TrimSpace(raw) + if raw == "" { + return 0, nil + } + d, err := parseExpiresIn(raw) + if err != nil { + return 0, errors.Wrap(err, "invalid --expires-in value") + } + if d <= 0 { + return 0, errors.Errorf("--expires-in must be positive, got %q", raw) + } + return time.Now().Add(d).UnixMilli(), nil +} + +// parseExpiresIn accepts a duration string. In addition to the standard Go +// duration syntax (e.g. "12h", "30m"), a trailing "d" is interpreted as days +// — the common case for token lifetimes which stdlib's time.ParseDuration +// does not support. +func parseExpiresIn(s string) (time.Duration, error) { + if prefix, ok := strings.CutSuffix(s, "d"); ok { + days, err := strconv.Atoi(prefix) + if err != nil { + return 0, errors.Errorf("%q is not a valid day count", s) + } + // time.Duration is int64 nanoseconds; days * 24h overflows past ~106751. + // Cap at the server-side bound so the CLI rejects values that the server + // would reject anyway, well below the int64-overflow point. + if days > model.MaxPersonalAccessTokenLifetimeDays { + return 0, errors.Errorf("%q exceeds maximum supported day count of %d", s, model.MaxPersonalAccessTokenLifetimeDays) + } + return time.Duration(days) * 24 * time.Hour, nil + } + return time.ParseDuration(s) +} + func listTokensOfAUserCmdF(c client.Client, command *cobra.Command, args []string) error { page, _ := command.Flags().GetInt("page") perPage, _ := command.Flags().GetInt("per-page") diff --git a/server/cmd/mmctl/commands/token_test.go b/server/cmd/mmctl/commands/token_test.go index 1778ae60f8d..8cff5ae6730 100644 --- a/server/cmd/mmctl/commands/token_test.go +++ b/server/cmd/mmctl/commands/token_test.go @@ -7,15 +7,48 @@ import ( "context" "fmt" "net/http" + "testing" + "time" + gomock "github.com/golang/mock/gomock" "github.com/mattermost/mattermost/server/public/model" "github.com/pkg/errors" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/v8/cmd/mmctl/printer" "github.com/spf13/cobra" ) +func TestParseExpiresIn(t *testing.T) { + for name, tc := range map[string]struct { + input string + want time.Duration + wantErr bool + }{ + "days": {input: "30d", want: 30 * 24 * time.Hour}, + "single day": {input: "1d", want: 24 * time.Hour}, + "hours": {input: "12h", want: 12 * time.Hour}, + "compound stdlib": {input: "1h30m", want: 90 * time.Minute}, + "minutes": {input: "45m", want: 45 * time.Minute}, + "non-numeric days": {input: "xd", wantErr: true}, + "non-numeric stdlib": {input: "abc", wantErr: true}, + "empty": {input: "", wantErr: true}, + "days beyond cap": {input: "36501d", wantErr: true}, + "days at cap accepted": {input: "36500d", want: 36500 * 24 * time.Hour}, + } { + t.Run(name, func(t *testing.T) { + got, err := parseExpiresIn(tc.input) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } +} + func (s *MmctlUnitTestSuite) TestGenerateTokenForAUserCmd() { s.Run("Should generate a token for a user", func() { printer.Clean() @@ -38,7 +71,7 @@ func (s *MmctlUnitTestSuite) TestGenerateTokenForAUserCmd() { s.client. EXPECT(). - CreateUserAccessToken(context.TODO(), mockUser.Id, mockToken.Description). + CreateUserAccessToken(context.TODO(), mockUser.Id, mockToken.Description, int64(0)). Return(&mockToken, &model.Response{}, nil). Times(1) @@ -83,7 +116,7 @@ func (s *MmctlUnitTestSuite) TestGenerateTokenForAUserCmd() { s.client. EXPECT(). - CreateUserAccessToken(context.TODO(), mockUser.Id, "description"). + CreateUserAccessToken(context.TODO(), mockUser.Id, "description", int64(0)). Return(nil, &model.Response{}, errors.New("error-message")). Times(1) @@ -91,6 +124,68 @@ func (s *MmctlUnitTestSuite) TestGenerateTokenForAUserCmd() { s.Require().NotNil(err) s.Require().Contains(err.Error(), fmt.Sprintf("could not create token for %q:", "user1")) }) + + s.Run("Should pass a positive expires_at when --expires-in is set", func() { + printer.Clean() + + userArg := "userId1" + mockUser := model.User{Id: "userId1", Email: "user1@example.com", Username: "user1"} + mockToken := model.UserAccessToken{Token: "token-id", Description: "ci-token"} + + s.client. + EXPECT(). + GetUserByUsername(context.TODO(), userArg, ""). + Return(nil, &model.Response{}, errors.New("no user found with the given username")). + Times(1) + s.client. + EXPECT(). + GetUser(context.TODO(), userArg, ""). + Return(&mockUser, &model.Response{}, nil). + Times(1) + + now := time.Now() + s.client. + EXPECT(). + CreateUserAccessToken(context.TODO(), mockUser.Id, mockToken.Description, gomock.AssignableToTypeOf(int64(0))). + DoAndReturn(func(_ context.Context, _, _ string, expiresAt int64) (*model.UserAccessToken, *model.Response, error) { + // 90 days = 7_776_000_000 ms; allow generous slack for clock between Now() calls. + expected := now.Add(90 * 24 * time.Hour).UnixMilli() + s.Require().InDelta(expected, expiresAt, float64(time.Minute.Milliseconds())) + return &mockToken, &model.Response{}, nil + }). + Times(1) + + cmd := &cobra.Command{} + cmd.Flags().String("expires-in", "", "") + s.Require().NoError(cmd.Flags().Set("expires-in", "90d")) + + err := generateTokenForAUserCmdF(s.client, cmd, []string{mockUser.Id, mockToken.Description}) + s.Require().NoError(err) + }) + + s.Run("Should reject invalid --expires-in", func() { + printer.Clean() + + cmd := &cobra.Command{} + cmd.Flags().String("expires-in", "", "") + s.Require().NoError(cmd.Flags().Set("expires-in", "not-a-duration")) + + err := generateTokenForAUserCmdF(s.client, cmd, []string{"userId1", "desc"}) + s.Require().Error(err) + s.Require().Contains(err.Error(), "invalid --expires-in") + }) + + s.Run("Should reject non-positive --expires-in", func() { + printer.Clean() + + cmd := &cobra.Command{} + cmd.Flags().String("expires-in", "", "") + s.Require().NoError(cmd.Flags().Set("expires-in", "-5h")) + + err := generateTokenForAUserCmdF(s.client, cmd, []string{"userId1", "desc"}) + s.Require().Error(err) + s.Require().Contains(err.Error(), "must be positive") + }) } func (s *MmctlUnitTestSuite) TestListTokensOfAUserCmdF() { diff --git a/server/cmd/mmctl/docs/mmctl_token_generate.rst b/server/cmd/mmctl/docs/mmctl_token_generate.rst index cb31a4292b8..1224a11132e 100644 --- a/server/cmd/mmctl/docs/mmctl_token_generate.rst +++ b/server/cmd/mmctl/docs/mmctl_token_generate.rst @@ -9,7 +9,7 @@ Synopsis ~~~~~~~~ -Generate token for a user +Generate token for a user. Use --expires-in to set an expiry, which may be required by the server's MaximumPersonalAccessTokenLifetimeDays setting. :: @@ -21,13 +21,16 @@ Examples :: generate testuser test-token + generate testuser ci-token --expires-in 90d + generate testuser short-lived --expires-in 12h Options ~~~~~~~ :: - -h, --help help for generate + --expires-in string Duration after which the token expires (e.g. 90d, 12h, 30m). Accepts the standard Go duration syntax plus a 'd' (days) suffix. If empty, the token does not expire. + -h, --help help for generate Options inherited from parent commands ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/server/cmd/mmctl/mocks/client_mock.go b/server/cmd/mmctl/mocks/client_mock.go index de9182a593c..4b0f4dcb50e 100644 --- a/server/cmd/mmctl/mocks/client_mock.go +++ b/server/cmd/mmctl/mocks/client_mock.go @@ -344,9 +344,9 @@ func (mr *MockClientMockRecorder) CreateUser(arg0, arg1 interface{}) *gomock.Cal } // CreateUserAccessToken mocks base method. -func (m *MockClient) CreateUserAccessToken(arg0 context.Context, arg1, arg2 string) (*model.UserAccessToken, *model.Response, error) { +func (m *MockClient) CreateUserAccessToken(arg0 context.Context, arg1, arg2 string, arg3 int64) (*model.UserAccessToken, *model.Response, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateUserAccessToken", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "CreateUserAccessToken", arg0, arg1, arg2, arg3) ret0, _ := ret[0].(*model.UserAccessToken) ret1, _ := ret[1].(*model.Response) ret2, _ := ret[2].(error) @@ -354,9 +354,9 @@ func (m *MockClient) CreateUserAccessToken(arg0 context.Context, arg1, arg2 stri } // CreateUserAccessToken indicates an expected call of CreateUserAccessToken. -func (mr *MockClientMockRecorder) CreateUserAccessToken(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) CreateUserAccessToken(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateUserAccessToken", reflect.TypeOf((*MockClient)(nil).CreateUserAccessToken), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateUserAccessToken", reflect.TypeOf((*MockClient)(nil).CreateUserAccessToken), arg0, arg1, arg2, arg3) } // DeleteCPAField mocks base method. diff --git a/server/config/client.go b/server/config/client.go index 5b255f1af97..d3027ec7c1b 100644 --- a/server/config/client.go +++ b/server/config/client.go @@ -37,6 +37,7 @@ func GenerateClientConfig(c *model.Config, telemetryID string, license *model.Li props["EnablePostUsernameOverride"] = strconv.FormatBool(*c.ServiceSettings.EnablePostUsernameOverride) props["EnablePostIconOverride"] = strconv.FormatBool(*c.ServiceSettings.EnablePostIconOverride) props["EnableUserAccessTokens"] = strconv.FormatBool(*c.ServiceSettings.EnableUserAccessTokens) + props["MaximumPersonalAccessTokenLifetimeDays"] = strconv.Itoa(*c.ServiceSettings.MaximumPersonalAccessTokenLifetimeDays) props["EnableLinkPreviews"] = strconv.FormatBool(*c.ServiceSettings.EnableLinkPreviews) props["EnablePermalinkPreviews"] = strconv.FormatBool(*c.ServiceSettings.EnablePermalinkPreviews) props["EnableTesting"] = strconv.FormatBool(*c.ServiceSettings.EnableTesting) diff --git a/server/i18n/en.json b/server/i18n/en.json index 32d54ef9d7b..e04562bfa22 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -9550,6 +9550,18 @@ "id": "app.user_access_token.expired", "translation": "The personal access token has expired." }, + { + "id": "app.user_access_token.expires_at_in_past.app_error", + "translation": "The provided expires_at is in the past." + }, + { + "id": "app.user_access_token.expires_at_required.app_error", + "translation": "Personal access tokens must include an expiry date because the server enforces a maximum token lifetime." + }, + { + "id": "app.user_access_token.expires_at_too_far.app_error", + "translation": "The provided expires_at exceeds the configured maximum personal access token lifetime ({{.Days}} days)." + }, { "id": "app.user_access_token.get_all.app_error", "translation": "Unable to get all personal access tokens." @@ -11774,6 +11786,10 @@ "id": "model.config.is_valid.max_payload_size.app_error", "translation": "Invalid max payload size for service settings. Must be a whole number greater than zero." }, + { + "id": "model.config.is_valid.max_personal_access_token_lifetime_days.app_error", + "translation": "Invalid MaximumPersonalAccessTokenLifetimeDays. Must be between 0 (unlimited) and {{.Max}} days." + }, { "id": "model.config.is_valid.max_url_length.app_error", "translation": "Invalid max URL length for service settings. Must be a whole number greater than zero." diff --git a/server/public/model/client4.go b/server/public/model/client4.go index 38371edda01..fee1be13840 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -1891,8 +1891,12 @@ func (c *Client4) SetProfileImage(ctx context.Context, userId string, data []byt // of a session token to access the REST API. Must have the 'create_user_access_token' // permission and if generating for another user, must have the 'edit_other_users' // permission. A non-blank description is required. -func (c *Client4) CreateUserAccessToken(ctx context.Context, userId, description string) (*UserAccessToken, *Response, error) { - requestBody := map[string]string{"description": description} +// +// expiresAt is the Unix-millis expiry for the token; 0 means the token does not +// expire, subject to server policy (ServiceSettings.MaximumPersonalAccessTokenLifetimeDays: +// a value > 0 requires tokens to expire within that many days and rejects 0). +func (c *Client4) CreateUserAccessToken(ctx context.Context, userId, description string, expiresAt int64) (*UserAccessToken, *Response, error) { + requestBody := &UserAccessToken{Description: description, ExpiresAt: expiresAt} r, err := c.doAPIPostJSON(ctx, c.userRoute(userId).Join("tokens"), requestBody) if err != nil { return nil, BuildResponse(r), err diff --git a/server/public/model/config.go b/server/public/model/config.go index 38721d7e1bb..0bc6e4b09ea 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -319,6 +319,12 @@ const ( StorageClassGlacierIR = "GLACIER_IR" StorageClassSnow = "SNOW" StorageClassExpressOnezone = "EXPRESS_ONEZONE" + + // MaxPersonalAccessTokenLifetimeDays is the upper bound accepted for + // ServiceSettings.MaximumPersonalAccessTokenLifetimeDays. 100 years is well + // past any realistic operational use and leaves ample headroom against + // int64 overflow when computing token expiry millis. + MaxPersonalAccessTokenLifetimeDays = 36500 ) func GetDefaultAppCustomURLSchemes() []string { @@ -361,48 +367,49 @@ type ServiceSettings struct { TLSMinVer *string `access:"write_restrictable,cloud_restrictable"` // telemetry: none TLSStrictTransport *bool `access:"write_restrictable,cloud_restrictable"` // In seconds. - TLSStrictTransportMaxAge *int64 `access:"write_restrictable,cloud_restrictable"` // telemetry: none - TLSOverwriteCiphers []string `access:"write_restrictable,cloud_restrictable"` // telemetry: none - UseLetsEncrypt *bool `access:"environment_web_server,write_restrictable,cloud_restrictable"` - LetsEncryptCertificateCacheFile *string `access:"environment_web_server,write_restrictable,cloud_restrictable"` // telemetry: none - Forward80To443 *bool `access:"environment_web_server,write_restrictable,cloud_restrictable"` - TrustedProxyIPHeader []string `access:"write_restrictable,cloud_restrictable"` // telemetry: none - ReadTimeout *int `access:"environment_web_server,write_restrictable,cloud_restrictable"` - WriteTimeout *int `access:"environment_web_server,write_restrictable,cloud_restrictable"` - IdleTimeout *int `access:"write_restrictable,cloud_restrictable"` - MaximumLoginAttempts *int `access:"authentication_password,write_restrictable,cloud_restrictable"` - GoroutineHealthThreshold *int `access:"write_restrictable,cloud_restrictable"` // telemetry: none - EnableOAuthServiceProvider *bool `access:"integrations_integration_management"` - EnableDynamicClientRegistration *bool `access:"integrations_integration_management"` - DCRRedirectURIAllowlist []string `access:"integrations_integration_management"` - EnableIncomingWebhooks *bool `access:"integrations_integration_management"` - EnableOutgoingWebhooks *bool `access:"integrations_integration_management"` - EnableOutgoingOAuthConnections *bool `access:"integrations_integration_management"` - EnableCommands *bool `access:"integrations_integration_management"` - OutgoingIntegrationRequestsTimeout *int64 `access:"integrations_integration_management"` // In seconds. - EnablePostUsernameOverride *bool `access:"integrations_integration_management"` - EnablePostIconOverride *bool `access:"integrations_integration_management"` - GoogleDeveloperKey *string `access:"site_posts,write_restrictable,cloud_restrictable"` - EnableLinkPreviews *bool `access:"site_posts"` - EnablePermalinkPreviews *bool `access:"site_posts"` - RestrictLinkPreviews *string `access:"site_posts"` - EnableTesting *bool `access:"environment_developer,write_restrictable,cloud_restrictable"` - EnableDeveloper *bool `access:"environment_developer,write_restrictable,cloud_restrictable"` - DeveloperFlags *string `access:"environment_developer,cloud_restrictable"` - EnableClientPerformanceDebugging *bool `access:"environment_developer,write_restrictable,cloud_restrictable"` - EnableSecurityFixAlert *bool `access:"environment_smtp,write_restrictable,cloud_restrictable"` - EnableInsecureOutgoingConnections *bool `access:"environment_web_server,write_restrictable,cloud_restrictable"` - AllowedUntrustedInternalConnections *string `access:"environment_web_server,write_restrictable,cloud_restrictable"` - EnableMultifactorAuthentication *bool `access:"authentication_mfa"` - EnforceMultifactorAuthentication *bool `access:"authentication_mfa"` - EnableUserAccessTokens *bool `access:"integrations_integration_management"` - AllowCorsFrom *string `access:"integrations_cors,write_restrictable,cloud_restrictable"` - CorsExposedHeaders *string `access:"integrations_cors,write_restrictable,cloud_restrictable"` - CorsAllowCredentials *bool `access:"integrations_cors,write_restrictable,cloud_restrictable"` - CorsDebug *bool `access:"integrations_cors,write_restrictable,cloud_restrictable"` - AllowCookiesForSubdomains *bool `access:"write_restrictable,cloud_restrictable"` - ExtendSessionLengthWithActivity *bool `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` - TerminateSessionsOnPasswordChange *bool `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` + TLSStrictTransportMaxAge *int64 `access:"write_restrictable,cloud_restrictable"` // telemetry: none + TLSOverwriteCiphers []string `access:"write_restrictable,cloud_restrictable"` // telemetry: none + UseLetsEncrypt *bool `access:"environment_web_server,write_restrictable,cloud_restrictable"` + LetsEncryptCertificateCacheFile *string `access:"environment_web_server,write_restrictable,cloud_restrictable"` // telemetry: none + Forward80To443 *bool `access:"environment_web_server,write_restrictable,cloud_restrictable"` + TrustedProxyIPHeader []string `access:"write_restrictable,cloud_restrictable"` // telemetry: none + ReadTimeout *int `access:"environment_web_server,write_restrictable,cloud_restrictable"` + WriteTimeout *int `access:"environment_web_server,write_restrictable,cloud_restrictable"` + IdleTimeout *int `access:"write_restrictable,cloud_restrictable"` + MaximumLoginAttempts *int `access:"authentication_password,write_restrictable,cloud_restrictable"` + GoroutineHealthThreshold *int `access:"write_restrictable,cloud_restrictable"` // telemetry: none + EnableOAuthServiceProvider *bool `access:"integrations_integration_management"` + EnableDynamicClientRegistration *bool `access:"integrations_integration_management"` + DCRRedirectURIAllowlist []string `access:"integrations_integration_management"` + EnableIncomingWebhooks *bool `access:"integrations_integration_management"` + EnableOutgoingWebhooks *bool `access:"integrations_integration_management"` + EnableOutgoingOAuthConnections *bool `access:"integrations_integration_management"` + EnableCommands *bool `access:"integrations_integration_management"` + OutgoingIntegrationRequestsTimeout *int64 `access:"integrations_integration_management"` // In seconds. + EnablePostUsernameOverride *bool `access:"integrations_integration_management"` + EnablePostIconOverride *bool `access:"integrations_integration_management"` + GoogleDeveloperKey *string `access:"site_posts,write_restrictable,cloud_restrictable"` + EnableLinkPreviews *bool `access:"site_posts"` + EnablePermalinkPreviews *bool `access:"site_posts"` + RestrictLinkPreviews *string `access:"site_posts"` + EnableTesting *bool `access:"environment_developer,write_restrictable,cloud_restrictable"` + EnableDeveloper *bool `access:"environment_developer,write_restrictable,cloud_restrictable"` + DeveloperFlags *string `access:"environment_developer,cloud_restrictable"` + EnableClientPerformanceDebugging *bool `access:"environment_developer,write_restrictable,cloud_restrictable"` + EnableSecurityFixAlert *bool `access:"environment_smtp,write_restrictable,cloud_restrictable"` + EnableInsecureOutgoingConnections *bool `access:"environment_web_server,write_restrictable,cloud_restrictable"` + AllowedUntrustedInternalConnections *string `access:"environment_web_server,write_restrictable,cloud_restrictable"` + EnableMultifactorAuthentication *bool `access:"authentication_mfa"` + EnforceMultifactorAuthentication *bool `access:"authentication_mfa"` + EnableUserAccessTokens *bool `access:"integrations_integration_management"` + MaximumPersonalAccessTokenLifetimeDays *int `access:"integrations_integration_management"` + AllowCorsFrom *string `access:"integrations_cors,write_restrictable,cloud_restrictable"` + CorsExposedHeaders *string `access:"integrations_cors,write_restrictable,cloud_restrictable"` + CorsAllowCredentials *bool `access:"integrations_cors,write_restrictable,cloud_restrictable"` + CorsDebug *bool `access:"integrations_cors,write_restrictable,cloud_restrictable"` + AllowCookiesForSubdomains *bool `access:"write_restrictable,cloud_restrictable"` + ExtendSessionLengthWithActivity *bool `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` + TerminateSessionsOnPasswordChange *bool `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` // Deprecated SessionLengthWebInDays *int `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` // telemetry: none @@ -569,6 +576,10 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) { s.EnableUserAccessTokens = new(false) } + if s.MaximumPersonalAccessTokenLifetimeDays == nil { + s.MaximumPersonalAccessTokenLifetimeDays = new(0) + } + if s.GoroutineHealthThreshold == nil { s.GoroutineHealthThreshold = new(-1) } @@ -4984,6 +4995,14 @@ func (s *ServiceSettings) isValid() *AppError { } } + // MaximumPersonalAccessTokenLifetimeDays: 0 means unlimited; negative is + // nonsensical; an upper bound of MaxPersonalAccessTokenLifetimeDays guards + // against int64 overflow when computing now + days*86_400_000 millis at + // token-creation time. + if *s.MaximumPersonalAccessTokenLifetimeDays < 0 || *s.MaximumPersonalAccessTokenLifetimeDays > MaxPersonalAccessTokenLifetimeDays { + return NewAppError("Config.IsValid", "model.config.is_valid.max_personal_access_token_lifetime_days.app_error", map[string]any{"Max": MaxPersonalAccessTokenLifetimeDays}, "", http.StatusBadRequest) + } + return nil } diff --git a/server/public/model/config_test.go b/server/public/model/config_test.go index fa34fa9cc66..52cca449ca5 100644 --- a/server/public/model/config_test.go +++ b/server/public/model/config_test.go @@ -148,6 +148,30 @@ func TestServiceSettingsIsValid(t *testing.T) { }, ExpectError: false, }, + "MaximumPersonalAccessTokenLifetimeDays zero (unlimited) is accepted": { + ServiceSettings: ServiceSettings{ + MaximumPersonalAccessTokenLifetimeDays: new(0), + }, + ExpectError: false, + }, + "MaximumPersonalAccessTokenLifetimeDays negative is rejected": { + ServiceSettings: ServiceSettings{ + MaximumPersonalAccessTokenLifetimeDays: new(-1), + }, + ExpectError: true, + }, + "MaximumPersonalAccessTokenLifetimeDays at upper bound is accepted": { + ServiceSettings: ServiceSettings{ + MaximumPersonalAccessTokenLifetimeDays: new(MaxPersonalAccessTokenLifetimeDays), + }, + ExpectError: false, + }, + "MaximumPersonalAccessTokenLifetimeDays beyond upper bound is rejected": { + ServiceSettings: ServiceSettings{ + MaximumPersonalAccessTokenLifetimeDays: new(MaxPersonalAccessTokenLifetimeDays + 1), + }, + ExpectError: true, + }, } { t.Run(name, func(t *testing.T) { test.ServiceSettings.SetDefaults(false) diff --git a/webapp/channels/src/components/dynamic_virtualized_list/README.md b/webapp/channels/src/components/dynamic_virtualized_list/README.md new file mode 100644 index 00000000000..326f8e12442 --- /dev/null +++ b/webapp/channels/src/components/dynamic_virtualized_list/README.md @@ -0,0 +1,17 @@ +# dynamic_virtualized_list + +A virtualized list that supports **dynamically-sized, unmeasured items** — rows whose +heights are not known ahead of time and are measured as they render. It is the rendering +engine behind Mattermost's long, variable-height message lists. + +## Origin + +This package was built from a fork of an alpha version of [`react-window`](https://github.com/bvaughn/react-window), +but took a substantially different approach — using relative rather than absolute positioning to render items whose sizes aren't known ahead of time. + +## Files + +| File | Purpose | +| --- | --- | +| `list_item.tsx` | Wrapper around each rendered row that reports its measured size back to the list. | +| `list_item_size_observer.ts` | Tracks rendered item sizes so the list can position rows without pre-measuring them. | diff --git a/webapp/channels/src/components/file_preview_modal/__snapshots__/file_preview_modal.test.tsx.snap b/webapp/channels/src/components/file_preview_modal/__snapshots__/file_preview_modal.test.tsx.snap index 8127eba41ec..a05ad4c5ec3 100644 --- a/webapp/channels/src/components/file_preview_modal/__snapshots__/file_preview_modal.test.tsx.snap +++ b/webapp/channels/src/components/file_preview_modal/__snapshots__/file_preview_modal.test.tsx.snap @@ -130,6 +130,9 @@ exports[`components/FilePreviewModal should match snapshot, loaded 1`] = `