From d38328976e2c8bb0fab91e656042a0d8ac37bc76 Mon Sep 17 00:00:00 2001 From: JoramWilander Date: Wed, 6 Sep 2017 16:24:34 -0400 Subject: Various patches --- api/general.go | 10 +++++++++- api/general_test.go | 20 ++++++++++++++++++++ api/oauth.go | 4 ++++ api/oauth_test.go | 42 +++++++++++++++++++++++++++++++++++++----- api4/oauth.go | 9 +++++++++ api4/oauth_test.go | 12 ++++++++++-- api4/status.go | 6 +++--- api4/status_test.go | 14 ++++++++++++++ 8 files changed, 106 insertions(+), 11 deletions(-) diff --git a/api/general.go b/api/general.go index 16a739704..ceb0b209f 100644 --- a/api/general.go +++ b/api/general.go @@ -10,6 +10,7 @@ import ( l4g "github.com/alecthomas/log4go" + "github.com/mattermost/platform/app" "github.com/mattermost/platform/model" "github.com/mattermost/platform/utils" ) @@ -30,7 +31,14 @@ func logClient(c *Context, w http.ResponseWriter, r *http.Request) { forceToDebug := false if !*utils.Cfg.ServiceSettings.EnableDeveloper { - forceToDebug = true + if c.Session.UserId == "" { + c.Err = model.NewAppError("Permissions", "api.context.permissions.app_error", nil, "", http.StatusForbidden) + return + } + + if !app.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) { + forceToDebug = true + } } m := model.MapFromJson(r.Body) diff --git a/api/general_test.go b/api/general_test.go index 51593ab9e..1fb041ae6 100644 --- a/api/general_test.go +++ b/api/general_test.go @@ -5,6 +5,8 @@ package api import ( "testing" + + "github.com/mattermost/platform/utils" ) func TestGetClientProperties(t *testing.T) { @@ -25,6 +27,24 @@ func TestLogClient(t *testing.T) { if ret, _ := th.BasicClient.LogClient("this is a test"); !ret { t.Fatal("failed to log") } + + enableDeveloper := *utils.Cfg.ServiceSettings.EnableDeveloper + defer func() { + *utils.Cfg.ServiceSettings.EnableDeveloper = enableDeveloper + }() + *utils.Cfg.ServiceSettings.EnableDeveloper = false + + th.BasicClient.Logout() + + if _, err := th.BasicClient.LogClient("this is a test"); err == nil { + t.Fatal("should have failed") + } + + *utils.Cfg.ServiceSettings.EnableDeveloper = true + + if ret, _ := th.BasicClient.LogClient("this is a test"); !ret { + t.Fatal("failed to log") + } } func TestGetPing(t *testing.T) { diff --git a/api/oauth.go b/api/oauth.go index a239e889b..bd52d05ed 100644 --- a/api/oauth.go +++ b/api/oauth.go @@ -41,6 +41,10 @@ func registerOAuthApp(c *Context, w http.ResponseWriter, r *http.Request) { return } + if !app.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) { + oauthApp.IsTrusted = false + } + oauthApp.CreatorId = c.Session.UserId rapp, err := app.CreateOAuthApp(oauthApp) diff --git a/api/oauth_test.go b/api/oauth_test.go index 584b4183b..612563622 100644 --- a/api/oauth_test.go +++ b/api/oauth_test.go @@ -22,7 +22,7 @@ func TestOAuthRegisterApp(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() Client := th.BasicClient - oauthApp := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}, IsTrusted: true} utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { @@ -82,10 +82,29 @@ func TestOAuthRegisterApp(t *testing.T) { Client.Logout() Client.Login(user.Email, user.Password) - oauthApp = &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp = &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}, IsTrusted: true} if _, err := Client.RegisterApp(oauthApp); err == nil { t.Fatal("should have failed. not enough permissions") } + + adminOnly := *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations + defer func() { + *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = adminOnly + utils.SetDefaultRolesBasedOnConfig() + }() + *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = false + utils.SetDefaultRolesBasedOnConfig() + + th.LoginBasic() + + if result, err := th.BasicClient.RegisterApp(oauthApp); err != nil { + t.Fatal(err) + } else { + rapp := result.Data.(*model.OAuthApp) + if rapp.IsTrusted { + t.Fatal("trusted should be false - created by non admin") + } + } } func TestOAuthAllow(t *testing.T) { @@ -463,7 +482,17 @@ func TestOAuthAuthorize(t *testing.T) { th := Setup().InitBasic() Client := th.BasicClient + enableOAuth := utils.Cfg.ServiceSettings.EnableOAuthServiceProvider + adminOnly := *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations + defer func() { + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = enableOAuth + *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = adminOnly + utils.SetDefaultRolesBasedOnConfig() + }() utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false + *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = false + utils.SetDefaultRolesBasedOnConfig() + if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { if r, err := HttpGet(Client.Url+"/oauth/authorize", Client.HttpClient, "", true); err == nil { t.Fatal("should have failed - oauth providing turned off") @@ -483,7 +512,7 @@ func TestOAuthAuthorize(t *testing.T) { } // register an app to authorize it - oauthApp := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://example.com"}} oauthApp = Client.Must(Client.RegisterApp(oauthApp)).Data.(*model.OAuthApp) if r, err := HttpGet(Client.Url+"/oauth/authorize?client_id="+oauthApp.Id+"&&redirect_uri=http://example.com&response_type="+model.AUTHCODE_RESPONSE_TYPE, Client.HttpClient, "", true); err == nil { t.Fatal("should have failed - user not logged") @@ -491,9 +520,12 @@ func TestOAuthAuthorize(t *testing.T) { } authToken := Client.AuthType + " " + Client.AuthToken - if r, err := HttpGet(Client.Url+"/oauth/authorize?client_id="+oauthApp.Id+"&redirect_uri=http://example.com&response_type="+model.AUTHCODE_RESPONSE_TYPE, Client.HttpClient, authToken, true); err != nil { + if _, err := HttpGet(Client.Url+"/oauth/authorize?client_id="+oauthApp.Id+"&redirect_uri=http://bad-redirect.com&response_type="+model.AUTHCODE_RESPONSE_TYPE, Client.HttpClient, authToken, true); err == nil { + t.Fatal("should have failed - bad redirect uri") + } + + if _, err := HttpGet(Client.Url+"/oauth/authorize?client_id="+oauthApp.Id+"&redirect_uri=https://example.com&response_type="+model.AUTHCODE_RESPONSE_TYPE, Client.HttpClient, authToken, true); err != nil { t.Fatal(err) - closeBody(r) } // lets authorize the app diff --git a/api4/oauth.go b/api4/oauth.go index ae5035fdc..392129143 100644 --- a/api4/oauth.go +++ b/api4/oauth.go @@ -57,6 +57,10 @@ func createOAuthApp(c *Context, w http.ResponseWriter, r *http.Request) { return } + if !app.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) { + oauthApp.IsTrusted = false + } + oauthApp.CreatorId = c.Session.UserId rapp, err := app.CreateOAuthApp(oauthApp) @@ -298,6 +302,11 @@ func authorizeOAuthPage(c *Context, w http.ResponseWriter, r *http.Request) { return } + if !oauthApp.IsValidRedirectURL(authRequest.RedirectUri) { + utils.RenderWebError(model.NewAppError("authorizeOAuthPage", "api.oauth.allow_oauth.redirect_callback.app_error", nil, "", http.StatusBadRequest), w, r) + return + } + isAuthorized := false if _, err := app.GetPreferenceByCategoryAndNameForUser(c.Session.UserId, model.PREFERENCE_CATEGORY_AUTHORIZED_OAUTH_APP, authRequest.ClientId); err == nil { diff --git a/api4/oauth_test.go b/api4/oauth_test.go index 963cd43c3..ceb44a44e 100644 --- a/api4/oauth_test.go +++ b/api4/oauth_test.go @@ -28,7 +28,7 @@ func TestCreateOAuthApp(t *testing.T) { utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true utils.SetDefaultRolesBasedOnConfig() - oapp := &model.OAuthApp{Name: GenerateTestAppName(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oapp := &model.OAuthApp{Name: GenerateTestAppName(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}, IsTrusted: true} rapp, resp := AdminClient.CreateOAuthApp(oapp) CheckNoError(t, resp) @@ -38,6 +38,10 @@ func TestCreateOAuthApp(t *testing.T) { t.Fatal("names did not match") } + if rapp.IsTrusted != oapp.IsTrusted { + t.Fatal("trusted did no match") + } + *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = true utils.SetDefaultRolesBasedOnConfig() _, resp = Client.CreateOAuthApp(oapp) @@ -45,10 +49,14 @@ func TestCreateOAuthApp(t *testing.T) { *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = false utils.SetDefaultRolesBasedOnConfig() - _, resp = Client.CreateOAuthApp(oapp) + rapp, resp = Client.CreateOAuthApp(oapp) CheckNoError(t, resp) CheckCreatedStatus(t, resp) + if rapp.IsTrusted { + t.Fatal("trusted should be false - created by non admin") + } + oapp.Name = "" _, resp = AdminClient.CreateOAuthApp(oapp) CheckBadRequestStatus(t, resp) diff --git a/api4/status.go b/api4/status.go index 3a2c5c762..42d2c8777 100644 --- a/api4/status.go +++ b/api4/status.go @@ -16,9 +16,9 @@ import ( func InitStatus() { l4g.Debug(utils.T("api.status.init.debug")) - BaseRoutes.User.Handle("/status", ApiHandler(getUserStatus)).Methods("GET") - BaseRoutes.Users.Handle("/status/ids", ApiHandler(getUserStatusesByIds)).Methods("POST") - BaseRoutes.User.Handle("/status", ApiHandler(updateUserStatus)).Methods("PUT") + BaseRoutes.User.Handle("/status", ApiSessionRequired(getUserStatus)).Methods("GET") + BaseRoutes.Users.Handle("/status/ids", ApiSessionRequired(getUserStatusesByIds)).Methods("POST") + BaseRoutes.User.Handle("/status", ApiSessionRequired(updateUserStatus)).Methods("PUT") } func getUserStatus(c *Context, w http.ResponseWriter, r *http.Request) { diff --git a/api4/status_test.go b/api4/status_test.go index c8277b3de..27e1fa53f 100644 --- a/api4/status_test.go +++ b/api4/status_test.go @@ -47,6 +47,10 @@ func TestGetUserStatus(t *testing.T) { } Client.Logout() + + _, resp = Client.GetUserStatus(th.BasicUser2.Id, "") + CheckUnauthorizedStatus(t, resp) + th.LoginBasic2() userStatus, resp = Client.GetUserStatus(th.BasicUser2.Id, "") CheckNoError(t, resp) @@ -89,6 +93,11 @@ func TestGetUsersStatusesByIds(t *testing.T) { t.Fatal("Status should be offline") } } + + Client.Logout() + + _, resp = Client.GetUsersStatusesByIds(usersIds) + CheckUnauthorizedStatus(t, resp) } func TestUpdateUserStatus(t *testing.T) { @@ -126,4 +135,9 @@ func TestUpdateUserStatus(t *testing.T) { if updateUserStatus.Status != "online" { t.Fatal("Should return online status") } + + Client.Logout() + + _, resp = Client.UpdateUserStatus(th.BasicUser2.Id, toUpdateUserStatus) + CheckUnauthorizedStatus(t, resp) } -- cgit v1.2.3-1-g7c22 From c68b866ef76234dbe65bf012509b7f3cde2bb24c Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 6 Sep 2017 17:16:25 -0500 Subject: more robust file upload extension (#7389) --- webapp/components/file_upload.jsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/webapp/components/file_upload.jsx b/webapp/components/file_upload.jsx index eb966aeed..479dfa145 100644 --- a/webapp/components/file_upload.jsx +++ b/webapp/components/file_upload.jsx @@ -296,8 +296,16 @@ class FileUpload extends React.Component { min = String(d.getMinutes()); } - const ext = file.name.lastIndexOf('.'); - const name = formatMessage(holders.pasted) + d.getFullYear() + '-' + (d.getMonth() + 1) + '-' + d.getDate() + ' ' + hour + '-' + min + (ext >= 0 ? file.name.substr(ext) : ''); + var ext = ''; + if (file.name) { + if (file.name.includes('.')) { + ext = file.name.substr(file.name.lastIndexOf('.')); + } + } else if (items[i].type.includes('/')) { + ext = '.' + items[i].type.split('/')[1].toLowerCase(); + } + + const name = formatMessage(holders.pasted) + d.getFullYear() + '-' + (d.getMonth() + 1) + '-' + d.getDate() + ' ' + hour + '-' + min + ext; const request = uploadFile( file, -- cgit v1.2.3-1-g7c22 From e589accdaf38bb82cb5d3b5dd84eadf9bfb58b5c Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Wed, 6 Sep 2017 23:39:44 -0400 Subject: Fix webapp plugins for production builds (#7392) --- webapp/components/profile_popover.jsx | 4 ++++ webapp/plugins/pluggable/pluggable.jsx | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/webapp/components/profile_popover.jsx b/webapp/components/profile_popover.jsx index 9e7d7636a..63812428c 100644 --- a/webapp/components/profile_popover.jsx +++ b/webapp/components/profile_popover.jsx @@ -19,6 +19,10 @@ import PropTypes from 'prop-types'; import React from 'react'; export default class ProfilePopover extends React.Component { + static getComponentName() { + return 'ProfilePopover'; + } + constructor(props) { super(props); diff --git a/webapp/plugins/pluggable/pluggable.jsx b/webapp/plugins/pluggable/pluggable.jsx index 566e024e5..c81d8df5e 100644 --- a/webapp/plugins/pluggable/pluggable.jsx +++ b/webapp/plugins/pluggable/pluggable.jsx @@ -33,6 +33,8 @@ export default class Pluggable extends React.PureComponent { return null; } + const childName = child.getComponentName(); + // Include any props passed to this component or to the child component let props = {...this.props}; Reflect.deleteProperty(props, 'children'); @@ -40,8 +42,8 @@ export default class Pluggable extends React.PureComponent { props = {...props, ...this.props.children.props}; // Override the default component with any registered plugin's component - if (components.hasOwnProperty(child.name)) { - const PluginComponent = components[child.name]; + if (components.hasOwnProperty(childName)) { + const PluginComponent = components[childName]; return (