From c3c81cb3d6a29ef8164a8e7ef4ba172d60117176 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Tue, 6 Dec 2022 12:44:48 -0500 Subject: [PATCH 1/6] MM-48120/MM-48623 Patch plugin bundle react-dom on webapp extract (#21171) Automatic Merge --- app/plugin.go | 10 +++- app/plugin_api_test.go | 12 ++--- app/plugin_hooks_test.go | 4 +- model/config.go | 5 ++ plugin/environment.go | 71 ++++++++++++++++++++++++++-- services/telemetry/telemetry.go | 1 + services/telemetry/telemetry_test.go | 1 + web/web_test.go | 2 +- 8 files changed, 93 insertions(+), 13 deletions(-) diff --git a/app/plugin.go b/app/plugin.go index 93d47fd5b6..7ce6c47fe6 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -243,7 +243,15 @@ func (ch *Channels) initPlugins(c *request.Context, pluginDir, webappPluginDir s return New(ServerConnector(ch)).NewPluginAPI(c, manifest) } - env, err := plugin.NewEnvironment(newAPIFunc, NewDriverImpl(ch.srv), pluginDir, webappPluginDir, ch.srv.Log(), ch.srv.GetMetrics()) + env, err := plugin.NewEnvironment( + newAPIFunc, + NewDriverImpl(ch.srv), + pluginDir, + webappPluginDir, + *ch.cfgSvc.Config().ExperimentalSettings.PatchPluginsReactDOM, + ch.srv.Log(), + ch.srv.GetMetrics(), + ) if err != nil { mlog.Error("Failed to start up plugins", mlog.Err(err)) return diff --git a/app/plugin_api_test.go b/app/plugin_api_test.go index 9f864109bf..759613781b 100644 --- a/app/plugin_api_test.go +++ b/app/plugin_api_test.go @@ -92,7 +92,7 @@ func setupMultiPluginAPITest(t *testing.T, pluginCodes []string, pluginManifests return app.NewPluginAPI(c, manifest) } - env, err := plugin.NewEnvironment(newPluginAPI, NewDriverImpl(app.Srv()), pluginDir, webappPluginDir, app.Log(), nil) + env, err := plugin.NewEnvironment(newPluginAPI, NewDriverImpl(app.Srv()), pluginDir, webappPluginDir, false, app.Log(), nil) require.NoError(t, err) require.Equal(t, len(pluginCodes), len(pluginIDs)) @@ -849,7 +849,7 @@ func TestPluginAPIGetPlugins(t *testing.T) { defer os.RemoveAll(pluginDir) defer os.RemoveAll(webappPluginDir) - env, err := plugin.NewEnvironment(th.NewPluginAPI, NewDriverImpl(th.Server), pluginDir, webappPluginDir, th.App.Log(), nil) + env, err := plugin.NewEnvironment(th.NewPluginAPI, NewDriverImpl(th.Server), pluginDir, webappPluginDir, false, th.App.Log(), nil) require.NoError(t, err) pluginIDs := []string{"pluginid1", "pluginid2", "pluginid3"} @@ -937,7 +937,7 @@ func TestInstallPlugin(t *testing.T) { return app.NewPluginAPI(c, manifest) } - env, err := plugin.NewEnvironment(newPluginAPI, NewDriverImpl(app.Srv()), pluginDir, webappPluginDir, app.Log(), nil) + env, err := plugin.NewEnvironment(newPluginAPI, NewDriverImpl(app.Srv()), pluginDir, webappPluginDir, false, app.Log(), nil) require.NoError(t, err) app.ch.SetPluginsEnvironment(env) @@ -1632,7 +1632,7 @@ func TestAPIMetrics(t *testing.T) { defer os.RemoveAll(pluginDir) defer os.RemoveAll(webappPluginDir) - env, err := plugin.NewEnvironment(th.NewPluginAPI, NewDriverImpl(th.Server), pluginDir, webappPluginDir, th.App.Log(), metricsMock) + env, err := plugin.NewEnvironment(th.NewPluginAPI, NewDriverImpl(th.Server), pluginDir, webappPluginDir, false, th.App.Log(), metricsMock) require.NoError(t, err) th.App.ch.SetPluginsEnvironment(env) @@ -2079,7 +2079,7 @@ func TestRegisterCollectionAndTopic(t *testing.T) { return th.App.NewPluginAPI(th.Context, manifest) } - env, err := plugin.NewEnvironment(newPluginAPI, NewDriverImpl(th.App.Srv()), pluginDir, webappPluginDir, th.App.Log(), nil) + env, err := plugin.NewEnvironment(newPluginAPI, NewDriverImpl(th.App.Srv()), pluginDir, webappPluginDir, false, th.App.Log(), nil) require.NoError(t, err) th.App.ch.SetPluginsEnvironment(env) @@ -2179,7 +2179,7 @@ func TestPluginUploadsAPI(t *testing.T) { newPluginAPI := func(manifest *model.Manifest) plugin.API { return th.App.NewPluginAPI(th.Context, manifest) } - env, err := plugin.NewEnvironment(newPluginAPI, NewDriverImpl(th.App.Srv()), pluginDir, webappPluginDir, th.App.Log(), nil) + env, err := plugin.NewEnvironment(newPluginAPI, NewDriverImpl(th.App.Srv()), pluginDir, webappPluginDir, false, th.App.Log(), nil) require.NoError(t, err) th.App.ch.SetPluginsEnvironment(env) diff --git a/app/plugin_hooks_test.go b/app/plugin_hooks_test.go index 7cd81f8fae..161994a1ab 100644 --- a/app/plugin_hooks_test.go +++ b/app/plugin_hooks_test.go @@ -33,7 +33,7 @@ func SetAppEnvironmentWithPlugins(t *testing.T, pluginCode []string, app *App, a webappPluginDir, err := os.MkdirTemp("", "") require.NoError(t, err) - env, err := plugin.NewEnvironment(apiFunc, NewDriverImpl(app.Srv()), pluginDir, webappPluginDir, app.Log(), nil) + env, err := plugin.NewEnvironment(apiFunc, NewDriverImpl(app.Srv()), pluginDir, webappPluginDir, false, app.Log(), nil) require.NoError(t, err) app.ch.SetPluginsEnvironment(env) @@ -1030,7 +1030,7 @@ func TestHookMetrics(t *testing.T) { defer os.RemoveAll(pluginDir) defer os.RemoveAll(webappPluginDir) - env, err := plugin.NewEnvironment(th.NewPluginAPI, NewDriverImpl(th.Server), pluginDir, webappPluginDir, th.App.Log(), metricsMock) + env, err := plugin.NewEnvironment(th.NewPluginAPI, NewDriverImpl(th.Server), pluginDir, webappPluginDir, false, th.App.Log(), metricsMock) require.NoError(t, err) th.App.ch.SetPluginsEnvironment(env) diff --git a/model/config.go b/model/config.go index 058a7b7666..dd0ad5cfd3 100644 --- a/model/config.go +++ b/model/config.go @@ -968,6 +968,7 @@ type ExperimentalSettings struct { EnableSharedChannels *bool `access:"experimental_features"` EnableRemoteClusterService *bool `access:"experimental_features"` EnableAppBar *bool `access:"experimental_features"` + PatchPluginsReactDOM *bool `access:"experimental_features"` } func (s *ExperimentalSettings) SetDefaults() { @@ -1002,6 +1003,10 @@ func (s *ExperimentalSettings) SetDefaults() { if s.EnableAppBar == nil { s.EnableAppBar = NewBool(false) } + + if s.PatchPluginsReactDOM == nil { + s.PatchPluginsReactDOM = NewBool(false) + } } type AnalyticsSettings struct { diff --git a/plugin/environment.go b/plugin/environment.go index db07f051bd..4ccbbdf023 100644 --- a/plugin/environment.go +++ b/plugin/environment.go @@ -4,10 +4,12 @@ package plugin import ( + "bytes" "fmt" "hash/fnv" "os" "path/filepath" + "strings" "sync" "time" @@ -57,15 +59,20 @@ type Environment struct { dbDriver Driver pluginDir string webappPluginDir string + patchReactDOM bool prepackagedPlugins []*PrepackagedPlugin prepackagedPluginsLock sync.RWMutex } -func NewEnvironment(newAPIImpl apiImplCreatorFunc, +func NewEnvironment( + newAPIImpl apiImplCreatorFunc, dbDriver Driver, - pluginDir string, webappPluginDir string, + pluginDir string, + webappPluginDir string, + patchReactDOM bool, logger *mlog.Logger, - metrics einterfaces.MetricsInterface) (*Environment, error) { + metrics einterfaces.MetricsInterface, +) (*Environment, error) { return &Environment{ logger: logger, metrics: metrics, @@ -73,6 +80,7 @@ func NewEnvironment(newAPIImpl apiImplCreatorFunc, dbDriver: dbDriver, pluginDir: pluginDir, webappPluginDir: webappPluginDir, + patchReactDOM: patchReactDOM, }, nil } @@ -451,6 +459,17 @@ func (env *Environment) UnpackWebappBundle(id string) (*model.Manifest, error) { return nil, errors.Wrapf(err, "unable to read webapp bundle: %v", id) } + if env.patchReactDOM { + newContents, changed := patchReactDOM(sourceBundleFileContents) + if changed { + sourceBundleFileContents = newContents + err = os.WriteFile(sourceBundleFilepath, sourceBundleFileContents, 0644) + if err != nil { + return nil, errors.Wrapf(err, "unable to overwrite webapp bundle: %v", id) + } + } + } + hash := fnv.New64a() if _, err = hash.Write(sourceBundleFileContents); err != nil { return nil, errors.Wrapf(err, "unable to generate hash for webapp bundle: %v", id) @@ -467,6 +486,52 @@ func (env *Environment) UnpackWebappBundle(id string) (*model.Manifest, error) { return manifest, nil } +func patchReactDOM(initialBytes []byte) ([]byte, bool) { + if !bytes.Contains(initialBytes, []byte("react-dom.production.min.js")) { + return initialBytes, false + } + + initial := string(initialBytes) + nameIndex := strings.Index(initial, "react-dom.production.min.js") + + beginning := strings.LastIndex(initial[:nameIndex], "{") + var end int + + argDefBeginning := strings.LastIndex(initial[:beginning], "function") + 9 + argDefEnd := strings.LastIndex(initial[:beginning], ")") - 1 + argsNames := strings.Split(initial[argDefBeginning:argDefEnd], ",") + if len(argsNames) != 3 { + return initialBytes, false + } + + exportsArgName := strings.TrimSpace(argsNames[1]) + + numOpenBraces := 0 + for i, c := range initial[beginning:] { + if end != 0 { + break + } + switch c { + case '}': + numOpenBraces-- + + if numOpenBraces == 0 { + end = beginning + i + } + case '{': + numOpenBraces++ + } + } + + beforePatch := initial[:end] + afterPatch := initial[end:] + + patch := fmt.Sprintf("; Object.assign(%s, window.ReactDOM)", exportsArgName) + + result := fmt.Sprintf("%s%s%s", beforePatch, patch, afterPatch) + return []byte(result), true +} + // HooksForPlugin returns the hooks API for the plugin with the given id. // // Consider using RunMultiPluginHook instead. diff --git a/services/telemetry/telemetry.go b/services/telemetry/telemetry.go index ee8fea6f11..ce9eb79419 100644 --- a/services/telemetry/telemetry.go +++ b/services/telemetry/telemetry.go @@ -734,6 +734,7 @@ func (ts *TelemetryService) trackConfig() { "enable_shared_channels": *cfg.ExperimentalSettings.EnableSharedChannels, "enable_remote_cluster_service": *cfg.ExperimentalSettings.EnableRemoteClusterService && cfg.FeatureFlags.EnableRemoteClusterService, "enable_app_bar": *cfg.ExperimentalSettings.EnableAppBar, + "patch_plugins_react_dom": *cfg.ExperimentalSettings.PatchPluginsReactDOM, }) ts.SendTelemetry(TrackConfigAnalytics, map[string]any{ diff --git a/services/telemetry/telemetry_test.go b/services/telemetry/telemetry_test.go index 31ab7656ad..32a86602a3 100644 --- a/services/telemetry/telemetry_test.go +++ b/services/telemetry/telemetry_test.go @@ -165,6 +165,7 @@ func initializeMocks(cfg *model.Config, cloudLicense bool) (*mocks.ServerIface, func(m *model.Manifest) plugin.API { return pluginsAPIMock }, nil, pluginDir, webappPluginDir, + false, logger, nil) serverIfaceMock.On("GetPluginsEnvironment").Return(pluginEnv, nil) diff --git a/web/web_test.go b/web/web_test.go index 139753675d..ae8e322f91 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -280,7 +280,7 @@ func TestPublicFilesRequest(t *testing.T) { defer os.RemoveAll(pluginDir) defer os.RemoveAll(webappPluginDir) - env, err := plugin.NewEnvironment(th.NewPluginAPI, app.NewDriverImpl(th.Server), pluginDir, webappPluginDir, th.App.Log(), nil) + env, err := plugin.NewEnvironment(th.NewPluginAPI, app.NewDriverImpl(th.Server), pluginDir, webappPluginDir, false, th.App.Log(), nil) require.NoError(t, err) pluginID := "com.mattermost.sample" From 03a5b4a288ba60c52e604e5ef017cf5fc179db51 Mon Sep 17 00:00:00 2001 From: Shivashis Padhi Date: Wed, 7 Dec 2022 03:17:26 +0530 Subject: [PATCH 2/6] [MM-48727] Restore license checks for SSO in self-hosted deployments (#21781) * Revert license checks for sso integrations - Revert license check changes for open-id connect options - Add option to integrate openid-connect for all cloud offerings * Move enterprise SSO features back to enterprise repository --- cmd/mattermost/main.go | 3 - config/client.go | 31 +- model/license.go | 24 +- model/oauthproviders/google/google.go | 158 -------- model/oauthproviders/google/google_test.go | 61 --- model/oauthproviders/office365/office365.go | 116 ------ .../office365/office365_test.go | 43 --- model/oauthproviders/openid/openid.go | 243 ------------ model/oauthproviders/openid/openid_test.go | 352 ------------------ 9 files changed, 35 insertions(+), 996 deletions(-) delete mode 100644 model/oauthproviders/google/google.go delete mode 100644 model/oauthproviders/google/google_test.go delete mode 100644 model/oauthproviders/office365/office365.go delete mode 100644 model/oauthproviders/office365/office365_test.go delete mode 100644 model/oauthproviders/openid/openid.go delete mode 100644 model/oauthproviders/openid/openid_test.go diff --git a/cmd/mattermost/main.go b/cmd/mattermost/main.go index b5f73a31f9..441ed9a752 100644 --- a/cmd/mattermost/main.go +++ b/cmd/mattermost/main.go @@ -11,9 +11,6 @@ import ( _ "github.com/mattermost/mattermost-server/v6/app/slashcommands" // Plugins _ "github.com/mattermost/mattermost-server/v6/model/oauthproviders/gitlab" - _ "github.com/mattermost/mattermost-server/v6/model/oauthproviders/google" - _ "github.com/mattermost/mattermost-server/v6/model/oauthproviders/office365" - _ "github.com/mattermost/mattermost-server/v6/model/oauthproviders/openid" // Enterprise Imports _ "github.com/mattermost/mattermost-server/v6/imports" diff --git a/config/client.go b/config/client.go index 7d05dda917..b8160993fd 100644 --- a/config/client.go +++ b/config/client.go @@ -303,11 +303,11 @@ func GenerateLimitedClientConfig(c *model.Config, telemetryID string, license *m props["SamlLoginButtonColor"] = "" props["SamlLoginButtonBorderColor"] = "" props["SamlLoginButtonTextColor"] = "" - props["EnableSignUpWithOpenId"] = strconv.FormatBool(*c.OpenIdSettings.Enable) - props["OpenIdButtonColor"] = *c.OpenIdSettings.ButtonColor - props["OpenIdButtonText"] = *c.OpenIdSettings.ButtonText - props["EnableSignUpWithGoogle"] = strconv.FormatBool(*c.GoogleSettings.Enable) - props["EnableSignUpWithOffice365"] = strconv.FormatBool(*c.Office365Settings.Enable) + props["EnableSignUpWithGoogle"] = "false" + props["EnableSignUpWithOffice365"] = "false" + props["EnableSignUpWithOpenId"] = "false" + props["OpenIdButtonText"] = "" + props["OpenIdButtonColor"] = "" props["CWSURL"] = "" props["EnableCustomBrand"] = strconv.FormatBool(*c.TeamSettings.EnableCustomBrand) props["CustomBrandText"] = *c.TeamSettings.CustomBrandText @@ -342,6 +342,27 @@ func GenerateLimitedClientConfig(c *model.Config, telemetryID string, license *m if *license.Features.MFA { props["EnforceMultifactorAuthentication"] = strconv.FormatBool(*c.ServiceSettings.EnforceMultifactorAuthentication) } + + if license.IsCloud() { + // MM-48727: enable SSO options for free cloud - not in self hosted + *license.Features.GoogleOAuth = true + *license.Features.Office365OAuth = true + *license.Features.OpenId = true + } + + if *license.Features.GoogleOAuth { + props["EnableSignUpWithGoogle"] = strconv.FormatBool(*c.GoogleSettings.Enable) + } + + if *license.Features.Office365OAuth { + props["EnableSignUpWithOffice365"] = strconv.FormatBool(*c.Office365Settings.Enable) + } + + if *license.Features.OpenId { + props["EnableSignUpWithOpenId"] = strconv.FormatBool(*c.OpenIdSettings.Enable) + props["OpenIdButtonColor"] = *c.OpenIdSettings.ButtonColor + props["OpenIdButtonText"] = *c.OpenIdSettings.ButtonText + } } for key, value := range c.FeatureFlags.ToMap() { diff --git a/model/license.go b/model/license.go index d04a88acef..94f0b81da4 100644 --- a/model/license.go +++ b/model/license.go @@ -78,18 +78,12 @@ type TrialLicenseRequest struct { } type Features struct { - Users *int `json:"users"` - LDAP *bool `json:"ldap"` - LDAPGroups *bool `json:"ldap_groups"` - MFA *bool `json:"mfa"` - - // Deprecated: This feature will be removed from the license because it's available without a license. - GoogleOAuth *bool `json:"google_oauth"` - - // Deprecated: This feature will be removed from the license because it's available without a license. - Office365OAuth *bool `json:"office365_oauth"` - - // Deprecated: This feature will be removed from the license because it's available without a license. + Users *int `json:"users"` + LDAP *bool `json:"ldap"` + LDAPGroups *bool `json:"ldap_groups"` + MFA *bool `json:"mfa"` + GoogleOAuth *bool `json:"google_oauth"` + Office365OAuth *bool `json:"office365_oauth"` OpenId *bool `json:"openid"` Compliance *bool `json:"compliance"` Cluster *bool `json:"cluster"` @@ -171,15 +165,15 @@ func (f *Features) SetDefaults() { } if f.GoogleOAuth == nil { - f.GoogleOAuth = NewBool(true) + f.GoogleOAuth = NewBool(*f.FutureFeatures) } if f.Office365OAuth == nil { - f.Office365OAuth = NewBool(true) + f.Office365OAuth = NewBool(*f.FutureFeatures) } if f.OpenId == nil { - f.OpenId = NewBool(true) + f.OpenId = NewBool(*f.FutureFeatures) } if f.Compliance == nil { diff --git a/model/oauthproviders/google/google.go b/model/oauthproviders/google/google.go deleted file mode 100644 index 19c4cf3353..0000000000 --- a/model/oauthproviders/google/google.go +++ /dev/null @@ -1,158 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -package oauthgoogle - -import ( - "encoding/json" - "errors" - "io" - "strings" - - "github.com/mattermost/mattermost-server/v6/einterfaces" - "github.com/mattermost/mattermost-server/v6/model" -) - -type GoogleProvider struct { -} - -type SourceElement struct { - Type string `json:"type"` - ID string `json:"id"` - Etag string `json:"etag"` - ProfileMetadata ProfileMetadata `json:"profileMetadata"` -} - -type ProfileMetadata struct { - ObjectType string `json:"objectType"` - UserTypes []string `json:"userTypes"` -} - -type GoogleUserRootMetadata struct { - Sources []SourceElement `json:"sources"` -} - -type GoogleUserMetadata struct { - Source map[string]string `json:"source"` -} - -type GoogleUserNameNode struct { - Metadata GoogleUserMetadata `json:"metadata"` - GivenName string `json:"givenName"` - FamilyName string `json:"familyName"` -} - -type GoogleGenericInfoNode struct { - Metadata GoogleUserMetadata `json:"metadata"` - Value string `json:"value"` -} - -type GoogleUser struct { - Metadata GoogleUserRootMetadata `json:"metadata"` - Nicknames []GoogleGenericInfoNode `json:"nicknames"` - Emails []GoogleGenericInfoNode `json:"emailAddresses"` - Names []GoogleUserNameNode `json:"names"` -} - -func init() { - provider := &GoogleProvider{} - einterfaces.RegisterOAuthProvider(model.ServiceGoogle, provider) -} - -func userFromGoogleUser(gu *GoogleUser) *model.User { - user := &model.User{} - - for _, e := range gu.Emails { - if e.Metadata.Source["type"] == "ACCOUNT" || e.Metadata.Source["type"] == "DOMAIN_PROFILE" { - user.Email = e.Value - user.Username = model.CleanUsername(strings.Split(user.Email, "@")[0]) - break - } - } - - for _, e := range gu.Names { - if e.Metadata.Source["type"] == "PROFILE" || e.Metadata.Source["type"] == "DOMAIN_PROFILE" { - user.FirstName = e.GivenName - user.LastName = e.FamilyName - break - } - } - - if len(gu.Nicknames) > 0 { - user.Nickname = gu.Nicknames[0].Value - } - - user.AuthData = new(string) - *user.AuthData = gu.getAuthData() - user.AuthService = model.ServiceGoogle - - return user -} - -func googleUserFromJSON(data io.Reader) (*GoogleUser, error) { - decoder := json.NewDecoder(data) - var gu GoogleUser - err := decoder.Decode(&gu) - if err != nil { - return nil, err - } - - return &gu, nil -} - -func (gu *GoogleUser) IsValid() error { - if len(gu.Metadata.Sources) == 0 { - return errors.New("invalid metadata sources") - } - - if len(gu.Emails) == 0 { - return errors.New("invalid emails") - } - - return nil -} - -func (gu *GoogleUser) getAuthData() string { - if len(gu.Metadata.Sources) > 0 { - return gu.Metadata.Sources[0].ID - } - - return "" -} - -func (m *GoogleProvider) GetIdentifier() string { - return model.ServiceGoogle -} - -func (m *GoogleProvider) GetUserFromJSON(data io.Reader, tokenUser *model.User) (*model.User, error) { - gu, err := googleUserFromJSON(data) - if err != nil { - return nil, err - } - return userFromGoogleUser(gu), nil -} - -func (m *GoogleProvider) GetAuthDataFromJSON(data io.Reader) (string, error) { - gu, err := googleUserFromJSON(data) - if err != nil { - return "", err - } - - if err = gu.IsValid(); err != nil { - return "", err - } - - return gu.getAuthData(), nil -} - -func (m *GoogleProvider) GetSSOSettings(config *model.Config, service string) (*model.SSOSettings, error) { - return &config.GoogleSettings, nil -} - -func (m *GoogleProvider) GetUserFromIdToken(idToken string) (*model.User, error) { - return nil, nil -} - -func (m *GoogleProvider) IsSameUser(dbUser, oauthUser *model.User) bool { - return dbUser.AuthData == oauthUser.AuthData -} diff --git a/model/oauthproviders/google/google_test.go b/model/oauthproviders/google/google_test.go deleted file mode 100644 index 1f30efb8eb..0000000000 --- a/model/oauthproviders/google/google_test.go +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -package oauthgoogle - -import ( - "bytes" - "encoding/json" - "strings" - "testing" - - "github.com/stretchr/testify/require" -) - -func TestGoogleUserFromJSON(t *testing.T) { - gu := GoogleUser{ - Metadata: GoogleUserRootMetadata{ - Sources: []SourceElement{ - { - Etag: "tag", - }, - }, - }, - Emails: []GoogleGenericInfoNode{ - { - Value: "ali@test.com", - }, - }, - Names: []GoogleUserNameNode{ - { - GivenName: "ali", - }, - }, - Nicknames: []GoogleGenericInfoNode{ - { - Value: "ila", - }, - }, - } - - provider := &GoogleProvider{} - - t.Run("valid google user", func(t *testing.T) { - b, err := json.Marshal(gu) - require.NoError(t, err) - - _, err = provider.GetUserFromJSON(bytes.NewReader(b), nil) - require.NoError(t, err) - - _, err = provider.GetAuthDataFromJSON(bytes.NewReader(b)) - require.NoError(t, err) - }) - - t.Run("empty body should fail without panic", func(t *testing.T) { - _, err := provider.GetUserFromJSON(strings.NewReader("{}"), nil) - require.NoError(t, err) - - _, err = provider.GetAuthDataFromJSON(strings.NewReader("{}")) - require.Error(t, err) - }) -} diff --git a/model/oauthproviders/office365/office365.go b/model/oauthproviders/office365/office365.go deleted file mode 100644 index 7f23d9e51c..0000000000 --- a/model/oauthproviders/office365/office365.go +++ /dev/null @@ -1,116 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -package oauthoffice365 - -import ( - "encoding/json" - "errors" - "io" - "strings" - - "github.com/mattermost/mattermost-server/v6/einterfaces" - "github.com/mattermost/mattermost-server/v6/model" -) - -type Office365Provider struct { -} - -type Office365User struct { - Id string `json:"id"` - FirstName string `json:"givenName"` - LastName string `json:"surname"` - Mail string `json:"mail"` - UserPrincipalName string `json:"userPrincipalName"` -} - -func init() { - provider := &Office365Provider{} - einterfaces.RegisterOAuthProvider(model.ServiceOffice365, provider) -} - -func userFromOffice365User(of *Office365User) *model.User { - user := &model.User{} - user.FirstName = of.FirstName - user.LastName = of.LastName - - if of.Mail != "" { - user.Email = of.Mail - } else if strings.Contains(of.UserPrincipalName, "@") { - user.Email = of.UserPrincipalName - } - - if user.Email != "" { - user.Username = model.CleanUsername(strings.Split(user.Email, "@")[0]) - } - - user.AuthData = new(string) - *user.AuthData = of.Id - user.AuthService = model.ServiceOffice365 - - return user -} - -func office365UserFromJSON(data io.Reader) (*Office365User, error) { - decoder := json.NewDecoder(data) - var of Office365User - err := decoder.Decode(&of) - if err != nil { - return nil, err - } - - return &of, nil -} - -func (of *Office365User) IsValid() error { - if of.Id == "" { - return errors.New("invalid user id") - } - - if of.Mail == "" && !strings.Contains(of.UserPrincipalName, "@") { - return errors.New("invalid email") - } - - return nil -} - -func (of *Office365User) getAuthData() string { - return of.Id -} - -func (m *Office365Provider) GetIdentifier() string { - return model.ServiceOffice365 -} - -func (m *Office365Provider) GetUserFromJSON(data io.Reader, tokenUser *model.User) (*model.User, error) { - of, err := office365UserFromJSON(data) - if err != nil { - return nil, err - } - return userFromOffice365User(of), nil -} - -func (m *Office365Provider) GetAuthDataFromJSON(data io.Reader) (string, error) { - of, err := office365UserFromJSON(data) - if err != nil { - return "", err - } - - if err = of.IsValid(); err != nil { - return "", err - } - - return of.getAuthData(), nil -} - -func (m *Office365Provider) GetSSOSettings(config *model.Config, service string) (*model.SSOSettings, error) { - return config.Office365Settings.SSOSettings(), nil -} - -func (m *Office365Provider) GetUserFromIdToken(idToken string) (*model.User, error) { - return nil, nil -} - -func (m *Office365Provider) IsSameUser(dbUser, oauthUser *model.User) bool { - return dbUser.AuthData == oauthUser.AuthData -} diff --git a/model/oauthproviders/office365/office365_test.go b/model/oauthproviders/office365/office365_test.go deleted file mode 100644 index 85496dd7ab..0000000000 --- a/model/oauthproviders/office365/office365_test.go +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -package oauthoffice365 - -import ( - "bytes" - "encoding/json" - "strings" - "testing" - - "github.com/stretchr/testify/require" -) - -func TestOffice365UserFromJSON(t *testing.T) { - ou := Office365User{ - FirstName: "ali", - Id: "12345", - LastName: "maya", - Mail: "ali@test.com", - } - - provider := &Office365Provider{} - - t.Run("valid office365 user", func(t *testing.T) { - b, err := json.Marshal(ou) - require.NoError(t, err) - - _, err = provider.GetUserFromJSON(bytes.NewReader(b), nil) - require.NoError(t, err) - - _, err = provider.GetAuthDataFromJSON(bytes.NewReader(b)) - require.NoError(t, err) - }) - - t.Run("empty body should fail without panic", func(t *testing.T) { - _, err := provider.GetUserFromJSON(strings.NewReader("{}"), nil) - require.NoError(t, err) - - _, err = provider.GetAuthDataFromJSON(strings.NewReader("{}")) - require.Error(t, err) - }) -} diff --git a/model/oauthproviders/openid/openid.go b/model/oauthproviders/openid/openid.go deleted file mode 100644 index e48be5d913..0000000000 --- a/model/oauthproviders/openid/openid.go +++ /dev/null @@ -1,243 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -package oauthopenid - -import ( - "encoding/base64" - "encoding/json" - "errors" - "io" - "net/http" - "strconv" - "strings" - "time" - - "github.com/mattermost/mattermost-server/v6/einterfaces" - "github.com/mattermost/mattermost-server/v6/model" -) - -type CacheData struct { - Service string - Expires int64 - Settings model.SSOSettings -} - -type OpenIdMetadata struct { - Issuer string `json:"issuer"` - AuthorizationEndpoint string `json:"authorization_endpoint"` - TokenEndpoint string `json:"token_endpoint"` - UserEndpoint string `json:"userinfo_endpoint"` - JwksURI string `json:"jwks_uri"` - Algorithms []string `json:"id_token_signing_alg_values_supported"` -} - -type OpenIdProvider struct { - CacheData *CacheData -} - -type OpenIdUser struct { - Id string `json:"sub"` - Oid string `json:"oid"` //Office 365 only - FirstName string `json:"given_name"` - LastName string `json:"family_name"` - Name string `json:"name"` - Nickname string `json:"nickname"` - Email string `json:"email"` -} - -func init() { - provider := &OpenIdProvider{} - einterfaces.RegisterOAuthProvider(model.ServiceOpenid, provider) -} - -func (o *OpenIdProvider) userFromOpenIdUser(u *OpenIdUser) *model.User { - user := &model.User{} - - user.Email = u.Email - user.Username = model.CleanUsername(strings.Split(user.Email, "@")[0]) - if o.CacheData.Service == model.ServiceGitlab && u.Nickname != "" { - user.Username = u.Nickname - } - - user.FirstName = u.FirstName - user.LastName = u.LastName - user.Nickname = u.Nickname - - user.AuthData = new(string) - *user.AuthData = o.getAuthData(u) - - return user -} - -func (o *OpenIdProvider) getAuthData(u *OpenIdUser) string { - if o.CacheData.Service == model.ServiceOffice365 { - if u.Oid != "" { - return u.Oid - } - } - return u.Id -} - -func openIDUserFromJSON(data io.Reader) (*OpenIdUser, error) { - decoder := json.NewDecoder(data) - var u OpenIdUser - err := decoder.Decode(&u) - if err != nil { - return nil, err - } - return &u, nil -} - -func (u *OpenIdUser) IsValid() error { - if u.Id == "" { - return errors.New("invalid id") - } - - if u.Email == "" { - return errors.New("invalid emails") - } - return nil -} - -func (u *OpenIdUser) GetIdentifier() string { - return model.ServiceOpenid -} - -func (o *OpenIdProvider) GetUserFromJSON(data io.Reader, tokenUser *model.User) (*model.User, error) { - oid, err := openIDUserFromJSON(data) - if err != nil { - return nil, err - } - jsonUser := o.userFromOpenIdUser(oid) - - if tokenUser != nil { - jsonUser = o.combineUsers(jsonUser, tokenUser) - } - return jsonUser, nil -} - -func (o *OpenIdProvider) combineUsers(jsonUser *model.User, tokenUser *model.User) *model.User { - if o.CacheData.Service == model.ServiceOffice365 { - jsonUser.AuthData = tokenUser.AuthData - } - return jsonUser -} - -func (o *OpenIdProvider) GetAuthDataFromJSON(data io.Reader) (string, error) { - u, err := openIDUserFromJSON(data) - if err != nil { - return "", err - } - - err = u.IsValid() - if err != nil { - return "", err - } - return o.getAuthData(u), nil -} - -// GetSSOSettings returns SSO Settings from Cache or Discovery Document -func (o *OpenIdProvider) GetSSOSettings(config *model.Config, service string) (*model.SSOSettings, error) { - settings := config.OpenIdSettings - if service == model.ServiceOffice365 { - settings = *config.Office365Settings.SSOSettings() - } else if service == model.ServiceGoogle { - settings = config.GoogleSettings - } else if service == model.ServiceGitlab { - settings = config.GitLabSettings - } - - if o.CacheData != nil && !settingsChanged(*o.CacheData, service, settings) && o.CacheData.Expires > time.Now().Unix() { - return &o.CacheData.Settings, nil - } - - var age int64 = 0 - if *settings.DiscoveryEndpoint != "" { - response, err := http.Get(*settings.DiscoveryEndpoint) - if err != nil { - return nil, err - } - defer response.Body.Close() - - for _, v := range strings.Split(response.Header.Get("Cache-Control"), ",") { - if strings.Contains(v, "max-age") { - ageValue := strings.Split(v, "=")[1] - age, _ = strconv.ParseInt(ageValue, 10, 64) - } - } - responseData, err := io.ReadAll(response.Body) - if err != nil { - return nil, err - } - - var openIDResponse OpenIdMetadata - err = json.Unmarshal(responseData, &openIDResponse) - if err != nil { - return nil, err - } - - settings.AuthEndpoint = &openIDResponse.AuthorizationEndpoint - settings.TokenEndpoint = &openIDResponse.TokenEndpoint - settings.UserAPIEndpoint = &openIDResponse.UserEndpoint - } - expires := time.Now().Unix() + age - - o.CacheData = &CacheData{ - Service: service, - Expires: expires, - Settings: settings, - } - return &settings, nil -} - -func settingsChanged(cacheData CacheData, service string, configSettings model.SSOSettings) bool { - if cacheData.Service == service && - cacheData.Settings.DiscoveryEndpoint == configSettings.DiscoveryEndpoint && - cacheData.Settings.Secret == configSettings.Secret && - cacheData.Settings.Id == configSettings.Id { - return false - } - return true -} - -func (o *OpenIdProvider) GetUserFromIdToken(idToken string) (*model.User, error) { - parts := strings.Split(idToken, ".") - if len(parts) != 3 { - return nil, errors.New("invalid Id Token") - } - - b, err := base64.RawURLEncoding.DecodeString(parts[1]) - if err != nil { - return nil, err - } - - claims := &OpenIdUser{} - json.Unmarshal(b, &claims) - - return o.userFromOpenIdUser(claims), nil -} - -func (o *OpenIdProvider) IsSameUser(dbUser, oauthUser *model.User) bool { - // Office365 OAuth would store Ids without dashes. (ie. 0e8fddd450d344999a93a390ee8cb83d) - // Office365 OpenId will return as a formatted GUID (ie. '0e8fddd4-50d3-4499-9a93-a390ee8cb83d') - // If this is a UUID that starts with all zero. (ie. 00000000-0000-0000-be95-fe607df5dbeb) - // For backwards compatibility we store the auth data from OAuth as be95fe607df5dbeb - if dbUser.AuthData == nil || oauthUser.AuthData == nil { - return false - } - dbID := *dbUser.AuthData - oauthID := *oauthUser.AuthData - if dbID == "" || oauthID == "" { - return false - } - parts := strings.Split(oauthID, "-") - for _, part := range parts { - if strings.Count(part, "0") != len(part) { - if !strings.Contains(dbID, part) { - return false - } - } - } - return true -} diff --git a/model/oauthproviders/openid/openid_test.go b/model/oauthproviders/openid/openid_test.go deleted file mode 100644 index 7e157c6a88..0000000000 --- a/model/oauthproviders/openid/openid_test.go +++ /dev/null @@ -1,352 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -package oauthopenid - -import ( - "bytes" - "encoding/json" - "fmt" - "net/http" - "net/http/httptest" - "strings" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/mattermost/mattermost-server/v6/model" -) - -func TestGetAuthData(t *testing.T) { - ou := OpenIdUser{ - Id: "12345", - FirstName: "firstname", - LastName: "lastname", - Nickname: "nickname", - Email: "name@test.com", - Oid: "0e8fddd4-50d3-4499-9a93-a390ee8cb83d", - } - - provider := &OpenIdProvider{ - CacheData: &CacheData{ - Service: model.ServiceGitlab, - }, - } - - t.Run("validate return id", func(t *testing.T) { - authData := provider.getAuthData(&ou) - assert.Equal(t, ou.Id, authData) - }) - - provider.CacheData.Service = model.ServiceOffice365 - - fmt.Println(provider.CacheData.Service) - t.Run("validate Oid return", func(t *testing.T) { - authData := provider.getAuthData(&ou) - assert.Equal(t, ou.Oid, authData) - }) -} -func TestOpenIdUserFromJSON(t *testing.T) { - ou := OpenIdUser{ - Id: "12345", - FirstName: "firstname", - LastName: "lastname", - Nickname: "nickname", - Email: "name@test.com", - } - - provider := &OpenIdProvider{ - CacheData: &CacheData{ - Service: model.ServiceOpenid, - }, - } - - t.Run("valid OpenId user", func(t *testing.T) { - b, err := json.Marshal(ou) - require.NoError(t, err) - - _, err = provider.GetUserFromJSON(bytes.NewReader(b), nil) - require.NoError(t, err) - - _, err = provider.GetAuthDataFromJSON(bytes.NewReader(b)) - require.NoError(t, err) - }) - - t.Run("empty body should fail without panic", func(t *testing.T) { - _, err := provider.GetUserFromJSON(strings.NewReader("{}"), nil) - require.NoError(t, err) - - _, err = provider.GetAuthDataFromJSON(strings.NewReader("{}")) - require.Error(t, err) - }) - - t.Run("test getUserFromIdToken", func(t *testing.T) { - header := "dummyHeader" - payload := "eyJpc3MiOiJodHRwczovL2FjY291bnRzLmdvb2dsZS5jb20iLCJhenAiOiIxMDIyOTIwNzU1ODQ2LWtyM2JrMjBxdDRhMTlkODhqMWt1cjNqcnM2MmI2ZXFjLmFwcHMuZ29vZ2xldXNlcmNvbnRlbnQuY29tIiwiYXVkIjoiMTAyMjkyMDc1NTg0Ni1rcjNiazIwcXQ0YTE5ZDg4ajFrdXIzanJzNjJiNmVxYy5hcHBzLmdvb2dsZXVzZXJjb250ZW50LmNvbSIsInN1YiI6IjExMDIxNjMwMDI2MzA5MTY3MzQ2MSIsImhkIjoibWF0dGVybW9zdC5jb20iLCJlbWFpbCI6InNjb3R0LmJpc2hlbEBtYXR0ZXJtb3N0LmNvbSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlLCJhdF9oYXNoIjoiWTVscFFoQlR0UkxHUGZqZ1BLSUhzUSIsIm5hbWUiOiJTY290dCBCaXNoZWwiLCJwaWN0dXJlIjoiaHR0cHM6Ly9saDMuZ29vZ2xldXNlcmNvbnRlbnQuY29tL2EtL0FPaDE0R2dMR1Nfa19KV2dacmc1Y1BGLU9JNV9oUkhaREFvUUNoUFUyVE1VPXM5Ni1jIiwiZ2l2ZW5fbmFtZSI6IlNjb3R0IiwiZmFtaWx5X25hbWUiOiJCaXNoZWwiLCJsb2NhbGUiOiJlbiIsImlhdCI6MTYwODI0OTg5MSwiZXhwIjoxNjA4MjUzNDkxfQ" - signature := "dummysignature" - - testToken := header - _, err := provider.GetUserFromIdToken(testToken) - require.Error(t, err) - - testToken = header + "." + payload - _, err = provider.GetUserFromIdToken(testToken) - require.Error(t, err) - - t.Run("non ascii string encoded in the payload", func(t *testing.T) { - cases := []struct { - payload string - expectedName string - }{ - { - payload: "eyJpc3MiOiJodHRwczovL2FjY291bnRzLmdvb2dsZS5jb20iLCJhenAiOiIxMDIyOTIwNzU1ODQ2LWtyM2JrMjBxdDRhMTlkODhqMWt1cjNqcnM2MmI2ZXFjLmFwcHMuZ29vZ2xldXNlcmNvbnRlbnQuY29tIiwiYXVkIjoiMTAyMjkyMDc1NTg0Ni1rcjNiazIwcXQ0YTE5ZDg4ajFrdXIzanJzNjJiNmVxYy5hcHBzLmdvb2dsZXVzZXJjb250ZW50LmNvbSIsInN1YiI6IjExMDIxNjMwMDI2MzA5MTY3MzQ2MSIsImhkIjoibWF0dGVybW9zdC5jb20iLCJlbWFpbCI6InNjb3R0LmJpc2hlbEBtYXR0ZXJtb3N0LmNvbSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlLCJhdF9oYXNoIjoiWTVscFFoQlR0UkxHUGZqZ1BLSUhzUSIsIm5hbWUiOiJTY290dCBCaXNoZWwiLCJwaWN0dXJlIjoiaHR0cHM6Ly9saDMuZ29vZ2xldXNlcmNvbnRlbnQuY29tL2EtL0FPaDE0R2dMR1Nfa19KV2dacmc1Y1BGLU9JNV9oUkhaREFvUUNoUFUyVE1VPXM5Ni1jIiwiZ2l2ZW5fbmFtZSI6InRlc3TFiMWhxb4iLCJmYW1pbHlfbmFtZSI6IkJpc2hlbCIsImxvY2FsZSI6ImVuIiwiaWF0IjoxNjA4MjQ5ODkxLCJleHAiOjE2MDgyNTM0OTF9", - expectedName: "testňšž", - }, - { - payload: "eyJpc3MiOiJodHRwczovL2FjY291bnRzLmdvb2dsZS5jb20iLCJhenAiOiIxMDIyOTIwNzU1ODQ2LWtyM2JrMjBxdDRhMTlkODhqMWt1cjNqcnM2MmI2ZXFjLmFwcHMuZ29vZ2xldXNlcmNvbnRlbnQuY29tIiwiYXVkIjoiMTAyMjkyMDc1NTg0Ni1rcjNiazIwcXQ0YTE5ZDg4ajFrdXIzanJzNjJiNmVxYy5hcHBzLmdvb2dsZXVzZXJjb250ZW50LmNvbSIsInN1YiI6IjExMDIxNjMwMDI2MzA5MTY3MzQ2MSIsImhkIjoibWF0dGVybW9zdC5jb20iLCJlbWFpbCI6InNjb3R0LmJpc2hlbEBtYXR0ZXJtb3N0LmNvbSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlLCJhdF9oYXNoIjoiWTVscFFoQlR0UkxHUGZqZ1BLSUhzUSIsIm5hbWUiOiJTY290dCBCaXNoZWwiLCJwaWN0dXJlIjoiaHR0cHM6Ly9saDMuZ29vZ2xldXNlcmNvbnRlbnQuY29tL2EtL0FPaDE0R2dMR1Nfa19KV2dacmc1Y1BGLU9JNV9oUkhaREFvUUNoUFUyVE1VPXM5Ni1jIiwiZ2l2ZW5fbmFtZSI6IlNjb3R0IiwiZmFtaWx5X25hbWUiOiJCaXNoZWwiLCJsb2NhbGUiOiJlbiIsImlhdCI6MTYwODI0OTg5MSwiZXhwIjoxNjA4MjUzNDkxfQ", - expectedName: "Scott", - }, - { - payload: "eyJpc3MiOiJodHRwczovL2FjY291bnRzLmdvb2dsZS5jb20iLCJhenAiOiIxMDIyOTIwNzU1ODQ2LWtyM2JrMjBxdDRhMTlkODhqMWt1cjNqcnM2MmI2ZXFjLmFwcHMuZ29vZ2xldXNlcmNvbnRlbnQuY29tIiwiYXVkIjoiMTAyMjkyMDc1NTg0Ni1rcjNiazIwcXQ0YTE5ZDg4ajFrdXIzanJzNjJiNmVxYy5hcHBzLmdvb2dsZXVzZXJjb250ZW50LmNvbSIsInN1YiI6IjExMDIxNjMwMDI2MzA5MTY3MzQ2MSIsImhkIjoibWF0dGVybW9zdC5jb20iLCJlbWFpbCI6InNjb3R0LmJpc2hlbEBtYXR0ZXJtb3N0LmNvbSIsImVtYWlsX3ZlcmlmaWVkIjp0cnVlLCJhdF9oYXNoIjoiWTVscFFoQlR0UkxHUGZqZ1BLSUhzUSIsIm5hbWUiOiJTY290dCBCaXNoZWwiLCJwaWN0dXJlIjoiaHR0cHM6Ly9saDMuZ29vZ2xldXNlcmNvbnRlbnQuY29tL2EtL0FPaDE0R2dMR1Nfa19KV2dacmc1Y1BGLU9JNV9oUkhaREFvUUNoUFUyVE1VPXM5Ni1jIiwiZ2l2ZW5fbmFtZSI6InRlc3TEjcSNxI0iLCJmYW1pbHlfbmFtZSI6IkJpc2hlbCIsImxvY2FsZSI6ImVuIiwiaWF0IjoxNjA4MjQ5ODkxLCJleHAiOjE2MDgyNTM0OTF9", - expectedName: "testččč", - }, - } - for _, c := range cases { - testToken = header + "." + c.payload + "." + signature - user, err := provider.GetUserFromIdToken(testToken) - require.NoError(t, err) - require.NotNil(t, user) - require.Equal(t, c.expectedName, user.FirstName) - } - }) - - }) -} - -func TestGetSSOSettings(t *testing.T) { - provider := &OpenIdProvider{ - CacheData: &CacheData{ - Service: model.ServiceOpenid, - }, - } - validJSON := `{ - "issuer": "issuer", - "authorization_endpoint": "authorization_endpoint", - "token_endpoint": "token_endpoint", - "userinfo_endpoint": "userinfo_endpoint", - "jwks_uri": "jwks_uri", - "id_token_signing_alg_values_supported": ["RS256"] - }` - var validFunctionCalled int - validServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Add("Cache-Control", "max-age=3600") - fmt.Fprintln(w, validJSON) - validFunctionCalled++ - })) - defer validServer.Close() - - validConfig := model.Config{ - OpenIdSettings: model.SSOSettings{ - Enable: model.NewBool(true), - Secret: model.NewString("secret string"), - Id: model.NewString("id"), - Scope: model.NewString("profile openid email"), - AuthEndpoint: model.NewString(""), - TokenEndpoint: model.NewString(""), - UserAPIEndpoint: model.NewString(""), - DiscoveryEndpoint: model.NewString(validServer.URL), - }, - } - - t.Run("Error", func(t *testing.T) { - errorFunctionCalled := 0 - errorServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - errorFunctionCalled++ - w.Header().Add("Cache-Control", "max-age=3600") - http.Error(w, "Not found", 404) - })) - - errCfg := validConfig - errCfg.OpenIdSettings.DiscoveryEndpoint = model.NewString(errorServer.URL) - _, err := provider.GetSSOSettings(&errCfg, model.ServiceOpenid) - assert.Error(t, err) - assert.Equal(t, 1, errorFunctionCalled) - }) - - t.Run("UseCache", func(t *testing.T) { - validFunctionCalled = 0 - - settings, _ := provider.GetSSOSettings(&validConfig, model.ServiceOpenid) - assert.Equal(t, "authorization_endpoint", *settings.AuthEndpoint) - assert.Equal(t, "token_endpoint", *settings.TokenEndpoint) - assert.Equal(t, "userinfo_endpoint", *settings.UserAPIEndpoint) - assert.Equal(t, 1, validFunctionCalled) - // Should set cache - assert.Equal(t, provider.CacheData.Settings, *settings) - assert.True(t, provider.CacheData.Expires > 0) - currentCacheExpires := provider.CacheData.Expires - - // Call again should come from cache - settings, _ = provider.GetSSOSettings(&validConfig, model.ServiceOpenid) - assert.Equal(t, provider.CacheData.Settings, *settings) - assert.Equal(t, currentCacheExpires, provider.CacheData.Expires) - // should still be 1 - assert.Equal(t, 1, validFunctionCalled) - }) - - t.Run("CacheExpired", func(t *testing.T) { - // reset to original cache settings - settings, _ := provider.GetSSOSettings(&validConfig, model.ServiceOpenid) - // Should set cache - assert.Equal(t, provider.CacheData.Settings, *settings) - - // set cache to expired - provider.CacheData.Expires = time.Now().Add(time.Duration(-1) * time.Minute).Unix() - - // same config, should call endpoint - validFunctionCalled = 0 - provider.GetSSOSettings(&validConfig, model.ServiceOpenid) - assert.Equal(t, 1, validFunctionCalled) - assert.True(t, provider.CacheData.Expires > time.Now().Unix()) - }) - - t.Run("NoCache", func(t *testing.T) { - noCacheFunctionCalled := 0 - noCacheServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintln(w, validJSON) - noCacheFunctionCalled++ - })) - defer noCacheServer.Close() - - newCfg := validConfig - newCfg.OpenIdSettings.DiscoveryEndpoint = model.NewString(noCacheServer.URL) - - settings, err := provider.GetSSOSettings(&newCfg, model.ServiceOpenid) - require.NoError(t, err) - assert.Equal(t, "authorization_endpoint", *settings.AuthEndpoint) - assert.Equal(t, "token_endpoint", *settings.TokenEndpoint) - assert.Equal(t, "userinfo_endpoint", *settings.UserAPIEndpoint) - assert.Equal(t, 1, noCacheFunctionCalled) - // Should set cache - assert.Equal(t, provider.CacheData.Settings, *settings) - // Cache Expires, set, less than, equal now. - assert.True(t, provider.CacheData.Expires <= time.Now().Unix()) - - // Call again, should call server again - _, err = provider.GetSSOSettings(&newCfg, model.ServiceOpenid) - require.NoError(t, err) - assert.Equal(t, 2, noCacheFunctionCalled) - }) - - t.Run("ChangeService", func(t *testing.T) { - // reset to original cache settings - settings, _ := provider.GetSSOSettings(&validConfig, model.ServiceOpenid) - // Should set cache - assert.Equal(t, provider.CacheData.Settings, *settings) - assert.True(t, provider.CacheData.Expires > time.Now().Unix()) - - // create identical setting for Google - googleCfg := model.Config{ - GoogleSettings: model.SSOSettings{}, - } - googleCfg.GoogleSettings = validConfig.OpenIdSettings - - // call with different service, same config settings - validFunctionCalled = 0 - provider.GetSSOSettings(&googleCfg, model.ServiceGoogle) - assert.Equal(t, model.ServiceGoogle, provider.CacheData.Service) - assert.Equal(t, 1, validFunctionCalled) - }) - - t.Run("ChangeConfigSettings", func(t *testing.T) { - secondFunctionCalled := 0 - secondServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Add("Cache-Control", "max-age=3600") - fmt.Fprintln(w, validJSON) - secondFunctionCalled++ - })) - defer secondServer.Close() - - newCfg := validConfig - newCfg.OpenIdSettings.DiscoveryEndpoint = model.NewString(secondServer.URL) - - // new URL - settings, err := provider.GetSSOSettings(&newCfg, model.ServiceOpenid) - require.NoError(t, err) - assert.Equal(t, "authorization_endpoint", *settings.AuthEndpoint) - assert.Equal(t, "token_endpoint", *settings.TokenEndpoint) - assert.Equal(t, "userinfo_endpoint", *settings.UserAPIEndpoint) - assert.Equal(t, 1, secondFunctionCalled) - - // new secret - newCfg.OpenIdSettings.Secret = model.NewString("NewSecret") - _, err = provider.GetSSOSettings(&newCfg, model.ServiceOpenid) - require.NoError(t, err) - assert.Equal(t, newCfg.OpenIdSettings.Secret, provider.CacheData.Settings.Secret) - assert.Equal(t, 2, secondFunctionCalled) - - // new Id - newCfg.OpenIdSettings.Id = model.NewString("NewId") - _, err = provider.GetSSOSettings(&newCfg, model.ServiceOpenid) - require.NoError(t, err) - assert.Equal(t, newCfg.OpenIdSettings.Id, provider.CacheData.Settings.Id) - assert.Equal(t, 3, secondFunctionCalled) - }) -} - -func TestCacheControlPanic(t *testing.T) { - provider := &OpenIdProvider{ - CacheData: &CacheData{ - Service: model.ServiceOpenid, - }, - } - - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintln(w, "no header") - })) - defer ts.Close() - - cfg := &model.Config{ - OpenIdSettings: model.SSOSettings{ - DiscoveryEndpoint: model.NewString(ts.URL), - }, - } - - require.NotPanics(t, func() { - provider.GetSSOSettings(cfg, model.ServiceOpenid) - }) -} - -func TestIsSameUser(t *testing.T) { - provider := &OpenIdProvider{ - CacheData: &CacheData{ - Service: model.ServiceOpenid, - }, - } - cases := []struct { - dbUser model.User - oauthUser model.User - verified bool - }{ - {model.User{AuthData: model.NewString("202993a800824dc1b4496d598d47c58a")}, model.User{AuthData: model.NewString("202993a8-0082-4dc1-b449-6d598d47c58a")}, true}, - {model.User{AuthData: model.NewString("202993a85a824dc1b4496d598d47c58a")}, model.User{AuthData: model.NewString("")}, false}, - {model.User{AuthData: model.NewString("")}, model.User{AuthData: model.NewString("202993a8-5a82-4dc1-b449-6d598d47c58a")}, false}, - {model.User{AuthData: model.NewString("be95fe607df5dbeb")}, model.User{AuthData: model.NewString("00000000-0000-0000-be95-fe607df5dbeb")}, true}, - {model.User{AuthData: model.NewString("be95fe607df5dbeb")}, model.User{AuthData: model.NewString("00000000-0000-0000-be90-fe607df5dbeb")}, false}, - {model.User{AuthData: model.NewString("be95fe607df5dbeb")}, model.User{AuthData: model.NewString("00000000-0000-0000-be95-fe607df5dbe0")}, false}, - {model.User{AuthData: model.NewString("hello")}, model.User{}, false}, - } - for _, c := range cases { - verified := provider.IsSameUser(&c.dbUser, &c.oauthUser) - if verified != c.verified { - if c.verified { - t.Logf("'%v' should have matched '%v'", c.dbUser, c.oauthUser) - } else { - t.Logf("'%v' should not have matched '%v'", c.dbUser, c.oauthUser) - } - t.FailNow() - } - } -} From 617c0244765c33a981f6cce60756c9a10218435f Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Wed, 7 Dec 2022 10:00:47 +0300 Subject: [PATCH 3/6] app: avoid plugin env check for RunMultiHook and use pluginslock while accessing plugin env (#21803) * app/channels: use pluginslock while accessing plugins environment * when using RunMultiHook we don't need to do a nil check on plugin env * trigger ci --- app/channel.go | 88 ++++++++++++++++-------------------- app/channels.go | 4 +- app/file.go | 42 ++++++++--------- app/login.go | 33 ++++++-------- app/plugin.go | 15 +++--- app/post.go | 118 ++++++++++++++++++++++-------------------------- app/reaction.go | 32 ++++++------- app/team.go | 48 +++++++++----------- app/upload.go | 5 -- app/user.go | 16 +++---- 10 files changed, 179 insertions(+), 222 deletions(-) diff --git a/app/channel.go b/app/channel.go index 273555f6b8..fdc8d17da3 100644 --- a/app/channel.go +++ b/app/channel.go @@ -343,15 +343,13 @@ func (a *App) CreateChannel(c request.CTX, channel *model.Channel, addMember boo a.InvalidateCacheForUser(channel.CreatorId) } - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.ChannelHasBeenCreated(pluginContext, sc) - return true - }, plugin.ChannelHasBeenCreatedID) - }) - } + a.Srv().Go(func() { + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.ChannelHasBeenCreated(pluginContext, sc) + return true + }, plugin.ChannelHasBeenCreatedID) + }) return sc, nil } @@ -429,15 +427,13 @@ func (a *App) handleCreationEvent(c request.CTX, userID, otherUserID string, cha a.InvalidateCacheForUser(userID) a.InvalidateCacheForUser(otherUserID) - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.ChannelHasBeenCreated(pluginContext, channel) - return true - }, plugin.ChannelHasBeenCreatedID) - }) - } + a.Srv().Go(func() { + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.ChannelHasBeenCreated(pluginContext, channel) + return true + }, plugin.ChannelHasBeenCreatedID) + }) message := model.NewWebSocketEvent(model.WebsocketEventDirectAdded, "", channel.Id, "", nil, "") message.Add("creator_id", userID) @@ -1599,15 +1595,13 @@ func (a *App) AddChannelMember(c request.CTX, userID string, channel *model.Chan return nil, err } - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.UserHasJoinedChannel(pluginContext, cm, userRequestor) - return true - }, plugin.UserHasJoinedChannelID) - }) - } + a.Srv().Go(func() { + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.UserHasJoinedChannel(pluginContext, cm, userRequestor) + return true + }, plugin.UserHasJoinedChannelID) + }) if opts.UserRequestorID == "" || userID == opts.UserRequestorID { if err := a.postJoinChannelMessage(c, user, channel); err != nil { @@ -2177,15 +2171,13 @@ func (a *App) JoinChannel(c request.CTX, channel *model.Channel, userID string) return err } - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.UserHasJoinedChannel(pluginContext, cm, nil) - return true - }, plugin.UserHasJoinedChannelID) - }) - } + a.Srv().Go(func() { + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.UserHasJoinedChannel(pluginContext, cm, nil) + return true + }, plugin.UserHasJoinedChannelID) + }) if err := a.postJoinChannelMessage(c, user, channel); err != nil { return err @@ -2484,21 +2476,19 @@ func (a *App) removeUserFromChannel(c request.CTX, userIDToRemove string, remove a.InvalidateCacheForUser(userIDToRemove) a.invalidateCacheForChannelMembers(channel.Id) - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - var actorUser *model.User - if removerUserId != "" { - actorUser, _ = a.GetUser(removerUserId) - } - - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.UserHasLeftChannel(pluginContext, cm, actorUser) - return true - }, plugin.UserHasLeftChannelID) - }) + var actorUser *model.User + if removerUserId != "" { + actorUser, _ = a.GetUser(removerUserId) } + a.Srv().Go(func() { + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.UserHasLeftChannel(pluginContext, cm, actorUser) + return true + }, plugin.UserHasLeftChannelID) + }) + message := model.NewWebSocketEvent(model.WebsocketEventUserRemoved, "", channel.Id, "", nil, "") message.Add("user_id", userIDToRemove) message.Add("remover_id", removerUserId) diff --git a/app/channels.go b/app/channels.go index 31ef2d429c..e4a25f3749 100644 --- a/app/channels.go +++ b/app/channels.go @@ -326,7 +326,7 @@ func (s *hooksService) RegisterHooks(productID string, hooks any) error { } func (ch *Channels) RunMultiHook(hookRunnerFunc func(hooks plugin.Hooks) bool, hookId int) { - if env := ch.pluginsEnvironment; env != nil { + if env := ch.GetPluginsEnvironment(); env != nil { env.RunMultiPluginHook(hookRunnerFunc, hookId) } @@ -336,7 +336,7 @@ func (ch *Channels) RunMultiHook(hookRunnerFunc func(hooks plugin.Hooks) bool, h func (ch *Channels) HooksForPluginOrProduct(id string) (plugin.Hooks, error) { var hooks plugin.Hooks - if env := ch.pluginsEnvironment; env != nil { + if env := ch.GetPluginsEnvironment(); env != nil { // we intentionally ignore the error here, because the id can be a product id // we are going to check if we have the hooks or not hooks, _ = env.HooksForPlugin(id) diff --git a/app/file.go b/app/file.go index 67a227cab0..06a516bc86 100644 --- a/app/file.go +++ b/app/file.go @@ -895,29 +895,27 @@ func (a *App) DoUploadFileExpectModification(c request.CTX, now time.Time, rawTe info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb." + getFileExtFromMimeType(info.MimeType) } - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - var rejectionError *model.AppError - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - var newBytes bytes.Buffer - replacementInfo, rejectionReason := hooks.FileWillBeUploaded(pluginContext, info, bytes.NewReader(data), &newBytes) - if rejectionReason != "" { - rejectionError = model.NewAppError("DoUploadFile", "File rejected by plugin. "+rejectionReason, nil, "", http.StatusBadRequest) - return false - } - if replacementInfo != nil { - info = replacementInfo - } - if newBytes.Len() != 0 { - data = newBytes.Bytes() - info.Size = int64(len(data)) - } - - return true - }, plugin.FileWillBeUploadedID) - if rejectionError != nil { - return nil, data, rejectionError + var rejectionError *model.AppError + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + var newBytes bytes.Buffer + replacementInfo, rejectionReason := hooks.FileWillBeUploaded(pluginContext, info, bytes.NewReader(data), &newBytes) + if rejectionReason != "" { + rejectionError = model.NewAppError("DoUploadFile", "File rejected by plugin. "+rejectionReason, nil, "", http.StatusBadRequest) + return false } + if replacementInfo != nil { + info = replacementInfo + } + if newBytes.Len() != 0 { + data = newBytes.Bytes() + info.Size = int64(len(data)) + } + + return true + }, plugin.FileWillBeUploadedID) + if rejectionError != nil { + return nil, data, rejectionError } if _, err := a.WriteFile(bytes.NewReader(data), info.Path); err != nil { diff --git a/app/login.go b/app/login.go index 98279d7f2f..e0854bef37 100644 --- a/app/login.go +++ b/app/login.go @@ -157,17 +157,15 @@ func (a *App) GetUserForLogin(id, loginId string) (*model.User, *model.AppError) } func (a *App) DoLogin(c *request.Context, w http.ResponseWriter, r *http.Request, user *model.User, deviceID string, isMobile, isOAuthUser, isSaml bool) *model.AppError { - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - var rejectionReason string - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - rejectionReason = hooks.UserWillLogIn(pluginContext, user) - return rejectionReason == "" - }, plugin.UserWillLogInID) + var rejectionReason string + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + rejectionReason = hooks.UserWillLogIn(pluginContext, user) + return rejectionReason == "" + }, plugin.UserWillLogInID) - if rejectionReason != "" { - return model.NewAppError("DoLogin", "Login rejected by plugin: "+rejectionReason, nil, "", http.StatusBadRequest) - } + if rejectionReason != "" { + return model.NewAppError("DoLogin", "Login rejected by plugin: "+rejectionReason, nil, "", http.StatusBadRequest) } session := &model.Session{UserId: user.Id, Roles: user.GetRawRoles(), DeviceId: deviceID, IsOAuth: false, Props: map[string]string{ @@ -226,15 +224,12 @@ func (a *App) DoLogin(c *request.Context, w http.ResponseWriter, r *http.Request }) } - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.UserHasLoggedIn(pluginContext, user) - return true - }, plugin.UserHasLoggedInID) - }) - } + a.Srv().Go(func() { + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.UserHasLoggedIn(pluginContext, user) + return true + }, plugin.UserHasLoggedInID) + }) return nil } diff --git a/app/plugin.go b/app/plugin.go index 7ce6c47fe6..679576e723 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -286,14 +286,13 @@ func (ch *Channels) initPlugins(c *request.Context, pluginDir, webappPluginDir s ch.installFeatureFlagPlugins() ch.syncPluginsActiveState() } - if pluginsEnvironment := ch.GetPluginsEnvironment(); pluginsEnvironment != nil { - ch.RunMultiHook(func(hooks plugin.Hooks) bool { - if err := hooks.OnConfigurationChange(); err != nil { - ch.srv.Log().Error("Plugin OnConfigurationChange hook failed", mlog.Err(err)) - } - return true - }, plugin.OnConfigurationChangeID) - } + + ch.RunMultiHook(func(hooks plugin.Hooks) bool { + if err := hooks.OnConfigurationChange(); err != nil { + ch.srv.Log().Error("Plugin OnConfigurationChange hook failed", mlog.Err(err)) + } + return true + }, plugin.OnConfigurationChangeID) }) ch.pluginsLock.Unlock() diff --git a/app/post.go b/app/post.go index ace950239e..a0daea37c6 100644 --- a/app/post.go +++ b/app/post.go @@ -263,38 +263,36 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel post.Metadata.Priority = nil } - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - var metadata *model.PostMetadata - if post.Metadata != nil { - metadata = post.Metadata.Copy() - } - var rejectionError *model.AppError - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - replacementPost, rejectionReason := hooks.MessageWillBePosted(pluginContext, post.ForPlugin()) - if rejectionReason != "" { - id := "Post rejected by plugin. " + rejectionReason - if rejectionReason == plugin.DismissPostError { - id = plugin.DismissPostError - } - rejectionError = model.NewAppError("createPost", id, nil, "", http.StatusBadRequest) - return false + var metadata *model.PostMetadata + if post.Metadata != nil { + metadata = post.Metadata.Copy() + } + var rejectionError *model.AppError + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + replacementPost, rejectionReason := hooks.MessageWillBePosted(pluginContext, post.ForPlugin()) + if rejectionReason != "" { + id := "Post rejected by plugin. " + rejectionReason + if rejectionReason == plugin.DismissPostError { + id = plugin.DismissPostError } - if replacementPost != nil { - post = replacementPost - if post.Metadata != nil && metadata != nil { - post.Metadata.Priority = metadata.Priority - } else { - post.Metadata = metadata - } - } - - return true - }, plugin.MessageWillBePostedID) - - if rejectionError != nil { - return nil, rejectionError + rejectionError = model.NewAppError("createPost", id, nil, "", http.StatusBadRequest) + return false } + if replacementPost != nil { + post = replacementPost + if post.Metadata != nil && metadata != nil { + post.Metadata.Priority = metadata.Priority + } else { + post.Metadata = metadata + } + } + + return true + }, plugin.MessageWillBePostedID) + + if rejectionError != nil { + return nil, rejectionError } // Pre-fill the CreateAt field for link previews to get the correct timestamp. @@ -328,16 +326,13 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel // We make a copy of the post for the plugin hook to avoid a race condition, // and to remove the non-GOB-encodable Metadata from it. - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - pluginPost := rpost.ForPlugin() - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.MessageHasBeenPosted(pluginContext, pluginPost) - return true - }, plugin.MessageHasBeenPostedID) - }) - } + pluginPost := rpost.ForPlugin() + a.Srv().Go(func() { + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.MessageHasBeenPosted(pluginContext, pluginPost) + return true + }, plugin.MessageHasBeenPostedID) + }) if a.Metrics() != nil { a.Metrics().IncrementPostCreate() @@ -658,20 +653,18 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) oldPost.RemoteId = model.NewString(*post.RemoteId) } - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - var rejectionReason string - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - newPost, rejectionReason = hooks.MessageWillBeUpdated(pluginContext, newPost.ForPlugin(), oldPost.ForPlugin()) - return post != nil - }, plugin.MessageWillBeUpdatedID) - if newPost == nil { - return nil, model.NewAppError("UpdatePost", "Post rejected by plugin. "+rejectionReason, nil, "", http.StatusBadRequest) - } - // Restore the post metadata that was stripped by the plugin. Set it to - // the last known good. - newPost.Metadata = oldPost.Metadata + var rejectionReason string + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + newPost, rejectionReason = hooks.MessageWillBeUpdated(pluginContext, newPost.ForPlugin(), oldPost.ForPlugin()) + return post != nil + }, plugin.MessageWillBeUpdatedID) + if newPost == nil { + return nil, model.NewAppError("UpdatePost", "Post rejected by plugin. "+rejectionReason, nil, "", http.StatusBadRequest) } + // Restore the post metadata that was stripped by the plugin. Set it to + // the last known good. + newPost.Metadata = oldPost.Metadata rpost, nErr := a.Srv().Store().Post().Update(newPost, oldPost) if nErr != nil { @@ -684,17 +677,14 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) } } - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - pluginOldPost := oldPost.ForPlugin() - pluginNewPost := newPost.ForPlugin() - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.MessageHasBeenUpdated(pluginContext, pluginNewPost, pluginOldPost) - return true - }, plugin.MessageHasBeenUpdatedID) - }) - } + pluginOldPost := oldPost.ForPlugin() + pluginNewPost := newPost.ForPlugin() + a.Srv().Go(func() { + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.MessageHasBeenUpdated(pluginContext, pluginNewPost, pluginOldPost) + return true + }, plugin.MessageHasBeenUpdatedID) + }) rpost = a.PreparePostForClientWithEmbedsAndImages(c, rpost, false, true, true) diff --git a/app/reaction.go b/app/reaction.go index 80bc24b4b4..fc6d54699f 100644 --- a/app/reaction.go +++ b/app/reaction.go @@ -43,15 +43,13 @@ func (a *App) SaveReactionForPost(c *request.Context, reaction *model.Reaction) // The post is always modified since the UpdateAt always changes a.invalidateCacheForChannelPosts(post.ChannelId) - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.ReactionHasBeenAdded(pluginContext, reaction) - return true - }, plugin.ReactionHasBeenAddedID) - }) - } + pluginContext := pluginContext(c) + a.Srv().Go(func() { + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.ReactionHasBeenAdded(pluginContext, reaction) + return true + }, plugin.ReactionHasBeenAddedID) + }) a.Srv().Go(func() { a.sendReactionEvent(model.WebsocketEventReactionAdded, reaction, post) @@ -142,15 +140,13 @@ func (a *App) DeleteReactionForPost(c *request.Context, reaction *model.Reaction // The post is always modified since the UpdateAt always changes a.invalidateCacheForChannelPosts(post.ChannelId) - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.ReactionHasBeenRemoved(pluginContext, reaction) - return true - }, plugin.ReactionHasBeenRemovedID) - }) - } + pluginContext := pluginContext(c) + a.Srv().Go(func() { + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.ReactionHasBeenRemoved(pluginContext, reaction) + return true + }, plugin.ReactionHasBeenRemovedID) + }) a.Srv().Go(func() { a.sendReactionEvent(model.WebsocketEventReactionRemoved, reaction, post) diff --git a/app/team.go b/app/team.go index 4376e2d77a..222d10e442 100644 --- a/app/team.go +++ b/app/team.go @@ -846,21 +846,19 @@ func (a *App) JoinUserToTeam(c request.CTX, team *model.Team, user *model.User, a.InvalidateCacheForUser(user.Id) a.invalidateCacheForUserTeams(user.Id) - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - var actor *model.User - if userRequestorId != "" { - actor, _ = a.GetUser(userRequestorId) - } - - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.UserHasJoinedTeam(pluginContext, teamMember, actor) - return true - }, plugin.UserHasJoinedTeamID) - }) + var actor *model.User + if userRequestorId != "" { + actor, _ = a.GetUser(userRequestorId) } + a.Srv().Go(func() { + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.UserHasJoinedTeam(pluginContext, teamMember, actor) + return true + }, plugin.UserHasJoinedTeamID) + }) + message := model.NewWebSocketEvent(model.WebsocketEventAddedToTeam, "", "", user.Id, nil, "") message.Add("team_id", team.Id) message.Add("user_id", user.Id) @@ -1220,21 +1218,19 @@ func (a *App) RemoveUserFromTeam(c request.CTX, teamID string, userID string, re } func (a *App) postProcessTeamMemberLeave(c request.CTX, teamMember *model.TeamMember, requestorId string) *model.AppError { - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - var actor *model.User - if requestorId != "" { - actor, _ = a.GetUser(requestorId) - } - - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.UserHasLeftTeam(pluginContext, teamMember, actor) - return true - }, plugin.UserHasLeftTeamID) - }) + var actor *model.User + if requestorId != "" { + actor, _ = a.GetUser(requestorId) } + a.Srv().Go(func() { + pluginContext := pluginContext(c) + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.UserHasLeftTeam(pluginContext, teamMember, actor) + return true + }, plugin.UserHasLeftTeamID) + }) + user, nErr := a.Srv().Store().User().Get(context.Background(), teamMember.UserId) if nErr != nil { var nfErr *store.ErrNotFound diff --git a/app/upload.go b/app/upload.go index b63725b135..4da0b853b6 100644 --- a/app/upload.go +++ b/app/upload.go @@ -49,11 +49,6 @@ func (a *App) genFileInfoFromReader(name string, file io.ReadSeeker, size int64) } func (a *App) runPluginsHook(c *request.Context, info *model.FileInfo, file io.Reader) *model.AppError { - pluginsEnvironment := a.GetPluginsEnvironment() - if pluginsEnvironment == nil { - return nil - } - filePath := info.Path // using a pipe to avoid loading the whole file content in memory. r, w := io.Pipe() diff --git a/app/user.go b/app/user.go index 953446071d..a2f72eddb5 100644 --- a/app/user.go +++ b/app/user.go @@ -308,15 +308,13 @@ func (a *App) createUserOrGuest(c request.CTX, user *model.User, guest bool) (*m message.Add("user_id", ruser.Id) a.Publish(message) - if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { - a.Srv().Go(func() { - pluginContext := pluginContext(c) - a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { - hooks.UserHasBeenCreated(pluginContext, ruser) - return true - }, plugin.UserHasBeenCreatedID) - }) - } + pluginContext := pluginContext(c) + a.Srv().Go(func() { + a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { + hooks.UserHasBeenCreated(pluginContext, ruser) + return true + }, plugin.UserHasBeenCreatedID) + }) _, cwsErr := a.SendSubscriptionHistoryEvent(ruser.Id) if cwsErr != nil { From 6e9dbbd237bd5f60d2727169dd8a6ba11837746b Mon Sep 17 00:00:00 2001 From: Konstantinos Pittas Date: Wed, 7 Dec 2022 11:16:56 +0200 Subject: [PATCH 4/6] [MM-46463] Return last_picture_update for new team members (#21758) * return last picture update * add API test Co-authored-by: Mattermod --- api4/insights_test.go | 41 ++++++++++++++++++++++++++++++++++-- model/insights.go | 15 +++++++------ store/sqlstore/team_store.go | 2 +- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/api4/insights_test.go b/api4/insights_test.go index e304a60a49..58a4f95d97 100644 --- a/api4/insights_test.go +++ b/api4/insights_test.go @@ -7,10 +7,12 @@ import ( "testing" "time" - "github.com/mattermost/mattermost-server/v6/model" - "github.com/mattermost/mattermost-server/v6/store" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost-server/v6/model" + "github.com/mattermost/mattermost-server/v6/store" + "github.com/mattermost/mattermost-server/v6/utils/testutils" ) // Top Reactions @@ -1157,6 +1159,41 @@ func TestNewTeamMembersSince(t *testing.T) { CheckNotImplementedStatus(t, resp) }) + t.Run("includes all data for the user", func(t *testing.T) { + checkUser := func(ntm *model.NewTeamMember, hasProfilePicture bool) { + require.Equal(t, th.BasicUser.Id, ntm.Id) + require.Equal(t, th.BasicUser.Username, ntm.Username) + require.Equal(t, th.BasicUser.FirstName, ntm.FirstName) + require.Equal(t, th.BasicUser.LastName, ntm.LastName) + require.Equal(t, th.BasicUser.Position, ntm.Position) + require.Equal(t, th.BasicUser.Nickname, ntm.Nickname) + member, err := th.App.GetTeamMember(team.Id, th.BasicUser.Id) + require.Nil(t, err) + require.Equal(t, member.CreateAt, ntm.CreateAt) + if hasProfilePicture { + require.Truef(t, ntm.LastPictureUpdate > int64(0), "should be greater than 0, but was %d", ntm.LastPictureUpdate) + } else { + require.Equal(t, int64(0), ntm.LastPictureUpdate) + } + } + + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) + list, resp, err := th.Client.GetNewTeamMembersSince(team.Id, model.TimeRangeToday, 0, 5) + require.NoError(t, err) + CheckOKStatus(t, resp) + checkUser(list.Items[0], false) + + data, err := testutils.ReadTestFile("test.png") + require.NoError(t, err) + _, err = th.Client.SetProfileImage(th.BasicUser.Id, data) + require.NoError(t, err) + + list, resp, err = th.Client.GetNewTeamMembersSince(team.Id, model.TimeRangeToday, 0, 5) + require.NoError(t, err) + CheckOKStatus(t, resp) + checkUser(list.Items[0], true) + }) + t.Run("implements pagination", func(t *testing.T) { // check the first page of results list, resp, err := th.Client.GetNewTeamMembersSince(team.Id, model.TimeRangeToday, 0, 2) diff --git a/model/insights.go b/model/insights.go index d66d48c283..437cdeec37 100644 --- a/model/insights.go +++ b/model/insights.go @@ -118,13 +118,14 @@ type NewTeamMembersList struct { } type NewTeamMember struct { - Id string `json:"id"` - Username string `json:"username"` - FirstName string `json:"first_name"` - LastName string `json:"last_name"` - Position string `json:"position"` - Nickname string `json:"nickname"` - CreateAt int64 `json:"create_at"` + Id string `json:"id"` + Username string `json:"username"` + FirstName string `json:"first_name"` + LastName string `json:"last_name"` + Position string `json:"position"` + Nickname string `json:"nickname"` + LastPictureUpdate int64 `json:"last_picture_update,omitempty"` + CreateAt int64 `json:"create_at"` } type DurationPostCount struct { diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index cbe8535a21..00b3d731ae 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -1675,7 +1675,7 @@ func (s SqlTeamStore) GetNewTeamMembersSince(teamID string, since int64, offset return nil, 0, errors.Wrap(err, "failed to count team members since") } - newTeamMembersBuilder := builderF("Users.Id, Users.Username, Users.FirstName, Users.LastName, Users.Position, TeamMembers.CreateAt, Users.Nickname"). + newTeamMembersBuilder := builderF("Users.Id, Users.Username, Users.FirstName, Users.LastName, Users.Position, Users.LastPictureUpdate, TeamMembers.CreateAt, Users.Nickname"). Limit(uint64(limit + 1)). Offset(uint64(offset)) query, args, err = newTeamMembersBuilder.ToSql() From b51c06e5a67e06ce5c26b9fb31c380b9499f90af Mon Sep 17 00:00:00 2001 From: Kyriakos Z <3829551+koox00@users.noreply.github.com> Date: Wed, 7 Dec 2022 16:44:48 +0200 Subject: [PATCH 5/6] Enables PostPriority FeatureFlag by default (#21720) Automatic Merge --- model/feature_flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/feature_flags.go b/model/feature_flags.go index ae1005b30d..9356df0b89 100644 --- a/model/feature_flags.go +++ b/model/feature_flags.go @@ -98,7 +98,7 @@ func (f *FeatureFlags) SetDefaults() { f.CallsEnabled = true f.BoardsProduct = false f.SendWelcomePost = true - f.PostPriority = false + f.PostPriority = true f.PeopleProduct = false f.WorkTemplate = false f.AnnualSubscription = false From 3872f24b0c2a5ed027897d5d0bb5d9196ebc2e23 Mon Sep 17 00:00:00 2001 From: Nick Misasi Date: Wed, 7 Dec 2022 10:05:48 -0500 Subject: [PATCH 6/6] [MM-47566] Add upcominginvoice field to Subscription type (#21717) * Add upcominginvoice field to Subscription type * Add support for fetching upcoming invoices * Put back upcominginvoice Co-authored-by: Mattermod --- api4/cloud.go | 2 +- model/cloud.go | 3 +++ web/context.go | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/api4/cloud.go b/api4/cloud.go index 3f9106710c..208fa4b6b1 100644 --- a/api4/cloud.go +++ b/api4/cloud.go @@ -39,7 +39,7 @@ func (api *API) InitCloud() { // GET /api/v4/cloud/subscription api.BaseRoutes.Cloud.Handle("/subscription", api.APISessionRequired(getSubscription)).Methods("GET") api.BaseRoutes.Cloud.Handle("/subscription/invoices", api.APISessionRequired(getInvoicesForSubscription)).Methods("GET") - api.BaseRoutes.Cloud.Handle("/subscription/invoices/{invoice_id:in_[A-Za-z0-9]+}/pdf", api.APISessionRequired(getSubscriptionInvoicePDF)).Methods("GET") + api.BaseRoutes.Cloud.Handle("/subscription/invoices/{invoice_id:[A-Za-z0-9]+}/pdf", api.APISessionRequired(getSubscriptionInvoicePDF)).Methods("GET") api.BaseRoutes.Cloud.Handle("/subscription", api.APISessionRequired(changeSubscription)).Methods("PUT") // GET /api/v4/cloud/request-trial diff --git a/model/cloud.go b/model/cloud.go index 2985c929ea..a409c77f98 100644 --- a/model/cloud.go +++ b/model/cloud.go @@ -16,6 +16,8 @@ const ( EventTypeTriggerDelinquencyEmail = "trigger-delinquency-email" ) +const UpcomingInvoice = "upcoming" + var MockCWS string type BillingScheme string @@ -152,6 +154,7 @@ type Subscription struct { DNS string `json:"dns"` IsPaidTier string `json:"is_paid_tier"` LastInvoice *Invoice `json:"last_invoice"` + UpcomingInvoice *Invoice `json:"upcoming_invoice"` IsFreeTrial string `json:"is_free_trial"` TrialEndAt int64 `json:"trial_end_at"` DelinquentSince *int64 `json:"delinquent_since"` diff --git a/web/context.go b/web/context.go index 056dca354c..2286f806c6 100644 --- a/web/context.go +++ b/web/context.go @@ -743,7 +743,7 @@ func (c *Context) RequireInvoiceId() *Context { return c } - if len(c.Params.InvoiceId) != 27 { + if len(c.Params.InvoiceId) != 27 && c.Params.InvoiceId != model.UpcomingInvoice { c.SetInvalidURLParam("invoice_id") }