From 0ecda4e093911515171c50c3306e7c98e12fd666 Mon Sep 17 00:00:00 2001 From: Douglas Lovell Date: Mon, 10 Sep 2018 14:53:33 -0300 Subject: [PATCH 01/43] Display password change from in Account Settings --- services/web/app/views/user/settings.pug | 103 ++++++++++------------- 1 file changed, 46 insertions(+), 57 deletions(-) diff --git a/services/web/app/views/user/settings.pug b/services/web/app/views/user/settings.pug index b6f4818d7f..9681da50f4 100644 --- a/services/web/app/views/user/settings.pug +++ b/services/web/app/views/user/settings.pug @@ -33,7 +33,7 @@ block content required, ng-model="email", ng-init="email = "+JSON.stringify(user.email), - ng-model-options="{ updateOn: 'blur' }" + ng-model-options="{ pdateOn: 'blur' }" ) span.small.text-primary(ng-show="settingsForm.email.$invalid && settingsForm.email.$dirty") | #{translate("must_be_email_address")} @@ -73,62 +73,51 @@ block content label.control-label #{translate("last_name")} div.form-control(readonly="true") #{user.last_name} - - if !externalAuthenticationSystemUsed() - .col-md-5.col-md-offset-1 - h3 #{translate("change_password")} - form(async-form="changepassword", name="changePasswordForm", action="/user/password/update", method="POST", novalidate) - input(type="hidden", name="_csrf", value=csrfToken) - .form-group - label(for='currentPassword') #{translate("current_password")} - input.form-control( - type='password', - name='currentPassword', - placeholder='*********', - ng-model="currentPassword", - required - ) - span.small.text-primary(ng-show="changePasswordForm.currentPassword.$invalid && changePasswordForm.currentPassword.$dirty") - | #{translate("required")} - .form-group - label(for='newPassword1') #{translate("new_password")} - input.form-control( - id='passwordField', - type='password', - name='newPassword1', - placeholder='*********', - ng-model="newPassword1", - required, - complex-password - ) - span.small.text-primary(ng-show="changePasswordForm.newPassword1.$error.complexPassword && changePasswordForm.newPassword1.$dirty", ng-bind-html="complexPasswordErrorMessage") - .form-group - label(for='newPassword2') #{translate("confirm_new_password")} - input.form-control( - type='password', - name='newPassword2', - placeholder='*********', - ng-model="newPassword2", - equals="passwordField" - ) - span.small.text-primary(ng-show="changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$dirty") - | #{translate("doesnt_match")} - span.small.text-primary(ng-show="!changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$invalid && changePasswordForm.newPassword2.$dirty") - | #{translate("invalid_password")} - .actions - button.btn.btn-primary( - type='submit', - ng-disabled="changePasswordForm.$invalid" - ) #{translate("change")} - - else - if settings.overleaf && settings.createV1AccountOnLogin - .col-md-5.col-md-offset-1 - h3 #{translate("change_password")} - p - | To change your password, - | please go to #[a(href=settings.overleaf.host+'/users/edit') Overleaf v1 settings] - + .col-md-5.col-md-offset-1 + h3 #{translate("change_password")} + form(async-form="changepassword", name="changePasswordForm", action="/user/password/update", method="POST", novalidate) + input(type="hidden", name="_csrf", value=csrfToken) + .form-group + label(for='currentPassword') #{translate("current_password")} + input.form-control( + type='password', + name='currentPassword', + placeholder='*********', + ng-model="currentPassword", + required + ) + span.small.text-primary(ng-show="changePasswordForm.currentPassword.$invalid && changePasswordForm.currentPassword.$dirty") + | #{translate("required")} + .form-group + label(for='newPassword1') #{translate("new_password")} + input.form-control( + id='passwordField', + type='password', + name='newPassword1', + placeholder='*********', + ng-model="newPassword1", + required, + complex-password + ) + span.small.text-primary(ng-show="changePasswordForm.newPassword1.$error.complexPassword && changePasswordForm.newPassword1.$dirty", ng-bind-html="complexPasswordErrorMessage") + .form-group + label(for='newPassword2') #{translate("confirm_new_password")} + input.form-control( + type='password', + name='newPassword2', + placeholder='*********', + ng-model="newPassword2", + equals="passwordField" + ) + span.small.text-primary(ng-show="changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$dirty") + | #{translate("doesnt_match")} + span.small.text-primary(ng-show="!changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$invalid && changePasswordForm.newPassword2.$dirty") + | #{translate("invalid_password")} + .actions + button.btn.btn-primary( + type='submit', + ng-disabled="changePasswordForm.$invalid" + ) #{translate("change")} | !{moduleIncludes("userSettings", locals)} From 7d10e64840c46689187d6af111a5bcc0637cec99 Mon Sep 17 00:00:00 2001 From: Douglas Lovell Date: Fri, 14 Sep 2018 16:19:02 -0300 Subject: [PATCH 02/43] Alter endpoint for account settings, change password form submission. --- services/web/app/views/user/settings.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/views/user/settings.pug b/services/web/app/views/user/settings.pug index 9681da50f4..497de3a01d 100644 --- a/services/web/app/views/user/settings.pug +++ b/services/web/app/views/user/settings.pug @@ -75,7 +75,7 @@ block content .col-md-5.col-md-offset-1 h3 #{translate("change_password")} - form(async-form="changepassword", name="changePasswordForm", action="/user/password/update", method="POST", novalidate) + form(async-form="changepassword", name="changePasswordForm", action="/change_password/v1", method="POST", novalidate) input(type="hidden", name="_csrf", value=csrfToken) .form-group label(for='currentPassword') #{translate("current_password")} From 4abbc5d5697fa333609d824bf726b37997071861 Mon Sep 17 00:00:00 2001 From: Douglas Lovell Date: Tue, 18 Sep 2018 09:10:33 -0300 Subject: [PATCH 03/43] Password change form conditioned on installation --- services/web/app/views/user/settings.pug | 101 +++++++++++++---------- 1 file changed, 58 insertions(+), 43 deletions(-) diff --git a/services/web/app/views/user/settings.pug b/services/web/app/views/user/settings.pug index 497de3a01d..3d7692dd7c 100644 --- a/services/web/app/views/user/settings.pug +++ b/services/web/app/views/user/settings.pug @@ -75,49 +75,64 @@ block content .col-md-5.col-md-offset-1 h3 #{translate("change_password")} - form(async-form="changepassword", name="changePasswordForm", action="/change_password/v1", method="POST", novalidate) - input(type="hidden", name="_csrf", value=csrfToken) - .form-group - label(for='currentPassword') #{translate("current_password")} - input.form-control( - type='password', - name='currentPassword', - placeholder='*********', - ng-model="currentPassword", - required - ) - span.small.text-primary(ng-show="changePasswordForm.currentPassword.$invalid && changePasswordForm.currentPassword.$dirty") - | #{translate("required")} - .form-group - label(for='newPassword1') #{translate("new_password")} - input.form-control( - id='passwordField', - type='password', - name='newPassword1', - placeholder='*********', - ng-model="newPassword1", - required, - complex-password - ) - span.small.text-primary(ng-show="changePasswordForm.newPassword1.$error.complexPassword && changePasswordForm.newPassword1.$dirty", ng-bind-html="complexPasswordErrorMessage") - .form-group - label(for='newPassword2') #{translate("confirm_new_password")} - input.form-control( - type='password', - name='newPassword2', - placeholder='*********', - ng-model="newPassword2", - equals="passwordField" - ) - span.small.text-primary(ng-show="changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$dirty") - | #{translate("doesnt_match")} - span.small.text-primary(ng-show="!changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$invalid && changePasswordForm.newPassword2.$dirty") - | #{translate("invalid_password")} - .actions - button.btn.btn-primary( - type='submit', - ng-disabled="changePasswordForm.$invalid" - ) #{translate("change")} + if externalAuthenticationSystemUsed() && !settings.overleaf + p + Password settings are managed externally to Overleaf + else + - var submitAction + if settings.overleaf + - submitAction = '/change_password/v1' + else + - submitAction = '/user/password/update' + form( + async-form="changepassword" + name="changePasswordForm" + action=submitAction + method="POST" + novalidate + ) + input(type="hidden", name="_csrf", value=csrfToken) + .form-group + label(for='currentPassword') #{translate("current_password")} + input.form-control( + type='password', + name='currentPassword', + placeholder='*********', + ng-model="currentPassword", + required + ) + span.small.text-primary(ng-show="changePasswordForm.currentPassword.$invalid && changePasswordForm.currentPassword.$dirty") + | #{translate("required")} + .form-group + label(for='newPassword1') #{translate("new_password")} + input.form-control( + id='passwordField', + type='password', + name='newPassword1', + placeholder='*********', + ng-model="newPassword1", + required, + complex-password + ) + span.small.text-primary(ng-show="changePasswordForm.newPassword1.$error.complexPassword && changePasswordForm.newPassword1.$dirty", ng-bind-html="complexPasswordErrorMessage") + .form-group + label(for='newPassword2') #{translate("confirm_new_password")} + input.form-control( + type='password', + name='newPassword2', + placeholder='*********', + ng-model="newPassword2", + equals="passwordField" + ) + span.small.text-primary(ng-show="changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$dirty") + | #{translate("doesnt_match")} + span.small.text-primary(ng-show="!changePasswordForm.newPassword2.$error.areEqual && changePasswordForm.newPassword2.$invalid && changePasswordForm.newPassword2.$dirty") + | #{translate("invalid_password")} + .actions + button.btn.btn-primary( + type='submit', + ng-disabled="changePasswordForm.$invalid" + ) #{translate("change")} | !{moduleIncludes("userSettings", locals)} From 98be2c2bf55b97d1a616c12013019fec0348f4e3 Mon Sep 17 00:00:00 2001 From: Douglas Lovell Date: Wed, 19 Sep 2018 07:58:04 -0300 Subject: [PATCH 04/43] Update wording and endpoint for change password form --- services/web/app/views/user/settings.pug | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/app/views/user/settings.pug b/services/web/app/views/user/settings.pug index 3d7692dd7c..b66d82e044 100644 --- a/services/web/app/views/user/settings.pug +++ b/services/web/app/views/user/settings.pug @@ -77,11 +77,11 @@ block content h3 #{translate("change_password")} if externalAuthenticationSystemUsed() && !settings.overleaf p - Password settings are managed externally to Overleaf + Password settings are managed externally else - var submitAction if settings.overleaf - - submitAction = '/change_password/v1' + - submitAction = '/user/change_password/v1' else - submitAction = '/user/password/update' form( From 52859cdfaa9911225ed98c8ca3099a8251c2ca2b Mon Sep 17 00:00:00 2001 From: hugh-obrien Date: Thu, 27 Sep 2018 16:11:11 +0100 Subject: [PATCH 05/43] make the zip fetching endpoint for exports generic to either zips or pdfs --- .../app/coffee/Features/Exports/ExportsController.coffee | 9 +++++---- .../app/coffee/Features/Exports/ExportsHandler.coffee | 5 ++--- services/web/app/coffee/router.coffee | 2 +- .../test/unit/coffee/Exports/ExportsHandlerTests.coffee | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/services/web/app/coffee/Features/Exports/ExportsController.coffee b/services/web/app/coffee/Features/Exports/ExportsController.coffee index ea82cf6a04..fc339f9957 100644 --- a/services/web/app/coffee/Features/Exports/ExportsController.coffee +++ b/services/web/app/coffee/Features/Exports/ExportsController.coffee @@ -46,10 +46,11 @@ module.exports = } res.send export_json: json - exportZip: (req, res) -> - {export_id} = req.params + exportDownload: (req, res) -> + {type, export_id} = req.params + AuthenticationController.getLoggedInUserId(req) - ExportsHandler.fetchZip export_id, (err, export_zip_url) -> + ExportsHandler.fetchDownload export_id, type, (err, export_file_url) -> return err if err? - res.redirect export_zip_url + res.redirect export_file_url diff --git a/services/web/app/coffee/Features/Exports/ExportsHandler.coffee b/services/web/app/coffee/Features/Exports/ExportsHandler.coffee index 885f063c8b..18644a66e6 100644 --- a/services/web/app/coffee/Features/Exports/ExportsHandler.coffee +++ b/services/web/app/coffee/Features/Exports/ExportsHandler.coffee @@ -122,10 +122,9 @@ module.exports = ExportsHandler = self = logger.err err:err, export:export_id, "v1 export returned failure status code: #{res.statusCode}" callback err - fetchZip: (export_id, callback=(err, zip_url) ->) -> - console.log("#{settings.apis.v1.url}/api/v1/sharelatex/exports/#{export_id}/zip_url") + fetchDownload: (export_id, type, callback=(err, file_url) ->) -> request.get { - url: "#{settings.apis.v1.url}/api/v1/sharelatex/exports/#{export_id}/zip_url" + url: "#{settings.apis.v1.url}/api/v1/sharelatex/exports/#{export_id}/#{type}_url" auth: {user: settings.apis.v1.user, pass: settings.apis.v1.pass } }, (err, res, body) -> if err? diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 578ff0ceb7..b9f48805c4 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -247,7 +247,7 @@ module.exports = class Router webRouter.post '/project/:project_id/export/:brand_variation_id', AuthorizationMiddlewear.ensureUserCanAdminProject, ExportsController.exportProject webRouter.get '/project/:project_id/export/:export_id', AuthorizationMiddlewear.ensureUserCanAdminProject, ExportsController.exportStatus - webRouter.get '/project/:project_id/export/:export_id/zip', AuthorizationMiddlewear.ensureUserCanAdminProject, ExportsController.exportZip + webRouter.get '/project/:project_id/export/:export_id/:type', AuthorizationMiddlewear.ensureUserCanAdminProject, ExportsController.exportDownload webRouter.get '/Project/:Project_id/download/zip', AuthorizationMiddlewear.ensureUserCanReadProject, ProjectDownloadsController.downloadProject webRouter.get '/project/download/zip', AuthorizationMiddlewear.ensureUserCanReadMultipleProjects, ProjectDownloadsController.downloadMultipleProjects diff --git a/services/web/test/unit/coffee/Exports/ExportsHandlerTests.coffee b/services/web/test/unit/coffee/Exports/ExportsHandlerTests.coffee index edd1ce127a..f766feb6dd 100644 --- a/services/web/test/unit/coffee/Exports/ExportsHandlerTests.coffee +++ b/services/web/test/unit/coffee/Exports/ExportsHandlerTests.coffee @@ -301,7 +301,7 @@ describe 'ExportsHandler', -> @callback.calledWith(null, { body: @body }) .should.equal true - describe 'fetchZip', -> + describe 'fetchDownload', -> beforeEach (done) -> @settings.apis = v1: @@ -316,7 +316,7 @@ describe 'ExportsHandler', -> describe "when all goes well", -> beforeEach (done) -> @stubRequest.get = @stubGet - @ExportsHandler.fetchZip @export_id, (error, body) => + @ExportsHandler.fetchDownload @export_id, 'zip', (error, body) => @callback(error, body) done() From 5ff66187a0d25927cb436d16517dbf409ee893b4 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Wed, 3 Oct 2018 16:00:10 +0100 Subject: [PATCH 06/43] remove unused confirmed field from user model --- services/web/app/coffee/models/User.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/services/web/app/coffee/models/User.coffee b/services/web/app/coffee/models/User.coffee index 23b59375f4..ef9a93b2f2 100644 --- a/services/web/app/coffee/models/User.coffee +++ b/services/web/app/coffee/models/User.coffee @@ -19,7 +19,6 @@ UserSchema = new Schema institution : {type : String, default : ''} hashedPassword : String isAdmin : {type : Boolean, default : false} - confirmed : {type : Boolean, default : false} signUpDate : {type : Date, default: () -> new Date() } lastLoggedIn : {type : Date} lastLoginIp : {type : String, default : ''} From 6692d06e5fa3ec774ed31d833d00dcef03b58f8e Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Thu, 4 Oct 2018 11:24:01 +0100 Subject: [PATCH 07/43] Handle image_src in metadata layout The CMS is already using `image`, which is an object based on data from the API. --- services/web/app/views/_metadata.pug | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/web/app/views/_metadata.pug b/services/web/app/views/_metadata.pug index 4857162a9e..253757b6a8 100644 --- a/services/web/app/views/_metadata.pug +++ b/services/web/app/views/_metadata.pug @@ -25,10 +25,16 @@ //- Image -if (metadata && metadata.image) + //- from the CMS meta(itemprop="image", name="image", content=metadata.image.fields.file.url) +-else if (metadata && metadata.image_src) + //- pages with custom metadata images, metadata.image_src is the full image URL + meta(itemprop="image", name="image", content=metadata.image_src) -else if (settings.overleaf) + //- the default image for Overleaf meta(itemprop="image", name="image", content=buildImgPath('ol-brand/overleaf_og_logo.png')) -else + //- the default image for ShareLaTeX meta(itemprop="image", name="image", content='/touch-icon-192x192.png') //- Keywords From d51549c4f0be55d3888125bc8de71ca85d647964 Mon Sep 17 00:00:00 2001 From: Chrystal Griffiths Date: Fri, 5 Oct 2018 11:19:20 +0100 Subject: [PATCH 08/43] Use ng-if to switch between pdf and editor --- services/web/app/views/project/editor/editor.pug | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/web/app/views/project/editor/editor.pug b/services/web/app/views/project/editor/editor.pug index fda0fa6446..93368ceba4 100644 --- a/services/web/app/views/project/editor/editor.pug +++ b/services/web/app/views/project/editor/editor.pug @@ -102,7 +102,6 @@ div.full-size( ) i.synctex-control-icon div.full-size( - ng-if="ui.pdfLayout == 'flat'" - ng-show="ui.view == 'pdf'" + ng-if="ui.pdfLayout == 'flat' && ui.view == 'pdf'" ) include ./pdf From a2ef0e1ae551c9184511a88d3790b184bf053958 Mon Sep 17 00:00:00 2001 From: Simon Detheridge Date: Fri, 5 Oct 2018 12:04:00 +0100 Subject: [PATCH 09/43] Add additional CTA link to emails Some mail clients don't show the CTA button correctly, or at all. Add an additional, smaller link to the bottom of the email for people who can't see the button. bug: sharelatex/web-sharelatex-internal#987 Signed-off-by: Simon Detheridge --- .../coffee/Features/Email/Bodies/SingleCTAEmailBody.coffee | 5 +++++ .../Features/Email/Bodies/ol-SingleCTAEmailBody.coffee | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/services/web/app/coffee/Features/Email/Bodies/SingleCTAEmailBody.coffee b/services/web/app/coffee/Features/Email/Bodies/SingleCTAEmailBody.coffee index bffa485f07..192f572beb 100644 --- a/services/web/app/coffee/Features/Email/Bodies/SingleCTAEmailBody.coffee +++ b/services/web/app/coffee/Features/Email/Bodies/SingleCTAEmailBody.coffee @@ -32,6 +32,11 @@ module.exports = _.template """ <%= secondaryMessage %>

<% } %> +
 
+

+ If the button above does not appear, please open the link in your browser here:
+ <%= ctaURL %> +

diff --git a/services/web/app/coffee/Features/Email/Bodies/ol-SingleCTAEmailBody.coffee b/services/web/app/coffee/Features/Email/Bodies/ol-SingleCTAEmailBody.coffee index ee9907c019..2b522d6c6c 100644 --- a/services/web/app/coffee/Features/Email/Bodies/ol-SingleCTAEmailBody.coffee +++ b/services/web/app/coffee/Features/Email/Bodies/ol-SingleCTAEmailBody.coffee @@ -32,6 +32,11 @@ module.exports = _.template """ <%= secondaryMessage %>

<% } %> +
 
+

+ If the button above does not appear, please open the link in your browser here:
+ <%= ctaURL %> +

From d316a761061ff09b5dc9bea0d046089a95fc79ce Mon Sep 17 00:00:00 2001 From: Simon Detheridge Date: Fri, 5 Oct 2018 13:16:42 +0100 Subject: [PATCH 10/43] Revert "add plain text link to email address confirmation emails" This reverts commit 48cd7e604dcc5f3b7ae8eb699f14b44bc073f107. --- services/web/app/coffee/Features/Email/EmailBuilder.coffee | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index f88fff3057..3326f8522b 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -97,8 +97,6 @@ templates.confirmEmail = CTAEmailTemplate({ title: () -> "Confirm Email" message: () -> "Please confirm your email on #{settings.appName}." ctaText: () -> "Confirm Email" - secondaryMessage: (opts) -> - "If the button does not appear, open this link in your browser: #{opts.confirmEmailUrl}" ctaURL: (opts) -> opts.confirmEmailUrl }) From 286f25529a11213cb35b8d45ed20393738258b27 Mon Sep 17 00:00:00 2001 From: Simon Detheridge Date: Fri, 5 Oct 2018 13:19:05 +0100 Subject: [PATCH 11/43] Remove secondary CTA link from account merge confirmation email bug: sharelatex/web-sharelatex-internal#987 Signed-off-by: Simon Detheridge --- services/web/app/coffee/Features/Email/EmailBuilder.coffee | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index 3326f8522b..2f37901dbe 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -51,8 +51,6 @@ templates.accountMergeToOverleafAddress = CTAEmailTemplate({ """ ctaText: () -> "Confirm Account Merge" ctaURL: (opts) -> opts.tokenLinkUrl - secondaryMessage: (opts) -> - "If the button does not appear, open this link in your browser: #{opts.tokenLinkUrl}" }) templates.accountMergeToSharelatexAddress = templates.accountMergeToOverleafAddress From 7b4b75b51a94aa523a0f284cd751191e3c031b69 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Fri, 5 Oct 2018 16:24:05 +0100 Subject: [PATCH 12/43] fix incorrect or missing test mocks --- .../coffee/Authentication/AuthenticationControllerTests.coffee | 2 ++ .../test/unit/coffee/Templates/TemplatesControllerTests.coffee | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee index 78f7b288f6..2f18743a1c 100644 --- a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee @@ -30,6 +30,8 @@ describe "AuthenticationController", -> revokeAllUserSessions: sinon.stub().callsArgWith(1, null) "../../infrastructure/Modules": @Modules = {hooks: {fire: sinon.stub().callsArgWith(2, null, [])}} "../SudoMode/SudoModeHandler": @SudoModeHandler = {activateSudoMode: sinon.stub().callsArgWith(1, null)} + "../Notifications/NotificationsBuilder": @NotificationsBuilder = + ipMatcherAffiliation: sinon.stub() @user = _id: ObjectId() email: @email = "USER@example.com" diff --git a/services/web/test/unit/coffee/Templates/TemplatesControllerTests.coffee b/services/web/test/unit/coffee/Templates/TemplatesControllerTests.coffee index a08789ede9..1e61b14c0b 100644 --- a/services/web/test/unit/coffee/Templates/TemplatesControllerTests.coffee +++ b/services/web/test/unit/coffee/Templates/TemplatesControllerTests.coffee @@ -59,7 +59,7 @@ describe 'TemplatesController', -> "uuid":v4:=>@uuid "request": @request "fs":@fs - "../../../../app/js/models/Project": {Project: @Project} + "../../../js/models/Project": {Project: @Project} @zipUrl = "%2Ftemplates%2F52fb86a81ae1e566597a25f6%2Fv%2F4%2Fzip&templateName=Moderncv%20Banking&compiler=pdflatex" @templateName = "project name here" @user_id = "1234" From c00a0a435daa57dfd43363e5b92003035249d8f7 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Fri, 5 Oct 2018 16:24:28 +0100 Subject: [PATCH 13/43] remove console.log in tests --- .../web/test/unit/coffee/Compile/CompileControllerTests.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/services/web/test/unit/coffee/Compile/CompileControllerTests.coffee b/services/web/test/unit/coffee/Compile/CompileControllerTests.coffee index 103a1f9eea..0d070d687e 100644 --- a/services/web/test/unit/coffee/Compile/CompileControllerTests.coffee +++ b/services/web/test/unit/coffee/Compile/CompileControllerTests.coffee @@ -194,7 +194,6 @@ describe "CompileController", -> .should.equal true it "should set the content-disposition header with a safe version of the project name", -> - console.log @res.setContentDisposition.args[0] @res.setContentDisposition .calledWith('', filename: "test_nam_.pdf") .should.equal true From e99165b475d506c8871d5f7fce2be3ec5b5d6fc0 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 4 Oct 2018 10:03:21 +0100 Subject: [PATCH 14/43] Validate password length when registering --- .../app/coffee/Features/User/UserRegistrationHandler.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee index 1291142dab..0928f64640 100644 --- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee +++ b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee @@ -20,10 +20,13 @@ module.exports = UserRegistrationHandler = hasZeroLength = true return hasZeroLength + isTooShort: (prop, length) -> + return prop.length < length + _registrationRequestIsValid : (body, callback)-> email = EmailHelper.parseEmail(body.email) or '' password = body.password - if @hasZeroLengths([password, email]) + if @hasZeroLengths([password, email]) or @isTooShort(password, 6) return false else return true From bf60fe7f6cba1f4e9df2ddbc0117aa640f3970be Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 4 Oct 2018 10:36:10 +0100 Subject: [PATCH 15/43] Add error handling for InvalidError --- .../web/app/coffee/Features/Errors/ErrorController.coffee | 4 ++++ services/web/app/coffee/Features/Errors/Errors.coffee | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/services/web/app/coffee/Features/Errors/ErrorController.coffee b/services/web/app/coffee/Features/Errors/ErrorController.coffee index c4f3089fe0..50f0ba3c06 100644 --- a/services/web/app/coffee/Features/Errors/ErrorController.coffee +++ b/services/web/app/coffee/Features/Errors/ErrorController.coffee @@ -25,6 +25,10 @@ module.exports = ErrorController = else if error instanceof Errors.TooManyRequestsError logger.warn {err: error, url: req.url}, "too many requests error" res.sendStatus(429) + else if error instanceof Errors.InvalidError + logger.warn {err: error, url: req.url}, "invalid error" + res.status(400) + res.send(error.message) else if error instanceof Errors.InvalidNameError logger.warn {err: error, url: req.url}, "invalid name error" res.status(400) diff --git a/services/web/app/coffee/Features/Errors/Errors.coffee b/services/web/app/coffee/Features/Errors/Errors.coffee index 94aeaa2a90..3239bbbb58 100644 --- a/services/web/app/coffee/Features/Errors/Errors.coffee +++ b/services/web/app/coffee/Features/Errors/Errors.coffee @@ -82,6 +82,13 @@ EmailExistsError = (message) -> return error EmailExistsError.prototype.__proto__ = Error.prototype +InvalidError = (message) -> + error = new Error(message) + error.name = "InvalidError" + error.__proto__ = InvalidError.prototype + return error +InvalidError.prototype.__proto__ = Error.prototype + module.exports = Errors = NotFoundError: NotFoundError ServiceNotConfiguredError: ServiceNotConfiguredError @@ -95,3 +102,4 @@ module.exports = Errors = V1ConnectionError: V1ConnectionError UnconfirmedEmailError: UnconfirmedEmailError EmailExistsError: EmailExistsError + InvalidError: InvalidError From 1fe8aebf5bdcc1e5d20c709f723d7d023d1f1ca7 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 4 Oct 2018 10:36:22 +0100 Subject: [PATCH 16/43] Add error handling for 400 responses --- services/web/public/coffee/directives/asyncForm.coffee | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/web/public/coffee/directives/asyncForm.coffee b/services/web/public/coffee/directives/asyncForm.coffee index acafec563d..9baadfd1f2 100644 --- a/services/web/public/coffee/directives/asyncForm.coffee +++ b/services/web/public/coffee/directives/asyncForm.coffee @@ -70,7 +70,11 @@ define [ onErrorHandler(httpResponse) return - if status == 403 # Forbidden + if status == 400 # Bad Request + response.message = + text: "Invalid Request. Please correct the data and try again." + type: 'error' + else if status == 403 # Forbidden response.message = text: "Session error. Please check you have cookies enabled. If the problem persists, try clearing your cache and cookies." type: "error" From 44c86b3769dcc6db8c2d6bee5cc767b55400b675 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 4 Oct 2018 12:00:33 +0100 Subject: [PATCH 17/43] Refactor to use password strength options --- .../AuthenticationManager.coffee | 18 +++++++++++++++--- .../User/UserRegistrationHandler.coffee | 15 +++------------ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee index 49bd994b2c..26c31de84c 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee @@ -28,13 +28,25 @@ module.exports = AuthenticationManager = else callback null, null - setUserPassword: (user_id, password, callback = (error) ->) -> + validateEmail: (email) -> + if !email?.length + return { message: 'email not set' } + return null + + validatePassword: (password) -> + if !password? + return { message: 'password not set' } if (Settings.passwordStrengthOptions?.length?.max? and Settings.passwordStrengthOptions?.length?.max < password.length) - return callback("password is too long") + return { message: 'password is too short' } if (Settings.passwordStrengthOptions?.length?.min? and Settings.passwordStrengthOptions?.length?.min > password.length) - return callback("password is too short") + return { message: "password is too short" } + return null + + setUserPassword: (user_id, password, callback = (error) ->) -> + validation = validatePassword(password) + return callback(validation.message) if validation? bcrypt.genSalt BCRYPT_ROUNDS, (error, salt) -> return callback(error) if error? diff --git a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee index 0928f64640..1fa1dc0d79 100644 --- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee +++ b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee @@ -13,20 +13,11 @@ settings = require "settings-sharelatex" EmailHelper = require("../Helpers/EmailHelper") module.exports = UserRegistrationHandler = - hasZeroLengths : (props) -> - hasZeroLength = false - props.forEach (prop) -> - if prop.length == 0 - hasZeroLength = true - return hasZeroLength - - isTooShort: (prop, length) -> - return prop.length < length - _registrationRequestIsValid : (body, callback)-> email = EmailHelper.parseEmail(body.email) or '' - password = body.password - if @hasZeroLengths([password, email]) or @isTooShort(password, 6) + invalidEmail = AuthenticationManager.validateEmail(email) + invalidPassword = AuthenticationManager.validatePassword(body.password) + if invalidEmail? or invalidPassword? return false else return true From 676557a05179d73a07ff26a0d13eb4eb8bba94eb Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 4 Oct 2018 13:47:53 +0100 Subject: [PATCH 18/43] Refactor to validate in AuthenticationManager --- .../Features/Authentication/AuthenticationManager.coffee | 8 +++++--- .../coffee/Features/User/UserRegistrationHandler.coffee | 5 ++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee index 26c31de84c..bedaf60d79 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee @@ -3,6 +3,7 @@ User = require("../../models/User").User {db, ObjectId} = require("../../infrastructure/mongojs") crypto = require 'crypto' bcrypt = require 'bcrypt' +EmailHelper = require("../Helpers/EmailHelper") BCRYPT_ROUNDS = Settings?.security?.bcryptRounds or 12 @@ -29,8 +30,9 @@ module.exports = AuthenticationManager = callback null, null validateEmail: (email) -> - if !email?.length - return { message: 'email not set' } + parsed = EmailHelper.parseEmail(email) + if !parsed? + return { message: 'email not valid' } return null validatePassword: (password) -> @@ -45,7 +47,7 @@ module.exports = AuthenticationManager = return null setUserPassword: (user_id, password, callback = (error) ->) -> - validation = validatePassword(password) + validation = @validatePassword(password) return callback(validation.message) if validation? bcrypt.genSalt BCRYPT_ROUNDS, (error, salt) -> diff --git a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee index 1fa1dc0d79..52d731c4bc 100644 --- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee +++ b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee @@ -14,9 +14,8 @@ EmailHelper = require("../Helpers/EmailHelper") module.exports = UserRegistrationHandler = _registrationRequestIsValid : (body, callback)-> - email = EmailHelper.parseEmail(body.email) or '' - invalidEmail = AuthenticationManager.validateEmail(email) - invalidPassword = AuthenticationManager.validatePassword(body.password) + invalidEmail = AuthenticationManager.validateEmail(body.email or '') + invalidPassword = AuthenticationManager.validatePassword(body.password or '') if invalidEmail? or invalidPassword? return false else From 1ef947b1febd7cb38bf4ebd06c7e1d0d0c9779a1 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 4 Oct 2018 13:48:57 +0100 Subject: [PATCH 19/43] Fix tests after refactoring register validation --- .../AuthenticationManagerTests.coffee | 42 +++++++++++++++++++ .../User/UserRegistrationHandlerTests.coffee | 33 +++++++-------- 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/services/web/test/unit/coffee/Authentication/AuthenticationManagerTests.coffee b/services/web/test/unit/coffee/Authentication/AuthenticationManagerTests.coffee index 0a041a0865..be2ed44979 100644 --- a/services/web/test/unit/coffee/Authentication/AuthenticationManagerTests.coffee +++ b/services/web/test/unit/coffee/Authentication/AuthenticationManagerTests.coffee @@ -94,6 +94,48 @@ describe "AuthenticationManager", -> it "should not return a user", -> @callback.calledWith(null, null).should.equal true + describe "validateEmail", -> + describe "valid", -> + it "should return null", -> + result = @AuthenticationManager.validateEmail 'foo@example.com' + expect(result).to.equal null + + describe "invalid", -> + it "should return validation error object for no email", -> + result = @AuthenticationManager.validateEmail '' + expect(result).to.not.equal null + expect(result.message).to.equal 'email not valid' + + it "should return validation error object for invalid", -> + result = @AuthenticationManager.validateEmail 'notanemail' + expect(result).to.not.equal null + expect(result.message).to.equal 'email not valid' + + describe "validatePassword", -> + it "should return null if valid", -> + result = @AuthenticationManager.validatePassword 'banana' + expect(result).to.equal null + + describe "invalid", -> + beforeEach -> + @settings.passwordStrengthOptions = + length: + max:10 + min:6 + + it "should return validation error object if not set", -> + result = @AuthenticationManager.validatePassword() + expect(result).to.not.equal null + expect(result.message).to.equal 'password not set' + + it "should return validation error object if too short", -> + result = @AuthenticationManager.validatePassword 'dsd' + expect(result).to.not.equal null + expect(result.message).to.equal 'password is too short' + + it "should return validation error object if too long", -> + result = @AuthenticationManager.validatePassword 'dsdsadsadsadsadsadkjsadjsadjsadljs' + describe "setUserPassword", -> beforeEach -> @user_id = ObjectId() diff --git a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee b/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee index f8bcce30ce..7fcd2147a5 100644 --- a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee +++ b/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee @@ -19,6 +19,8 @@ describe "UserRegistrationHandler", -> @UserCreator = createNewUser:sinon.stub().callsArgWith(1, null, @user) @AuthenticationManager = + validateEmail: sinon.stub().returns(null) + validatePassword: sinon.stub().returns(null) setUserPassword: sinon.stub().callsArgWith(2) @NewsLetterManager = subscribe: sinon.stub().callsArgWith(1) @@ -44,28 +46,25 @@ describe "UserRegistrationHandler", -> describe 'validate Register Request', -> - - - it 'allow working account through', -> + it 'allows passing validation through', -> result = @handler._registrationRequestIsValid @passingRequest result.should.equal true - - it 'not allow not valid email through ', ()-> - @passingRequest.email = "notemail" - result = @handler._registrationRequestIsValid @passingRequest - result.should.equal false - it 'not allow no email through ', -> - @passingRequest.email = "" - result = @handler._registrationRequestIsValid @passingRequest - result.should.equal false - - it 'not allow no password through ', ()-> - @passingRequest.password= "" - result = @handler._registrationRequestIsValid @passingRequest - result.should.equal false + describe 'failing email validation', -> + beforeEach -> + @AuthenticationManager.validateEmail.returns({ message: 'email not set' }) + it 'does not allow through', -> + result = @handler._registrationRequestIsValid @passingRequest + result.should.equal false + describe 'failing password validation', -> + beforeEach -> + @AuthenticationManager.validatePassword.returns({ message: 'password is too short' }) + + it 'does not allow through', -> + result = @handler._registrationRequestIsValid @passingRequest + result.should.equal false describe "registerNewUser", -> From 2edca417b180db65c003830fcaae2cbb119df040 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 4 Oct 2018 15:22:25 +0100 Subject: [PATCH 20/43] Reduce padding --- services/web/public/stylesheets/app/homepage.less | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/web/public/stylesheets/app/homepage.less b/services/web/public/stylesheets/app/homepage.less index 795d04488f..3295d769c4 100644 --- a/services/web/public/stylesheets/app/homepage.less +++ b/services/web/public/stylesheets/app/homepage.less @@ -94,6 +94,9 @@ border-radius: 9999px; } } + .register-banner__password-error { + padding: 2px; + } .screenshot { height: 600px; margin: auto; From af499e45395dc34bf4dfb040e1283ea97207bbe8 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 4 Oct 2018 15:49:42 +0100 Subject: [PATCH 21/43] Style error message on register form --- services/web/public/stylesheets/app/homepage.less | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/web/public/stylesheets/app/homepage.less b/services/web/public/stylesheets/app/homepage.less index 3295d769c4..8087a96dd0 100644 --- a/services/web/public/stylesheets/app/homepage.less +++ b/services/web/public/stylesheets/app/homepage.less @@ -94,8 +94,12 @@ border-radius: 9999px; } } + .hp-register-password-error { + margin-top: 5px; + } .register-banner__password-error { padding: 2px; + position: relative; } .screenshot { height: 600px; From 8777b0f5f88a5b69fb9a6fcd1c88265e378d4ca5 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 4 Oct 2018 16:40:50 +0100 Subject: [PATCH 22/43] Style error message after moving it above inputs --- services/web/public/stylesheets/app/homepage.less | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/services/web/public/stylesheets/app/homepage.less b/services/web/public/stylesheets/app/homepage.less index 8087a96dd0..afe1ebf357 100644 --- a/services/web/public/stylesheets/app/homepage.less +++ b/services/web/public/stylesheets/app/homepage.less @@ -95,10 +95,11 @@ } } .hp-register-password-error { - margin-top: 5px; + margin-bottom: 9px; } .register-banner__password-error { - padding: 2px; + padding: 3px; + border: none; position: relative; } .screenshot { From e37a54e25449ead87554aa331be3535db727f9fd Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 4 Oct 2018 16:51:42 +0100 Subject: [PATCH 23/43] Make error message match other styles --- services/web/public/stylesheets/app/homepage.less | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/public/stylesheets/app/homepage.less b/services/web/public/stylesheets/app/homepage.less index afe1ebf357..ff11ef3776 100644 --- a/services/web/public/stylesheets/app/homepage.less +++ b/services/web/public/stylesheets/app/homepage.less @@ -98,9 +98,9 @@ margin-bottom: 9px; } .register-banner__password-error { - padding: 3px; - border: none; - position: relative; + padding: 5px 9px; + border: none; + border-radius: @btn-border-radius-base; } .screenshot { height: 600px; From 04572f61bb444037414689ec17eb2c32be93b384 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Fri, 5 Oct 2018 10:18:53 +0100 Subject: [PATCH 24/43] Fix copy/paste error --- .../coffee/Features/Authentication/AuthenticationManager.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee index bedaf60d79..64da009387 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee @@ -43,7 +43,7 @@ module.exports = AuthenticationManager = return { message: 'password is too short' } if (Settings.passwordStrengthOptions?.length?.min? and Settings.passwordStrengthOptions?.length?.min > password.length) - return { message: "password is too short" } + return { message: "password is too long" } return null setUserPassword: (user_id, password, callback = (error) ->) -> From f26f30e677890d79394d7f40c7f0cdc8fd5d2f0b Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Fri, 5 Oct 2018 10:19:04 +0100 Subject: [PATCH 25/43] Fix spaces instead of tabs --- services/web/public/stylesheets/app/homepage.less | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/public/stylesheets/app/homepage.less b/services/web/public/stylesheets/app/homepage.less index ff11ef3776..9f60c58cb3 100644 --- a/services/web/public/stylesheets/app/homepage.less +++ b/services/web/public/stylesheets/app/homepage.less @@ -98,9 +98,9 @@ margin-bottom: 9px; } .register-banner__password-error { - padding: 5px 9px; - border: none; - border-radius: @btn-border-radius-base; + padding: 5px 9px; + border: none; + border-radius: @btn-border-radius-base; } .screenshot { height: 600px; From 8ef7f6c26db20e647ad3118484e5a60a866582a6 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Wed, 3 Oct 2018 13:59:55 +0100 Subject: [PATCH 26/43] Adjust margins to separate clickable items on templates --- .../public/stylesheets/app/templates-v2.less | 65 ++++++++++--------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/services/web/public/stylesheets/app/templates-v2.less b/services/web/public/stylesheets/app/templates-v2.less index 7f708670b4..8d8358a6b4 100644 --- a/services/web/public/stylesheets/app/templates-v2.less +++ b/services/web/public/stylesheets/app/templates-v2.less @@ -16,10 +16,14 @@ } .cta-links { - margin-bottom: @margin-md; - .cta-link.btn { - margin-right: @margin-sm; + margin: 0 @margin-sm @margin-sm 0; + } +} + +.popular-tags { + .template-thumbnail { + margin: 0 0 1em 0!important; } } @@ -28,25 +32,24 @@ } .templates-container { - column-count: 3; - column-gap: 1em; + column-count: 2; + column-gap: 2em; } .template-thumbnail { - &.template-thumbnail__container { - display: inline-block; - margin: 0 0 1em; - width: 100%; - } + display: inline-block; + margin: 0 0 2em; + width: 100%; .thumbnail { box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1); - margin: 5% 0; + margin: 0 0 @margin-sm 0; padding:0px; display: flex; justify-content: center; align-items: center; overflow: hidden; + width: 100%; h3 { color:@link-color; @@ -67,33 +70,37 @@ .caption__description { font-style: italic; - padding: 5px 0; + padding: 0 0 5px 0; .text-overflow(); } .caption__title { display: inline-block; - max-width: 100%; + width: 100%; + text-align: center; .text-overflow(); } +} - /* Media Queries */ - @media (max-width: @screen-md-min) { - .thumbnail { - margin: 5% auto; - } +.template-large-pdf-preview { + border: solid 1px @gray-lightest; + margin-top: @margin-lg; +} - .caption .description { - padding: 5px 50px; - } +/* Media Queries */ +@media (min-width: @screen-sm-min) { + .templates-container { + column-count: 3; + column-gap: 3em; } } - .section-tags { - margin-bottom: @margin-xl; - margin-top: @margin-md; - } - +@media (min-width: @screen-md-min) { .template-large-pdf-preview { - border: solid 1px @gray-lightest; - margin-bottom: 30px; - } \ No newline at end of file + margin-top: 0; + } +} + +.section-tags { + margin-bottom: @margin-xl; + margin-top: @margin-md; +} \ No newline at end of file From e129172553231a8bd9b310ccb09301aff09b3724 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 8 Oct 2018 11:25:24 +0100 Subject: [PATCH 27/43] Fix ordering of boolean check to be more readable --- .../Features/Authentication/AuthenticationManager.coffee | 8 ++++---- .../Authentication/AuthenticationManagerTests.coffee | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee index 64da009387..f381735969 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee @@ -39,11 +39,11 @@ module.exports = AuthenticationManager = if !password? return { message: 'password not set' } if (Settings.passwordStrengthOptions?.length?.max? and - Settings.passwordStrengthOptions?.length?.max < password.length) - return { message: 'password is too short' } - if (Settings.passwordStrengthOptions?.length?.min? and - Settings.passwordStrengthOptions?.length?.min > password.length) + password.length > Settings.passwordStrengthOptions?.length?.max) return { message: "password is too long" } + if (Settings.passwordStrengthOptions?.length?.min? and + password.length < Settings.passwordStrengthOptions?.length?.min) + return { message: 'password is too short' } return null setUserPassword: (user_id, password, callback = (error) ->) -> diff --git a/services/web/test/unit/coffee/Authentication/AuthenticationManagerTests.coffee b/services/web/test/unit/coffee/Authentication/AuthenticationManagerTests.coffee index be2ed44979..39880b112e 100644 --- a/services/web/test/unit/coffee/Authentication/AuthenticationManagerTests.coffee +++ b/services/web/test/unit/coffee/Authentication/AuthenticationManagerTests.coffee @@ -135,6 +135,8 @@ describe "AuthenticationManager", -> it "should return validation error object if too long", -> result = @AuthenticationManager.validatePassword 'dsdsadsadsadsadsadkjsadjsadjsadljs' + expect(result).to.not.equal null + expect(result.message).to.equal 'password is too long' describe "setUserPassword", -> beforeEach -> From e66210d2af973ae88b7bbde91a8c2bf15e5f650e Mon Sep 17 00:00:00 2001 From: Simon Detheridge Date: Sat, 6 Oct 2018 18:09:41 +0100 Subject: [PATCH 28/43] Add method to sanitize full paths For convenience, add a method to SafePath to break a path into components and verify the status of each one. bug: overleaf/sharelatex#908 Signed-off-by: Simon Detheridge --- .../coffee/Features/Project/SafePath.coffee | 9 +++ .../unit/coffee/Project/SafePathTests.coffee | 59 ++++++++++++++++++- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Project/SafePath.coffee b/services/web/app/coffee/Features/Project/SafePath.coffee index a77f83ce2b..8684817df3 100644 --- a/services/web/app/coffee/Features/Project/SafePath.coffee +++ b/services/web/app/coffee/Features/Project/SafePath.coffee @@ -67,6 +67,15 @@ load = () -> !BADFILE_RX.test(filename) && !BLOCKEDFILE_RX.test(filename) + isCleanPath: (path) -> + elements = path.split('/') + return false if elements[elements.length - 1].length == 0 + + for element in elements + return false if element.length > 0 && !SafePath.isCleanFilename element + + return true + isAllowedLength: (pathname) -> return pathname.length > 0 && pathname.length <= MAX_PATH diff --git a/services/web/test/unit/coffee/Project/SafePathTests.coffee b/services/web/test/unit/coffee/Project/SafePathTests.coffee index 6e0c55a5bc..afea03fdea 100644 --- a/services/web/test/unit/coffee/Project/SafePathTests.coffee +++ b/services/web/test/unit/coffee/Project/SafePathTests.coffee @@ -83,6 +83,59 @@ describe 'SafePath', -> result = @SafePath.isCleanFilename 'foo\\bar' result.should.equal false + describe 'isCleanPath', -> + it 'should accept a valid filename "main.tex"', -> + result = @SafePath.isCleanPath 'main.tex' + result.should.equal true + + it 'should accept a valid path "foo/main.tex"', -> + result = @SafePath.isCleanPath 'foo/main.tex' + result.should.equal true + + it 'should accept empty path elements', -> + result = @SafePath.isCleanPath 'foo//main.tex' + result.should.equal true + + it 'should not accept an empty filename', -> + result = @SafePath.isCleanPath 'foo/bar/' + result.should.equal false + + it 'should accept a path that starts with a slash', -> + result = @SafePath.isCleanPath '/etc/passwd' + result.should.equal true + + it 'should not accept a path that has an asterisk as the 0th element', -> + result = @SafePath.isCleanPath '*/foo/bar' + result.should.equal false + + it 'should not accept a path that has an asterisk as a middle element', -> + result = @SafePath.isCleanPath 'foo/*/bar' + result.should.equal false + + it 'should not accept a path that has an asterisk as the filename', -> + result = @SafePath.isCleanPath 'foo/bar/*' + result.should.equal false + + it 'should not accept a path that contains an asterisk in the 0th element', -> + result = @SafePath.isCleanPath 'f*o/bar/baz' + result.should.equal false + + it 'should not accept a path that contains an asterisk in a middle element', -> + result = @SafePath.isCleanPath 'foo/b*r/baz' + result.should.equal false + + it 'should not accept a path that contains an asterisk in the filename', -> + result = @SafePath.isCleanPath 'foo/bar/b*z' + result.should.equal false + + it 'should not accept multiple problematic elements', -> + result = @SafePath.isCleanPath 'f*o/b*r/b*z' + result.should.equal false + + it 'should not accept a problematic path with an empty element', -> + result = @SafePath.isCleanPath 'foo//*/bar' + result.should.equal false + describe 'isAllowedLength', -> it 'should accept a valid path "main.tex"', -> result = @SafePath.isAllowedLength 'main.tex' @@ -96,7 +149,7 @@ describe 'SafePath', -> it 'should not accept an empty path', -> result = @SafePath.isAllowedLength '' result.should.equal false - + describe 'clean', -> it 'should not modify a valid filename', -> result = @SafePath.clean 'main.tex' @@ -105,7 +158,7 @@ describe 'SafePath', -> it 'should replace invalid characters with _', -> result = @SafePath.clean 'foo/bar*/main.tex' result.should.equal 'foo_bar__main.tex' - + it 'should replace "." with "_"', -> result = @SafePath.clean '.' result.should.equal '_' @@ -133,7 +186,7 @@ describe 'SafePath', -> it 'should prefix javascript property names with @', -> result = @SafePath.clean 'prototype' result.should.equal '@prototype' - + it 'should prefix javascript property names in the prototype with @', -> result = @SafePath.clean 'hasOwnProperty' result.should.equal '@hasOwnProperty' From d9c98aa45ea1d4c20779289e18fcc440f499bd71 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Mon, 8 Oct 2018 15:01:18 +0100 Subject: [PATCH 29/43] Add default Twitter and OG images --- services/web/app/views/_metadata.pug | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/services/web/app/views/_metadata.pug b/services/web/app/views/_metadata.pug index 253757b6a8..15f08b038b 100644 --- a/services/web/app/views/_metadata.pug +++ b/services/web/app/views/_metadata.pug @@ -51,8 +51,15 @@ meta(name="twitter:card", content=metadata.twitterCardType ? metadata.twitterCar -if (metadata && metadata.twitterDescription) meta(itemprop="twitter:description", content=metadata.twitterDescription) -if (metadata && metadata.twitterImage) + //- from the CMS meta(itemprop="image", name="twitter:image", content=metadata.twitterImage.fields.file.url) meta(itemprop="image", name="twitter:image:alt", content=metadata.twitterImage.fields.title) +-else if (settings.overleaf) + //- the default image for Overleaf + meta(itemprop="image", name="twitter:image", content=buildImgPath('ol-brand/overleaf_og_logo.png')) +-else + //- the default image for ShareLaTeX + meta(itemprop="image", name="twitter:image", content='/touch-icon-192x192.png') //- Open Graph //- to do - add og:url @@ -61,7 +68,14 @@ meta(name="twitter:card", content=metadata.twitterCardType ? metadata.twitterCar -if (metadata && metadata.openGraphDescription) meta(itemprop="description", name="og:description", content=metadata.openGraphDescription) -if (metadata && metadata.openGraphImage) + //- from the CMS meta(itemprop="image", name="og:image", content=metadata.openGraphImage.fields.file.url) +-else if (settings.overleaf) + //- the default image for Overleaf + meta(itemprop="image", name="og:image", content=buildImgPath('ol-brand/overleaf_og_logo.png')) +-else + //- the default image for ShareLaTeX + meta(itemprop="image", name="og:image", content='/touch-icon-192x192.png') -if (metadata && metadata.openGraphType) meta(name="og:type", metadata.openGraphType) -else From 56dcbefb5b4dad0e2aaa6e4ab8311d8acdba004b Mon Sep 17 00:00:00 2001 From: Simon Detheridge Date: Sat, 6 Oct 2018 18:12:33 +0100 Subject: [PATCH 30/43] Check for safe paths in all ProjectEntityHandler methods Some import mechanisms (for example, Github project import) call methods such as 'upsert*' directly, bypassing existing filename checks. Added checks to all methods in ProjectEntityHandler that can create or rename a file. bug: overleaf/sharelatex#908 Signed-off-by: Simon Detheridge --- .../Project/ProjectEntityUpdateHandler.coffee | 17 + .../coffee/Features/Project/SafePath.coffee | 9 +- .../ProjectEntityUpdateHandlerTests.coffee | 594 ++++++++++++------ 3 files changed, 423 insertions(+), 197 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee index b7be80cb47..079065a948 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee @@ -129,6 +129,8 @@ module.exports = ProjectEntityUpdateHandler = self = Project.update {_id:project_id}, {$unset: {rootDoc_id: true}}, {}, callback addDoc: wrapWithLock (project_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=> + if not SafePath.isCleanFilename docName + return callback new Errors.InvalidNameError("invalid element name") self.addDocWithoutUpdatingHistory.withoutLock project_id, folder_id, docName, docLines, userId, (error, doc, folder_id, path, project) -> return callback(error) if error? projectHistoryId = project.overleaf?.history?.id @@ -166,6 +168,8 @@ module.exports = ProjectEntityUpdateHandler = self = addFile: wrapWithLock beforeLock: (next) -> (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback) -> + if not SafePath.isCleanFilename fileName + return callback new Errors.InvalidNameError("invalid element name") ProjectEntityUpdateHandler._uploadFile project_id, folder_id, fileName, fsPath, linkedFileData, userId, (error, fileRef, fileStoreUrl) -> return callback(error) if error? next(project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback) @@ -241,6 +245,8 @@ module.exports = ProjectEntityUpdateHandler = self = # the history unless you are making sure it is updated in some other way. beforeLock: (next) -> (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback) -> + if not SafePath.isCleanFilename fileName + return callback(new Errors.InvalidNameError("invalid element name")) ProjectEntityUpdateHandler._uploadFile project_id, folder_id, fileName, fsPath, linkedFileData, userId, (error, fileRef, fileStoreUrl) -> return callback(error) if error? next(project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback) @@ -250,6 +256,8 @@ module.exports = ProjectEntityUpdateHandler = self = callback(null, fileRef, folder_id, result?.path?.fileSystem, fileStoreUrl) upsertDoc: wrapWithLock (project_id, folder_id, docName, docLines, source, userId, callback = (err, doc, folder_id, isNewDoc)->)-> + if not SafePath.isCleanFilename docName + return callback new Errors.InvalidNameError("invalid element name") ProjectLocator.findElement project_id: project_id, element_id: folder_id, type: "folder", (error, folder) -> return callback(error) if error? return callback(new Error("Couldn't find folder")) if !folder? @@ -272,6 +280,8 @@ module.exports = ProjectEntityUpdateHandler = self = upsertFile: wrapWithLock beforeLock: (next) -> (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback)-> + if not SafePath.isCleanFilename fileName + return callback new Errors.InvalidNameError("invalid element name") # create a new file fileRef = new File( name: fileName @@ -301,6 +311,8 @@ module.exports = ProjectEntityUpdateHandler = self = callback null, newFileRef, !existingFile?, existingFile upsertDocWithPath: wrapWithLock (project_id, elementPath, docLines, source, userId, callback) -> + if not SafePath.isCleanPath elementPath + return callback new Errors.InvalidNameError("invalid element name") docName = path.basename(elementPath) folderPath = path.dirname(elementPath) self.mkdirp.withoutLock project_id, folderPath, (err, newFolders, folder) -> @@ -312,6 +324,8 @@ module.exports = ProjectEntityUpdateHandler = self = upsertFileWithPath: wrapWithLock beforeLock: (next) -> (project_id, elementPath, fsPath, linkedFileData, userId, callback)-> + if not SafePath.isCleanPath elementPath + return callback new Errors.InvalidNameError("invalid element name") fileName = path.basename(elementPath) folderPath = path.dirname(elementPath) # create a new file @@ -351,6 +365,9 @@ module.exports = ProjectEntityUpdateHandler = self = self.deleteEntity.withoutLock project_id, element._id, type, userId, callback mkdirp: wrapWithLock (project_id, path, callback = (err, newlyCreatedFolders, lastFolderInPath)->)-> + for folder in path.split('/') + if folder.length > 0 and not SafePath.isCleanFilename folder + return callback new Errors.InvalidNameError("invalid element name") ProjectEntityMongoUpdateHandler.mkdirp project_id, path, callback addFolder: wrapWithLock (project_id, parentFolder_id, folderName, callback) -> diff --git a/services/web/app/coffee/Features/Project/SafePath.coffee b/services/web/app/coffee/Features/Project/SafePath.coffee index 8684817df3..4e5a288ea9 100644 --- a/services/web/app/coffee/Features/Project/SafePath.coffee +++ b/services/web/app/coffee/Features/Project/SafePath.coffee @@ -52,6 +52,7 @@ load = () -> MAX_PATH = 1024 # Maximum path length, in characters. This is fairly arbitrary. SafePath = + # convert any invalid characters to underscores in the given filename clean: (filename) -> filename = filename.replace BADCHAR_RX, '_' # for BADFILE_RX replace any matches with an equal number of underscores @@ -61,15 +62,21 @@ load = () -> filename = filename.replace BLOCKEDFILE_RX, "@$1" return filename + # returns whether the filename is 'clean' (does not contain any invalid + # characters or reserved words) isCleanFilename: (filename) -> return SafePath.isAllowedLength(filename) && !BADCHAR_RX.test(filename) && !BADFILE_RX.test(filename) && !BLOCKEDFILE_RX.test(filename) + # returns whether a full path is 'clean' - e.g. is a full or relative path + # that points to a file, and each element passes the rules in 'isCleanFilename' isCleanPath: (path) -> elements = path.split('/') - return false if elements[elements.length - 1].length == 0 + + lastElementIsEmpty = elements[elements.length - 1].length == 0 + return false if lastElementIsEmpty for element in elements return false if element.length > 0 && !SafePath.isCleanFilename element diff --git a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee index 3de4546e6d..5c31113552 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee @@ -289,71 +289,101 @@ describe 'ProjectEntityUpdateHandler', -> .should.equal true describe 'addDoc', -> - beforeEach -> - @path = "/path/to/doc" + describe 'adding a doc', -> + beforeEach -> + @path = "/path/to/doc" - @newDoc = _id: doc_id - @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory = - withoutLock: sinon.stub().yields(null, @newDoc, folder_id, @path, @project) - @ProjectEntityUpdateHandler.addDoc project_id, folder_id, @docName, @docLines, userId, @callback + @newDoc = _id: doc_id + @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory = + withoutLock: sinon.stub().yields(null, @newDoc, folder_id, @path, @project) + @ProjectEntityUpdateHandler.addDoc project_id, folder_id, @docName, @docLines, userId, @callback - it "creates the doc without history", () -> - @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory.withoutLock - .calledWith(project_id, folder_id, @docName, @docLines, userId) - .should.equal true + it "creates the doc without history", () -> + @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory.withoutLock + .calledWith(project_id, folder_id, @docName, @docLines, userId) + .should.equal true - it "sends the change in project structure to the doc updater", () -> - newDocs = [ - doc: @newDoc - path: @path - docLines: @docLines.join('\n') - ] - @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, projectHistoryId, userId, {newDocs}) - .should.equal true + it "sends the change in project structure to the doc updater", () -> + newDocs = [ + doc: @newDoc + path: @path + docLines: @docLines.join('\n') + ] + @DocumentUpdaterHandler.updateProjectStructure + .calledWith(project_id, projectHistoryId, userId, {newDocs}) + .should.equal true + + describe 'adding a doc with an invalid name', -> + beforeEach -> + @path = "/path/to/doc" + + @newDoc = _id: doc_id + @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory = + withoutLock: sinon.stub().yields(null, @newDoc, folder_id, @path, @project) + @ProjectEntityUpdateHandler.addDoc project_id, folder_id, "*" + @docName, @docLines, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true describe 'addFile', -> - beforeEach -> - @path = "/path/to/file" + describe 'adding a file', -> + beforeEach -> + @path = "/path/to/file" - @newFile = {_id: file_id, rev: 0, name: @fileName, linkedFileData: @linkedFileData} - @TpdsUpdateSender.addFile = sinon.stub().yields() - @ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project) - @ProjectEntityUpdateHandler.addFile project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId, @callback + @newFile = {_id: file_id, rev: 0, name: @fileName, linkedFileData: @linkedFileData} + @TpdsUpdateSender.addFile = sinon.stub().yields() + @ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project) + @ProjectEntityUpdateHandler.addFile project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId, @callback - it "updates the file in the filestore", () -> - @FileStoreHandler.uploadFileFromDisk - .calledWith(project_id, file_id, @fileSystemPath) - .should.equal true + it "updates the file in the filestore", () -> + @FileStoreHandler.uploadFileFromDisk + .calledWith(project_id, file_id, @fileSystemPath) + .should.equal true - it "updates the file in mongo", () -> - fileMatcher = sinon.match (file) => - file.name == @fileName + it "updates the file in mongo", () -> + fileMatcher = sinon.match (file) => + file.name == @fileName - @ProjectEntityMongoUpdateHandler.addFile - .calledWithMatch(project_id, folder_id, fileMatcher) - .should.equal true + @ProjectEntityMongoUpdateHandler.addFile + .calledWithMatch(project_id, folder_id, fileMatcher) + .should.equal true - it "notifies the tpds", () -> - @TpdsUpdateSender.addFile - .calledWith({ - project_id: project_id - project_name: @project.name - file_id: file_id - rev: 0 + it "notifies the tpds", () -> + @TpdsUpdateSender.addFile + .calledWith({ + project_id: project_id + project_name: @project.name + file_id: file_id + rev: 0 + path: @path + }) + .should.equal true + + it "sends the change in project structure to the doc updater", () -> + newFiles = [ + file: @newFile path: @path - }) - .should.equal true + url: @fileUrl + ] + @DocumentUpdaterHandler.updateProjectStructure + .calledWith(project_id, projectHistoryId, userId, {newFiles}) + .should.equal true - it "sends the change in project structure to the doc updater", () -> - newFiles = [ - file: @newFile - path: @path - url: @fileUrl - ] - @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, projectHistoryId, userId, {newFiles}) - .should.equal true + describe 'adding a file with an invalid name', -> + beforeEach -> + @path = "/path/to/file" + + @newFile = {_id: file_id, rev: 0, name: @fileName, linkedFileData: @linkedFileData} + @TpdsUpdateSender.addFile = sinon.stub().yields() + @ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project) + @ProjectEntityUpdateHandler.addFile project_id, folder_id, "*" + @fileName, @fileSystemPath, @linkedFileData, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true describe 'replaceFile', -> beforeEach -> @@ -404,83 +434,116 @@ describe 'ProjectEntityUpdateHandler', -> .should.equal true describe 'addDocWithoutUpdatingHistory', -> - beforeEach -> - @path = "/path/to/doc" + describe 'adding a doc', -> + beforeEach -> + @path = "/path/to/doc" - @project = _id: project_id, name: 'some project' + @project = _id: project_id, name: 'some project' - @TpdsUpdateSender.addDoc = sinon.stub().yields() - @DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5) - @ProjectEntityMongoUpdateHandler.addDoc = sinon.stub().yields(null, {path: fileSystem: @path}, @project) - @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory project_id, folder_id, @docName, @docLines, userId, @callback + @TpdsUpdateSender.addDoc = sinon.stub().yields() + @DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5) + @ProjectEntityMongoUpdateHandler.addDoc = sinon.stub().yields(null, {path: fileSystem: @path}, @project) + @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory project_id, folder_id, @docName, @docLines, userId, @callback - it "updates the doc in the docstore", () -> - @DocstoreManager.updateDoc - .calledWith(project_id, doc_id, @docLines, 0, {}) - .should.equal true + it "updates the doc in the docstore", () -> + @DocstoreManager.updateDoc + .calledWith(project_id, doc_id, @docLines, 0, {}) + .should.equal true - it "updates the doc in mongo", () -> - docMatcher = sinon.match (doc) => - doc.name == @docName + it "updates the doc in mongo", () -> + docMatcher = sinon.match (doc) => + doc.name == @docName - @ProjectEntityMongoUpdateHandler.addDoc - .calledWithMatch(project_id, folder_id, docMatcher) - .should.equal true + @ProjectEntityMongoUpdateHandler.addDoc + .calledWithMatch(project_id, folder_id, docMatcher) + .should.equal true - it "notifies the tpds", () -> - @TpdsUpdateSender.addDoc - .calledWith({ - project_id: project_id - project_name: @project.name - doc_id: doc_id - rev: 0 - path: @path - }) - .should.equal true + it "notifies the tpds", () -> + @TpdsUpdateSender.addDoc + .calledWith({ + project_id: project_id + project_name: @project.name + doc_id: doc_id + rev: 0 + path: @path + }) + .should.equal true - it "should not should send the change in project structure to the doc updater", () -> - @DocumentUpdaterHandler.updateProjectStructure - .called - .should.equal false + it "should not should send the change in project structure to the doc updater", () -> + @DocumentUpdaterHandler.updateProjectStructure + .called + .should.equal false + + describe 'adding a doc with an invalid name', -> + beforeEach -> + @path = "/path/to/doc" + + @project = _id: project_id, name: 'some project' + + @TpdsUpdateSender.addDoc = sinon.stub().yields() + @DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5) + @ProjectEntityMongoUpdateHandler.addDoc = sinon.stub().yields(null, {path: fileSystem: @path}, @project) + @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory project_id, folder_id, "*" + @docName, @docLines, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true describe 'addFileWithoutUpdatingHistory', -> - beforeEach -> - @path = "/path/to/file" + describe 'adding a file', -> + beforeEach -> + @path = "/path/to/file" - @project = _id: project_id, name: 'some project' + @project = _id: project_id, name: 'some project' - @TpdsUpdateSender.addFile = sinon.stub().yields() - @ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project) - @ProjectEntityUpdateHandler.addFileWithoutUpdatingHistory project_id, folder_id, @fileName, @fileSystemPath, userId, @callback + @TpdsUpdateSender.addFile = sinon.stub().yields() + @ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project) + @ProjectEntityUpdateHandler.addFileWithoutUpdatingHistory project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId, @callback - it "updates the file in the filestore", () -> - @FileStoreHandler.uploadFileFromDisk - .calledWith(project_id, file_id, @fileSystemPath) - .should.equal true + it "updates the file in the filestore", () -> + @FileStoreHandler.uploadFileFromDisk + .calledWith(project_id, file_id, @fileSystemPath) + .should.equal true - it "updates the file in mongo", () -> - fileMatcher = sinon.match (file) => - file.name == @fileName + it "updates the file in mongo", () -> + fileMatcher = sinon.match (file) => + file.name == @fileName - @ProjectEntityMongoUpdateHandler.addFile - .calledWithMatch(project_id, folder_id, fileMatcher) - .should.equal true + @ProjectEntityMongoUpdateHandler.addFile + .calledWithMatch(project_id, folder_id, fileMatcher) + .should.equal true - it "notifies the tpds", () -> - @TpdsUpdateSender.addFile - .calledWith({ - project_id: project_id - project_name: @project.name - file_id: file_id - rev: 0 - path: @path - }) - .should.equal true + it "notifies the tpds", () -> + @TpdsUpdateSender.addFile + .calledWith({ + project_id: project_id + project_name: @project.name + file_id: file_id + rev: 0 + path: @path + }) + .should.equal true - it "should not should send the change in project structure to the doc updater", () -> - @DocumentUpdaterHandler.updateProjectStructure - .called - .should.equal false + it "should not should send the change in project structure to the doc updater", () -> + @DocumentUpdaterHandler.updateProjectStructure + .called + .should.equal false + + describe 'adding a file with an invalid name', -> + beforeEach -> + @path = "/path/to/file" + + @project = _id: project_id, name: 'some project' + + @TpdsUpdateSender.addFile = sinon.stub().yields() + @ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project) + @ProjectEntityUpdateHandler.addFileWithoutUpdatingHistory project_id, folder_id, "*" + @fileName, @fileSystemPath, @linkedFileData, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true describe 'upsertDoc', -> describe 'upserting into an invalid folder', -> @@ -543,6 +606,20 @@ describe 'ProjectEntityUpdateHandler', -> it 'returns the doc', -> @callback.calledWith(null, @newDoc, true) + describe 'upserting a new doc with an invalid name', -> + beforeEach -> + @folder = _id: folder_id, docs: [] + @newDoc = _id: doc_id + @ProjectLocator.findElement = sinon.stub().yields(null, @folder) + @ProjectEntityUpdateHandler.addDoc = withoutLock: sinon.stub().yields(null, @newDoc) + + @ProjectEntityUpdateHandler.upsertDoc project_id, folder_id, "*" + @docName, @docLines, @source, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true + describe 'upsertFile', -> describe 'upserting into an invalid folder', -> beforeEach -> @@ -593,63 +670,155 @@ describe 'ProjectEntityUpdateHandler', -> it 'returns the file', -> @callback.calledWith(null, @newFile, true) + describe 'upserting a new file with an invalid name', -> + beforeEach -> + @folder = _id: folder_id, fileRefs: [] + @newFile = _id: file_id + @ProjectLocator.findElement = sinon.stub().yields(null, @folder) + @ProjectEntityUpdateHandler.addFile = mainTask: sinon.stub().yields(null, @newFile) + + @ProjectEntityUpdateHandler.upsertFile project_id, folder_id, '*' + @fileName, @fileSystemPath, @linkedFileData, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true + describe 'upsertDocWithPath', -> - beforeEach -> - @path = "/folder/doc.tex" - @newFolders = [ 'mock-a', 'mock-b' ] - @folder = _id: folder_id - @doc = _id: doc_id - @isNewDoc = true - @ProjectEntityUpdateHandler.mkdirp = - withoutLock: sinon.stub().yields(null, @newFolders, @folder) - @ProjectEntityUpdateHandler.upsertDoc = - withoutLock: sinon.stub().yields(null, @doc, @isNewDoc) + describe 'upserting a doc', -> + beforeEach -> + @path = "/folder/doc.tex" + @newFolders = [ 'mock-a', 'mock-b' ] + @folder = _id: folder_id + @doc = _id: doc_id + @isNewDoc = true + @ProjectEntityUpdateHandler.mkdirp = + withoutLock: sinon.stub().yields(null, @newFolders, @folder) + @ProjectEntityUpdateHandler.upsertDoc = + withoutLock: sinon.stub().yields(null, @doc, @isNewDoc) - @ProjectEntityUpdateHandler.upsertDocWithPath project_id, @path, @docLines, @source, userId, @callback + @ProjectEntityUpdateHandler.upsertDocWithPath project_id, @path, @docLines, @source, userId, @callback - it 'creates any necessary folders', -> - @ProjectEntityUpdateHandler.mkdirp.withoutLock - .calledWith(project_id, '/folder') - .should.equal true + it 'creates any necessary folders', -> + @ProjectEntityUpdateHandler.mkdirp.withoutLock + .calledWith(project_id, '/folder') + .should.equal true - it 'upserts the doc', -> - @ProjectEntityUpdateHandler.upsertDoc.withoutLock - .calledWith(project_id, @folder._id, 'doc.tex', @docLines, @source, userId) - .should.equal true + it 'upserts the doc', -> + @ProjectEntityUpdateHandler.upsertDoc.withoutLock + .calledWith(project_id, @folder._id, 'doc.tex', @docLines, @source, userId) + .should.equal true - it 'calls the callback', -> - @callback - .calledWith(null, @doc, @isNewDoc, @newFolders, @folder) - .should.equal true + it 'calls the callback', -> + @callback + .calledWith(null, @doc, @isNewDoc, @newFolders, @folder) + .should.equal true + + describe 'upserting a doc with an invalid path', -> + beforeEach -> + @path = "/*folder/doc.tex" + @newFolders = [ 'mock-a', 'mock-b' ] + @folder = _id: folder_id + @doc = _id: doc_id + @isNewDoc = true + @ProjectEntityUpdateHandler.mkdirp = + withoutLock: sinon.stub().yields(null, @newFolders, @folder) + @ProjectEntityUpdateHandler.upsertDoc = + withoutLock: sinon.stub().yields(null, @doc, @isNewDoc) + + @ProjectEntityUpdateHandler.upsertDocWithPath project_id, @path, @docLines, @source, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true + + describe 'upserting a doc with an invalid name', -> + beforeEach -> + @path = "/folder/*doc.tex" + @newFolders = [ 'mock-a', 'mock-b' ] + @folder = _id: folder_id + @doc = _id: doc_id + @isNewDoc = true + @ProjectEntityUpdateHandler.mkdirp = + withoutLock: sinon.stub().yields(null, @newFolders, @folder) + @ProjectEntityUpdateHandler.upsertDoc = + withoutLock: sinon.stub().yields(null, @doc, @isNewDoc) + + @ProjectEntityUpdateHandler.upsertDocWithPath project_id, @path, @docLines, @source, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true describe 'upsertFileWithPath', -> - beforeEach -> - @path = "/folder/file.png" - @newFolders = [ 'mock-a', 'mock-b' ] - @folder = _id: folder_id - @file = _id: file_id - @isNewFile = true - @ProjectEntityUpdateHandler.mkdirp = - withoutLock: sinon.stub().yields(null, @newFolders, @folder) - @ProjectEntityUpdateHandler.upsertFile = - mainTask: sinon.stub().yields(null, @file, @isNewFile) + describe 'upserting a file', -> + beforeEach -> + @path = "/folder/file.png" + @newFolders = [ 'mock-a', 'mock-b' ] + @folder = _id: folder_id + @file = _id: file_id + @isNewFile = true + @ProjectEntityUpdateHandler.mkdirp = + withoutLock: sinon.stub().yields(null, @newFolders, @folder) + @ProjectEntityUpdateHandler.upsertFile = + mainTask: sinon.stub().yields(null, @file, @isNewFile) - @ProjectEntityUpdateHandler.upsertFileWithPath project_id, @path, @fileSystemPath, @linkedFileData, userId, @callback + @ProjectEntityUpdateHandler.upsertFileWithPath project_id, @path, @fileSystemPath, @linkedFileData, userId, @callback - it 'creates any necessary folders', -> - @ProjectEntityUpdateHandler.mkdirp.withoutLock - .calledWith(project_id, '/folder') - .should.equal true + it 'creates any necessary folders', -> + @ProjectEntityUpdateHandler.mkdirp.withoutLock + .calledWith(project_id, '/folder') + .should.equal true - it 'upserts the file', -> - @ProjectEntityUpdateHandler.upsertFile.mainTask - .calledWith(project_id, @folder._id, 'file.png', @fileSystemPath, @linkedFileData, userId) - .should.equal true + it 'upserts the file', -> + @ProjectEntityUpdateHandler.upsertFile.mainTask + .calledWith(project_id, @folder._id, 'file.png', @fileSystemPath, @linkedFileData, userId) + .should.equal true - it 'calls the callback', -> - @callback - .calledWith(null, @file, @isNewFile, undefined, @newFolders, @folder) - .should.equal true + it 'calls the callback', -> + @callback + .calledWith(null, @file, @isNewFile, undefined, @newFolders, @folder) + .should.equal true + + describe 'upserting a file with an invalid path', -> + beforeEach -> + @path = "/*folder/file.png" + @newFolders = [ 'mock-a', 'mock-b' ] + @folder = _id: folder_id + @file = _id: file_id + @isNewFile = true + @ProjectEntityUpdateHandler.mkdirp = + withoutLock: sinon.stub().yields(null, @newFolders, @folder) + @ProjectEntityUpdateHandler.upsertFile = + mainTask: sinon.stub().yields(null, @file, @isNewFile) + + @ProjectEntityUpdateHandler.upsertFileWithPath project_id, @path, @fileSystemPath, @linkedFileData, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true + + describe 'upserting a file with an invalid name', -> + beforeEach -> + @path = "/folder/*file.png" + @newFolders = [ 'mock-a', 'mock-b' ] + @folder = _id: folder_id + @file = _id: file_id + @isNewFile = true + @ProjectEntityUpdateHandler.mkdirp = + withoutLock: sinon.stub().yields(null, @newFolders, @folder) + @ProjectEntityUpdateHandler.upsertFile = + mainTask: sinon.stub().yields(null, @file, @isNewFile) + + @ProjectEntityUpdateHandler.upsertFileWithPath project_id, @path, @fileSystemPath, @linkedFileData, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true describe 'deleteEntity', -> beforeEach -> @@ -721,16 +890,29 @@ describe 'ProjectEntityUpdateHandler', -> .should.equal true describe 'addFolder', -> - beforeEach -> - @parentFolder_id = '123asdf' - @folderName = 'new-folder' - @ProjectEntityMongoUpdateHandler.addFolder = sinon.stub().yields() - @ProjectEntityUpdateHandler.addFolder project_id, @parentFolder_id, @folderName, @callback + describe 'adding a folder', -> + beforeEach -> + @parentFolder_id = '123asdf' + @folderName = 'new-folder' + @ProjectEntityMongoUpdateHandler.addFolder = sinon.stub().yields() + @ProjectEntityUpdateHandler.addFolder project_id, @parentFolder_id, @folderName, @callback - it 'calls ProjectEntityMongoUpdateHandler', -> - @ProjectEntityMongoUpdateHandler.addFolder - .calledWith(project_id, @parentFolder_id, @folderName) - .should.equal true + it 'calls ProjectEntityMongoUpdateHandler', -> + @ProjectEntityMongoUpdateHandler.addFolder + .calledWith(project_id, @parentFolder_id, @folderName) + .should.equal true + + describe 'adding a folder with an invalid name', -> + beforeEach -> + @parentFolder_id = '123asdf' + @folderName = '*new-folder' + @ProjectEntityMongoUpdateHandler.addFolder = sinon.stub().yields() + @ProjectEntityUpdateHandler.addFolder project_id, @parentFolder_id, @folderName, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true describe 'moveEntity', -> beforeEach -> @@ -763,35 +945,57 @@ describe 'ProjectEntityUpdateHandler', -> .should.equal true describe "renameEntity", -> - beforeEach -> - @project_name = 'project name' - @startPath = '/folder/a.tex' - @endPath = '/folder/b.tex' - @rev = 2 - @changes = newDocs: ['old-doc'], newFiles: ['old-file'] - @newDocName = 'b.tex' - @ProjectEntityMongoUpdateHandler.renameEntity = sinon.stub().yields( - null, @project, @startPath, @endPath, @rev, @changes - ) - @TpdsUpdateSender.moveEntity = sinon.stub() - @DocumentUpdaterHandler.updateProjectStructure = sinon.stub() + describe 'renaming an entity', -> + beforeEach -> + @project_name = 'project name' + @startPath = '/folder/a.tex' + @endPath = '/folder/b.tex' + @rev = 2 + @changes = newDocs: ['old-doc'], newFiles: ['old-file'] + @newDocName = 'b.tex' + @ProjectEntityMongoUpdateHandler.renameEntity = sinon.stub().yields( + null, @project, @startPath, @endPath, @rev, @changes + ) + @TpdsUpdateSender.moveEntity = sinon.stub() + @DocumentUpdaterHandler.updateProjectStructure = sinon.stub() - @ProjectEntityUpdateHandler.renameEntity project_id, doc_id, 'doc', @newDocName, userId, @callback + @ProjectEntityUpdateHandler.renameEntity project_id, doc_id, 'doc', @newDocName, userId, @callback - it 'moves the entity in mongo', -> - @ProjectEntityMongoUpdateHandler.renameEntity - .calledWith(project_id, doc_id, 'doc', @newDocName) - .should.equal true + it 'moves the entity in mongo', -> + @ProjectEntityMongoUpdateHandler.renameEntity + .calledWith(project_id, doc_id, 'doc', @newDocName) + .should.equal true - it 'notifies tpds', -> - @TpdsUpdateSender.moveEntity - .calledWith({project_id, @project_name, @startPath, @endPath, @rev}) - .should.equal true + it 'notifies tpds', -> + @TpdsUpdateSender.moveEntity + .calledWith({project_id, @project_name, @startPath, @endPath, @rev}) + .should.equal true - it 'sends the changes in project structure to the doc updater', -> - @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, projectHistoryId, userId, @changes, @callback) - .should.equal true + it 'sends the changes in project structure to the doc updater', -> + @DocumentUpdaterHandler.updateProjectStructure + .calledWith(project_id, projectHistoryId, userId, @changes, @callback) + .should.equal true + + describe 'renaming an entity to an invalid name', -> + beforeEach -> + @project_name = 'project name' + @startPath = '/folder/a.tex' + @endPath = '/folder/b.tex' + @rev = 2 + @changes = newDocs: ['old-doc'], newFiles: ['old-file'] + @newDocName = '*b.tex' + @ProjectEntityMongoUpdateHandler.renameEntity = sinon.stub().yields( + null, @project, @startPath, @endPath, @rev, @changes + ) + @TpdsUpdateSender.moveEntity = sinon.stub() + @DocumentUpdaterHandler.updateProjectStructure = sinon.stub() + + @ProjectEntityUpdateHandler.renameEntity project_id, doc_id, 'doc', @newDocName, userId, @callback + + it 'returns an error', -> + errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) + @callback.calledWithMatch(errorMatcher) + .should.equal true describe "resyncProjectHistory", -> describe "a deleted project", -> @@ -998,5 +1202,3 @@ describe 'ProjectEntityUpdateHandler', -> it "should call the callback", -> @callback.called.should.equal true - - From cf47fc0b1c1d05788b436bde123d8bb7d3e1f0ce Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Mon, 8 Oct 2018 15:51:24 +0100 Subject: [PATCH 31/43] fix link to link sharing help page --- services/web/app/views/project/editor/share.pug | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/app/views/project/editor/share.pug b/services/web/app/views/project/editor/share.pug index 17da2df67a..ac238d7ad5 100644 --- a/services/web/app/views/project/editor/share.pug +++ b/services/web/app/views/project/editor/share.pug @@ -19,7 +19,7 @@ script(type='text/ng-template', id='shareProjectModalTemplate') ) #{translate('turn_on_link_sharing')} span    a( - href="/learn/Kb/what_is_link_sharing" + href="/learn/how-to/What_is_Link_Sharing%3F" target="_blank" ) i.fa.fa-question-circle( @@ -38,7 +38,7 @@ script(type='text/ng-template', id='shareProjectModalTemplate') ) #{translate('turn_off_link_sharing')} span    a( - href="/learn/Kb/what_is_link_sharing" + href="/learn/how-to/What_is_Link_Sharing%3F" target="_blank" ) i.fa.fa-question-circle( From 849c5253c732cb4fb8afe49f891a87bec6ceb5d6 Mon Sep 17 00:00:00 2001 From: Chrystal Griffiths Date: Mon, 8 Oct 2018 17:50:10 +0100 Subject: [PATCH 32/43] Avoid duplicating code --- services/web/app/views/project/editor/pdf.pug | 4 ++-- services/web/public/coffee/ide.coffee | 17 +++++++++++++++++ .../ide/pdf/controllers/PdfController.coffee | 16 ---------------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/services/web/app/views/project/editor/pdf.pug b/services/web/app/views/project/editor/pdf.pug index 44410a44a8..f0d1a11666 100644 --- a/services/web/app/views/project/editor/pdf.pug +++ b/services/web/app/views/project/editor/pdf.pug @@ -106,7 +106,7 @@ div.full-size.pdf(ng-controller="PdfController") | #{translate("code_check_failed")} a( href, - ng-click="switchToFlatLayout()" + ng-click="switchToFlatLayout('pdf')" ng-show="ui.pdfLayout == 'sideBySide'" tooltip=translate('full_screen') tooltip-placement="bottom" @@ -116,7 +116,7 @@ div.full-size.pdf(ng-controller="PdfController") i.full-screen a( href, - ng-click="switchToSideBySideLayout()" + ng-click="switchToSideBySideLayout('editor')" ng-show="ui.pdfLayout == 'flat'" tooltip=translate('split_screen') tooltip-placement="bottom" diff --git a/services/web/public/coffee/ide.coffee b/services/web/public/coffee/ide.coffee index 1b9eda57d6..12f6dba2ce 100644 --- a/services/web/public/coffee/ide.coffee +++ b/services/web/public/coffee/ide.coffee @@ -190,6 +190,23 @@ define [ ide.localStorage = localStorage ide.browserIsSafari = false + + $scope.switchToFlatLayout = (view) -> + $scope.ui.pdfLayout = 'flat' + $scope.ui.view = view + ide.localStorage "pdf.layout", "flat" + + $scope.switchToSideBySideLayout = (view) -> + $scope.ui.pdfLayout = 'sideBySide' + $scope.ui.view = view + localStorage "pdf.layout", "split" + + if pdfLayout = localStorage("pdf.layout") + $scope.switchToSideBySideLayout() if pdfLayout == "split" + $scope.switchToFlatLayout() if pdfLayout == "flat" + else + $scope.switchToSideBySideLayout() + try userAgent = navigator.userAgent ide.browserIsSafari = ( diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 001178c5dd..a9e99f3d84 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -588,22 +588,6 @@ define [ {doc, line} = data ide.editorManager.openDoc(doc, gotoLine: line) - $scope.switchToFlatLayout = () -> - $scope.ui.pdfLayout = 'flat' - $scope.ui.view = 'pdf' - ide.localStorage "pdf.layout", "flat" - - $scope.switchToSideBySideLayout = () -> - $scope.ui.pdfLayout = 'sideBySide' - $scope.ui.view = 'editor' - localStorage "pdf.layout", "split" - - if pdfLayout = localStorage("pdf.layout") - $scope.switchToSideBySideLayout() if pdfLayout == "split" - $scope.switchToFlatLayout() if pdfLayout == "flat" - else - $scope.switchToSideBySideLayout() - App.factory "synctex", ["ide", "$http", "$q", (ide, $http, $q) -> # enable per-user containers by default perUserCompile = true From 7ae39a0f84db57c7f918f1521e1dc1b9b742f91e Mon Sep 17 00:00:00 2001 From: Chrystal Griffiths Date: Tue, 9 Oct 2018 10:30:24 +0100 Subject: [PATCH 33/43] Revert to ng-show --- services/web/app/views/project/editor/editor.pug | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/web/app/views/project/editor/editor.pug b/services/web/app/views/project/editor/editor.pug index 93368ceba4..fda0fa6446 100644 --- a/services/web/app/views/project/editor/editor.pug +++ b/services/web/app/views/project/editor/editor.pug @@ -102,6 +102,7 @@ div.full-size( ) i.synctex-control-icon div.full-size( - ng-if="ui.pdfLayout == 'flat' && ui.view == 'pdf'" + ng-if="ui.pdfLayout == 'flat'" + ng-show="ui.view == 'pdf'" ) include ./pdf From 98d35b4e50e2650d89c40b71a2798d2a537523a7 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Tue, 9 Oct 2018 11:23:21 +0100 Subject: [PATCH 34/43] Use settings instead of hard-coding ports --- services/web/config/settings.defaults.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index 9097c4e433..5136ef19f2 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -91,7 +91,7 @@ module.exports = settings = # running which conflict, or want to run the web process on port 80. internal: web: - port: webPort = 3000 + port: webPort = process.env['WEB_PORT'] or 3000 host: process.env['LISTEN_ADDRESS'] or 'localhost' documentupdater: port: docUpdaterPort = 3003 @@ -192,7 +192,7 @@ module.exports = settings = #clsiCookieKey: "clsiserver" # Same, but with http auth credentials. - httpAuthSiteUrl: 'http://#{httpAuthUser}:#{httpAuthPass}@localhost:3000' + httpAuthSiteUrl: 'http://#{httpAuthUser}:#{httpAuthPass}@#{siteUrl}' maxEntitiesPerProject: 2000 From 8c3b5acdd06aa73530639943d6ac0be775da36be Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 9 Oct 2018 11:46:19 +0100 Subject: [PATCH 35/43] update to metrics v1.8.0 for metrics.globalGauge --- services/web/npm-shrinkwrap.json | 6 +++--- services/web/package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/npm-shrinkwrap.json b/services/web/npm-shrinkwrap.json index 8fe259f82c..37a05d1681 100644 --- a/services/web/npm-shrinkwrap.json +++ b/services/web/npm-shrinkwrap.json @@ -6962,9 +6962,9 @@ "resolved": "https://registry.npmjs.org/methods/-/methods-1.1.2.tgz" }, "metrics-sharelatex": { - "version": "1.7.1", - "from": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.7.1", - "resolved": "git+https://github.com/sharelatex/metrics-sharelatex.git#166961924c599b1f9468f2e17846fa2a9d12372d", + "version": "1.8.0", + "from": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.8.0", + "resolved": "git+https://github.com/sharelatex/metrics-sharelatex.git#e57f1a84539cdf0398d0768b7f7af0c79ea5b05b", "dependencies": { "coffee-script": { "version": "1.6.0", diff --git a/services/web/package.json b/services/web/package.json index 0bacc0d2d1..d43544dcbe 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -62,7 +62,7 @@ "mailchimp-api-v3": "^1.12.0", "marked": "^0.3.5", "method-override": "^2.3.3", - "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.7.1", + "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.8.0", "minimist": "1.2.0", "mocha": "^5.0.1", "mongojs": "2.4.0", From 48e5c6b5236ec7a8027be188a5461899b78df93f Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Tue, 25 Sep 2018 14:10:06 +0100 Subject: [PATCH 36/43] add UserMembership logic --- .../Institutions/InstitutionsLocator.coffee | 9 + .../SubscriptionGroupController.coffee | 12 -- .../SubscriptionGroupHandler.coffee | 42 +---- .../Subscription/SubscriptionRouter.coffee | 3 +- .../Subscription/SubscriptionUpdater.coffee | 5 +- .../coffee/Features/User/UserGetter.coffee | 9 +- .../UserMembershipController.coffee | 65 +++++++ .../UserMembershipHandler.coffee | 85 +++++++++ .../UserMembershipRouter.coffee | 26 +++ .../UserMembershipViewModel.coffee | 45 +++++ .../web/app/coffee/models/Institution.coffee | 11 ++ services/web/app/coffee/router.coffee | 2 + .../index.pug} | 28 ++- services/web/public/coffee/main.coffee | 2 +- ...-members.coffee => user-membership.coffee} | 15 +- .../InstitutionsLocatorTests.coffee | 29 +++ .../SubscriptionGroupControllerTests.coffee | 19 -- .../SubscriptionGroupHandlerTests.coffee | 46 ----- .../SubscriptionUpdaterTests.coffee | 9 + .../UserMembershipControllerTests.coffee | 95 ++++++++++ .../UserMembershipHandlerTests.coffee | 172 ++++++++++++++++++ .../UserMembershipViewModelTests.coffee | 83 +++++++++ 22 files changed, 669 insertions(+), 143 deletions(-) create mode 100644 services/web/app/coffee/Features/Institutions/InstitutionsLocator.coffee create mode 100644 services/web/app/coffee/Features/UserMembership/UserMembershipController.coffee create mode 100644 services/web/app/coffee/Features/UserMembership/UserMembershipHandler.coffee create mode 100644 services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee create mode 100644 services/web/app/coffee/Features/UserMembership/UserMembershipViewModel.coffee create mode 100644 services/web/app/coffee/models/Institution.coffee rename services/web/app/views/{subscriptions/group_admin.pug => user_membership/index.pug} (68%) rename services/web/public/coffee/main/{group-members.coffee => user-membership.coffee} (75%) create mode 100644 services/web/test/unit/coffee/Institutions/InstitutionsLocatorTests.coffee create mode 100644 services/web/test/unit/coffee/UserMembership/UserMembershipControllerTests.coffee create mode 100644 services/web/test/unit/coffee/UserMembership/UserMembershipHandlerTests.coffee create mode 100644 services/web/test/unit/coffee/UserMembership/UserMembershipViewModelTests.coffee diff --git a/services/web/app/coffee/Features/Institutions/InstitutionsLocator.coffee b/services/web/app/coffee/Features/Institutions/InstitutionsLocator.coffee new file mode 100644 index 0000000000..013bd2ac00 --- /dev/null +++ b/services/web/app/coffee/Features/Institutions/InstitutionsLocator.coffee @@ -0,0 +1,9 @@ +Institution = require('../../models/Institution').Institution +logger = require("logger-sharelatex") +ObjectId = require('mongoose').Types.ObjectId + +module.exports = InstitutionLocator = + + findManagedInstitution: (managerId, callback)-> + logger.log managerId: managerId, "finding managed Institution" + Institution.findOne managerIds: managerId, callback diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee index 0fb7b6e7c9..3ebd096b70 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee @@ -52,18 +52,6 @@ module.exports = return res.sendStatus 500 res.send() - renderSubscriptionGroupAdminPage: (req, res, next)-> - user_id = AuthenticationController.getLoggedInUserId(req) - getManagedSubscription user_id, (error, subscription)-> - return next(error) if error? - if !subscription?.groupPlan - return res.redirect("/user/subscription") - SubscriptionGroupHandler.getPopulatedListOfMembers subscription._id, (err, users)-> - res.render "subscriptions/group_admin", - title: 'group_admin' - users: users - subscription: subscription - exportGroupCsv: (req, res)-> user_id = AuthenticationController.getLoggedInUserId(req) logger.log user_id: user_id, "exporting group csv" diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index badcde4c36..bb849fa7bd 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -11,6 +11,7 @@ TeamInvitesHandler = require("./TeamInvitesHandler") EmailHandler = require("../Email/EmailHandler") settings = require("settings-sharelatex") NotificationsBuilder = require("../Notifications/NotificationsBuilder") +UserMembershipViewModel = require("../UserMembership/UserMembershipViewModel") module.exports = SubscriptionGroupHandler = @@ -31,12 +32,12 @@ module.exports = SubscriptionGroupHandler = logger.err err:err, "error adding user to group" return callback(err) NotificationsBuilder.groupPlan(user, {subscription_id:subscription._id}).read() - userViewModel = buildUserViewModel(user) + userViewModel = UserMembershipViewModel.build(user) callback(err, userViewModel) else TeamInvitesHandler.createInvite subscriptionId, newEmail, (err) -> return callback(err) if err? - userViewModel = buildEmailInviteViewModel(newEmail) + userViewModel = UserMembershipViewModel.build(newEmail) callback(err, userViewModel) removeUserFromGroup: (subscriptionId, userToRemove_id, callback)-> @@ -51,28 +52,6 @@ module.exports = SubscriptionGroupHandler = replaceInArray Subscription, "member_ids", oldId, newId, callback - getPopulatedListOfMembers: (subscriptionId, callback)-> - SubscriptionLocator.getSubscription subscriptionId, (err, subscription)-> - users = [] - - for email in subscription.invited_emails or [] - users.push buildEmailInviteViewModel(email) - - for teamInvite in subscription.teamInvites or [] - users.push buildEmailInviteViewModel(teamInvite.email) - - jobs = _.map subscription.member_ids, (user_id)-> - return (cb)-> - UserGetter.getUser user_id, (err, user)-> - if err? or !user? - users.push _id:user_id - return cb() - userViewModel = buildUserViewModel(user) - users.push(userViewModel) - cb() - async.series jobs, (err)-> - callback(err, users) - isUserPartOfGroup: (user_id, subscription_id, callback=(err, partOfGroup)->)-> SubscriptionLocator.getSubscriptionByMemberIdAndId user_id, subscription_id, (err, subscription)-> if subscription? @@ -99,18 +78,3 @@ replaceInArray = (model, property, oldValue, newValue, callback) -> model.update query, { $addToSet: setNewValue }, { multi: true }, (error) -> return callback(error) if error? model.update query, { $pull: setOldValue }, { multi: true }, callback - -buildUserViewModel = (user)-> - u = - email: user.email - first_name: user.first_name - last_name: user.last_name - invite: user.holdingAccount - _id: user._id - return u - -buildEmailInviteViewModel = (email) -> - return { - email: email - invite: true - } diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee index 8950e3c5bd..79954b33e0 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee @@ -20,7 +20,8 @@ module.exports = webRouter.get '/user/subscription/thank-you', AuthenticationController.requireLogin(), SubscriptionController.successful_subscription - webRouter.get '/subscription/group', AuthenticationController.requireLogin(), SubscriptionGroupController.renderSubscriptionGroupAdminPage + webRouter.get '/subscription/group', AuthenticationController.requireLogin(), (req, res, next) -> + res.redirect('/manage/group/members') # legacy route webRouter.post '/subscription/group/user', AuthenticationController.requireLogin(), SubscriptionGroupController.addUserToGroup webRouter.get '/subscription/group/export', AuthenticationController.requireLogin(), SubscriptionGroupController.exportGroupCsv webRouter.delete '/subscription/group/user/:user_id', AuthenticationController.requireLogin(), SubscriptionGroupController.removeUserFromGroup diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index edcc58d57e..299320f061 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -55,7 +55,10 @@ module.exports = SubscriptionUpdater = if err? logger.err err:err, searchOps:searchOps, removeOperation:removeOperation, "error removing user from group" return callback(err) - FeaturesUpdater.refreshFeatures user_id, callback + UserGetter.getUserOrUserStubById user_id, {}, (error, user, isStub) -> + return callback(error) if error + return callback() if isStub + FeaturesUpdater.refreshFeatures user_id, callback deleteWithV1Id: (v1TeamId, callback)-> Subscription.deleteOne { "overleaf.id": v1TeamId }, callback diff --git a/services/web/app/coffee/Features/User/UserGetter.coffee b/services/web/app/coffee/Features/User/UserGetter.coffee index 4291cda6b7..e22ea204fd 100644 --- a/services/web/app/coffee/Features/User/UserGetter.coffee +++ b/services/web/app/coffee/Features/User/UserGetter.coffee @@ -66,15 +66,18 @@ module.exports = UserGetter = db.users.find { _id: { $in: user_ids} }, projection, callback - getUserOrUserStubById: (user_id, projection, callback = (error, user) ->) -> + getUserOrUserStubById: (user_id, projection, callback = (error, user, isStub) ->) -> try query = _id: ObjectId(user_id.toString()) catch e return callback(new Error(e)) db.users.findOne query, projection, (error, user) -> return callback(error) if error? - return callback(null, user) if user? - db.userstubs.findOne query, projection, callback + return callback(null, user, false) if user? + db.userstubs.findOne query, projection, (error, user) -> + return callback(error) if error + return callback() if !user? + callback(null, user, true) # check for duplicate email address. This is also enforced at the DB level ensureUniqueEmailAddress: (newEmail, callback) -> diff --git a/services/web/app/coffee/Features/UserMembership/UserMembershipController.coffee b/services/web/app/coffee/Features/UserMembership/UserMembershipController.coffee new file mode 100644 index 0000000000..bc673e1d5c --- /dev/null +++ b/services/web/app/coffee/Features/UserMembership/UserMembershipController.coffee @@ -0,0 +1,65 @@ +AuthenticationController = require('../Authentication/AuthenticationController') +UserMembershipHandler = require('./UserMembershipHandler') +logger = require("logger-sharelatex") + +module.exports = + index: (entityName, req, res, next)-> + userId = AuthenticationController.getLoggedInUserId(req) + + UserMembershipHandler.getEntity entityName, userId, (error, entity)-> + return next(error) if error? + UserMembershipHandler.getUsers entityName, entity, (error, users)-> + return next(error) if error? + res.render "user_membership/index", + users: users + entity: entity + translations: getTranslationsFor(entityName) + paths: getPathsFor(entityName) + + add: (entityName, req, res, next)-> + userId = AuthenticationController.getLoggedInUserId(req) + email = req.body.email + return res.sendStatus 422 unless email + + UserMembershipHandler.getEntity entityName, userId, (error, entity)-> + return next(error) if error? + UserMembershipHandler.addUser entityName, entity, email, (error, user)-> + return next(error) if error? + res.json(user: user) + + remove: (entityName, req, res, next)-> + loggedInUserId = AuthenticationController.getLoggedInUserId(req) + userId = req.params.userId + + UserMembershipHandler.getEntity entityName, loggedInUserId, (error, entity)-> + return next(error) if error? + UserMembershipHandler.removeUser entityName, entity, userId, (error, user)-> + return next(error) if error? + res.send() + +getTranslationsFor = (entityName) -> + switch entityName + when 'group' + title: 'group_account' + remove: 'remove_from_group' + when 'groupManagers' + title: 'group_managers' + remove: 'remove_manager' + when 'institution' + title: 'institution_managers' + remove: 'remove_manager' + + +getPathsFor = (entityName) -> + switch entityName + when 'group' + addMember: '/subscription/invites' + removeMember: '/subscription/group/user' + removeInvite: '/subscription/invites' + exportMembers: '/subscription/group/export' + when 'groupManagers' + addMember: "/manage/group/managers" + removeMember: "/manage/group/managers" + when 'institution' + addMember: "/manage/institution/managers" + removeMember: "/manage/institution/managers" diff --git a/services/web/app/coffee/Features/UserMembership/UserMembershipHandler.coffee b/services/web/app/coffee/Features/UserMembership/UserMembershipHandler.coffee new file mode 100644 index 0000000000..fa7b75fcb9 --- /dev/null +++ b/services/web/app/coffee/Features/UserMembership/UserMembershipHandler.coffee @@ -0,0 +1,85 @@ +async = require("async") +Errors = require('../Errors/Errors') +SubscriptionLocator = require('../Subscription/SubscriptionLocator') +InstitutionsLocator = require('../Institutions/InstitutionsLocator') +UserMembershipViewModel = require('./UserMembershipViewModel') +UserGetter = require('../User/UserGetter') +logger = require('logger-sharelatex') + +module.exports = + getEntity: (entityName, userId, callback = (error, entity) ->) -> + switch entityName + when 'group' then getGroupSubscription(userId, callback) + when 'groupManagers' + getGroupSubscription userId, (error, subscription) -> + subscription.membersLimit = null if subscription # managers are unlimited + callback(error, subscription) + when 'institution' then getInstitution(userId, callback) + else callback(new Errors.NotFoundError("No such entity: #{entityName}")) + + getUsers: (entityName, entity, callback = (error, users) ->) -> + attributes = switch entityName + when 'group' then ['invited_emails', 'teamInvites', 'member_ids'] + when 'groupManagers' then ['manager_ids'] + when 'institution' then ['managerIds'] + getPopulatedListOfMembers(entity, attributes, callback) + + addUser: (entityName, entity, email, callback = (error, user) ->) -> + attribute = switch entityName + when 'groupManagers' then 'manager_ids' + when 'institution' then 'managerIds' + unless attribute + return callback(new Errors.NotFoundError("Cannot add user to entity: #{entityName}")) + UserGetter.getUserByAnyEmail email, (error, user) -> + error ||= new Errors.NotFoundError("No user found with email #{email}") unless user + return callback(error) if error? + addUserToEntity entity, attribute, user, (error) -> + callback(error, UserMembershipViewModel.build(user)) + + removeUser: (entityName, entity, userId, callback = (error) ->) -> + attribute = switch entityName + when 'groupManagers' then 'manager_ids' + when 'institution' then 'managerIds' + else callback(new Errors.NotFoundError("Cannot remove user from entity: #{entityName}")) + removeUserFromEntity entity, attribute, userId, callback + +getGroupSubscription = (managerId, callback = (error, subscription) ->) -> + SubscriptionLocator.findManagedSubscription managerId, (err, subscription)-> + if subscription? and subscription.groupPlan + logger.log managerId: managerId, 'got managed subscription' + else + err ||= new Errors.NotFoundError("No subscription found managed by user #{managerId}") + + callback(err, subscription) + +getInstitution = (managerId, callback = (error, institution) ->) -> + InstitutionsLocator.findManagedInstitution managerId, (err, institution)-> + if institution? + logger.log managerId: managerId, 'got managed subscription' + else + err ||= new Errors.NotFoundError("No institution found managed by user #{managerId}") + + callback(err, institution) + +getPopulatedListOfMembers = (entity, attributes, callback = (error, users)->)-> + userObjects = [] + + for attribute in attributes + for userObject in entity[attribute] or [] + # userObject can be an email as String, a user id as ObjectId or an + # invite as Object with an email attribute as String. We want to pass to + # UserMembershipViewModel either an email as (String) or a user id (ObjectId) + userIdOrEmail = userObject.email || userObject + userObjects.push userIdOrEmail + + async.map userObjects, UserMembershipViewModel.buildAsync, callback + +addUserToEntity = (entity, attribute, user, callback = (error)->) -> + fieldUpdate = {} + fieldUpdate[attribute] = user._id + entity.update { $addToSet: fieldUpdate }, callback + +removeUserFromEntity = (entity, attribute, userId, callback = (error)->) -> + fieldUpdate = {} + fieldUpdate[attribute] = userId + entity.update { $pull: fieldUpdate }, callback diff --git a/services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee b/services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee new file mode 100644 index 0000000000..223b412020 --- /dev/null +++ b/services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee @@ -0,0 +1,26 @@ +AuthenticationController = require('../Authentication/AuthenticationController') +UserMembershipController = require './UserMembershipController' + +module.exports = + apply: (webRouter) -> + webRouter.get '/manage/group/members', + AuthenticationController.requireLogin(), + (req, res, next) -> UserMembershipController.index('group', req, res, next) + + + regularEntitites = + group: 'groupManagers' + institution: 'institution' + for pathName, entityName of regularEntitites + do (pathName, entityName) -> + webRouter.get "/manage/#{pathName}/managers", + AuthenticationController.requireLogin(), + (req, res, next) -> UserMembershipController.index(entityName, req, res, next) + + webRouter.post "/manage/#{pathName}/managers", + AuthenticationController.requireLogin(), + (req, res, next) -> UserMembershipController.add(entityName, req, res, next) + + webRouter.delete "/manage/#{pathName}/managers/:userId", + AuthenticationController.requireLogin(), + (req, res, next) -> UserMembershipController.remove(entityName, req, res, next) diff --git a/services/web/app/coffee/Features/UserMembership/UserMembershipViewModel.coffee b/services/web/app/coffee/Features/UserMembership/UserMembershipViewModel.coffee new file mode 100644 index 0000000000..ac5ba42168 --- /dev/null +++ b/services/web/app/coffee/Features/UserMembership/UserMembershipViewModel.coffee @@ -0,0 +1,45 @@ +ObjectId = require('mongojs').ObjectId +UserGetter = require('../User/UserGetter') + +module.exports = UserMembershipViewModel = + build: (userOrEmail) -> + if userOrEmail._id + buildUserViewModel userOrEmail + else + buildUserViewModelWithEmail userOrEmail + + + buildAsync: (userOrIdOrEmail, callback = (error, viewModel)->) -> + unless userOrIdOrEmail instanceof ObjectId + # userOrIdOrEmail is a user or an email and can be parsed by #build + return callback(null, UserMembershipViewModel.build(userOrIdOrEmail)) + + userId = userOrIdOrEmail + projection = { email: 1, first_name: 1, last_name: 1 } + UserGetter.getUserOrUserStubById userId, projection, (error, user, isStub) -> + if error? or !user? + return callback(null, buildUserViewModelWithId(userId.toString())) + if isStub + return callback(null, buildUserViewModelWithStub(user)) + callback(null, buildUserViewModel(user)) + + +buildUserViewModel = (user, isInvite = false) -> + _id: user._id or null + email: user.email or null + first_name: user.first_name or null + last_name: user.last_name or null + invite: isInvite + + +buildUserViewModelWithEmail = (email) -> + buildUserViewModel({ email }, true) + + +buildUserViewModelWithStub = (user) -> + # user stubs behave as invites + buildUserViewModel(user, true) + + +buildUserViewModelWithId = (id) -> + buildUserViewModel({ _id: id }, false) diff --git a/services/web/app/coffee/models/Institution.coffee b/services/web/app/coffee/models/Institution.coffee new file mode 100644 index 0000000000..decf114a96 --- /dev/null +++ b/services/web/app/coffee/models/Institution.coffee @@ -0,0 +1,11 @@ +mongoose = require 'mongoose' +Schema = mongoose.Schema +ObjectId = Schema.ObjectId + +InstitutionSchema = new Schema + v1Id: { type: Number, required: true } + managerIds: [ type:ObjectId, ref:'User' ] + +mongoose.model 'Institution', InstitutionSchema +exports.Institution = mongoose.model 'Institution' +exports.InstitutionSchema = InstitutionSchema diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 21fe396da9..8c44f505cb 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -50,6 +50,7 @@ TokenAccessController = require('./Features/TokenAccess/TokenAccessController') Features = require('./infrastructure/Features') LinkedFilesRouter = require './Features/LinkedFiles/LinkedFilesRouter' TemplatesRouter = require './Features/Templates/TemplatesRouter' +UserMembershipRouter = require './Features/UserMembership/UserMembershipRouter' logger = require("logger-sharelatex") _ = require("underscore") @@ -84,6 +85,7 @@ module.exports = class Router AnalyticsRouter.apply(webRouter, privateApiRouter, publicApiRouter) LinkedFilesRouter.apply(webRouter, privateApiRouter, publicApiRouter) TemplatesRouter.apply(webRouter) + UserMembershipRouter.apply(webRouter) Modules.applyRouter(webRouter, privateApiRouter, publicApiRouter) diff --git a/services/web/app/views/subscriptions/group_admin.pug b/services/web/app/views/user_membership/index.pug similarity index 68% rename from services/web/app/views/subscriptions/group_admin.pug rename to services/web/app/views/user_membership/index.pug index cdb18a0320..4a512a1c11 100644 --- a/services/web/app/views/subscriptions/group_admin.pug +++ b/services/web/app/views/user_membership/index.pug @@ -5,16 +5,16 @@ block content .container .row .col-md-10.col-md-offset-1 - .card(ng-controller="SubscriptionGroupMembersController") + .card(ng-controller="UserMembershipController") .page-header .pull-right(ng-cloak) - small(ng-show="selectedUsers.length == 0") !{translate("you_have_added_x_of_group_size_y", {addedUsersSize:"{{ users.length }}", groupSize:"{{ groupSize }}"})} + small(ng-show="groupSize && selectedUsers.length == 0") !{translate("you_have_added_x_of_group_size_y", {addedUsersSize:"{{ users.length }}", groupSize:"{{ groupSize }}"})} a.btn.btn-danger( href, ng-show="selectedUsers.length > 0" ng-click="removeMembers()" - ) #{translate("remove_from_group")} - h1 #{translate("group_account")} + ) #{translate(translations.remove)} + h1 #{translate(translations.title)} .row-spaced-small ul.list-unstyled.structured-list( @@ -35,7 +35,7 @@ block content span.header #{translate("accepted_invite")} li.container-fluid( ng-repeat="user in users | orderBy:'email':true", - ng-controller="SubscriptionGroupMemberListItemController" + ng-controller="UserMembershipListItemController" ) .row .col-md-5 @@ -60,7 +60,7 @@ block content small #{translate("no_members")} hr - div(ng-if="users.length < groupSize", ng-cloak) + div(ng-if="!groupSize || users.length < groupSize", ng-cloak) p.small #{translate("add_more_members")} form.form .row @@ -74,18 +74,16 @@ block content ) .col-xs-4 button.btn.btn-primary(ng-click="addMembers()") #{translate("add")} - .col-xs-2 - a(href="/subscription/group/export") Export CSV + .col-xs-2(ng-if="paths.exportMembers", ng-cloak) + a(href=paths.exportMembers) Export CSV - div(ng-if="users.length >= groupSize && users.length > 0", ng-cloak) + div(ng-if="groupSize && users.length >= groupSize && users.length > 0", ng-cloak) .row - .col-xs-2.col-xs-offset-10 - a(href="/subscription/group/export") Export CSV + .col-xs-2.col-xs-offset-10(ng-if="paths.exportMembers", ng-cloak) + a(href=paths.exportMembers) Export CSV script(type="text/javascript"). window.users = !{JSON.stringify(users)}; - window.groupSize = #{subscription.membersLimit}; - - - + window.paths = !{JSON.stringify(paths)}; + window.groupSize = #{entity.membersLimit || 'null'}; diff --git a/services/web/public/coffee/main.coffee b/services/web/public/coffee/main.coffee index fc62c6bc38..49b211b0aa 100644 --- a/services/web/public/coffee/main.coffee +++ b/services/web/public/coffee/main.coffee @@ -5,7 +5,7 @@ define [ "main/clear-sessions" "main/account-upgrade" "main/plans" - "main/group-members" + "main/user-membership" "main/scribtex-popup" "main/event" "main/bonus" diff --git a/services/web/public/coffee/main/group-members.coffee b/services/web/public/coffee/main/user-membership.coffee similarity index 75% rename from services/web/public/coffee/main/group-members.coffee rename to services/web/public/coffee/main/user-membership.coffee index 028639c741..1d2df70407 100644 --- a/services/web/public/coffee/main/group-members.coffee +++ b/services/web/public/coffee/main/user-membership.coffee @@ -1,9 +1,10 @@ define [ "base" ], (App) -> - App.controller "SubscriptionGroupMembersController", ($scope, queuedHttp) -> + App.controller "UserMembershipController", ($scope, queuedHttp) -> $scope.users = window.users $scope.groupSize = window.groupSize + $scope.paths = window.paths $scope.selectedUsers = [] $scope.inputs = @@ -22,7 +23,7 @@ define [ emails = parseEmails($scope.inputs.emails) for email in emails queuedHttp - .post("/subscription/invites", { + .post(paths.addMember, { email: email, _csrf: window.csrfToken }) @@ -34,10 +35,12 @@ define [ $scope.removeMembers = () -> for user in $scope.selectedUsers do (user) -> - if user.invite and !user._id? - url = "/subscription/invites/#{encodeURIComponent(user.email)}" + if paths.removeInvite and user.invite and !user._id? + url = "#{paths.removeInvite}/#{encodeURIComponent(user.email)}" + else if paths.removeMember and user._id? + url = "#{paths.removeMember}/#{user._id}" else - url = "/subscription/group/user/#{user._id}" + return queuedHttp({ method: "DELETE", url: url @@ -53,7 +56,7 @@ define [ $scope.updateSelectedUsers = () -> $scope.selectedUsers = $scope.users.filter (user) -> user.selected - App.controller "SubscriptionGroupMemberListItemController", ($scope) -> + App.controller "UserMembershipListItemController", ($scope) -> $scope.$watch "user.selected", (value) -> if value? $scope.updateSelectedUsers() diff --git a/services/web/test/unit/coffee/Institutions/InstitutionsLocatorTests.coffee b/services/web/test/unit/coffee/Institutions/InstitutionsLocatorTests.coffee new file mode 100644 index 0000000000..3edb55e09a --- /dev/null +++ b/services/web/test/unit/coffee/Institutions/InstitutionsLocatorTests.coffee @@ -0,0 +1,29 @@ +SandboxedModule = require('sandboxed-module') +should = require('chai').should() +sinon = require('sinon') +assertCalledWith = sinon.assert.calledWith +assertNotCalled = sinon.assert.notCalled +modulePath = "../../../../app/js/Features/Institutions/InstitutionsLocator" +assert = require("chai").assert +ObjectId = require('mongoose').Types.ObjectId + +describe 'InstitutionsLocator', -> + beforeEach -> + @user = + _id: "5208dd34438842e2db333333" + @institution = + v1Id: 123 + managersIds: [ObjectId(), ObjectId()] + @Institution = + findOne: sinon.stub().yields(null, @institution) + @InstitutionsLocator = SandboxedModule.require modulePath, requires: + '../../models/Institution': Institution: @Institution + "logger-sharelatex": log:-> + + describe "finding managed institution", -> + + it "should query the database", (done) -> + @InstitutionsLocator.findManagedInstitution @user._id, (err, institution)=> + assertCalledWith(@Institution.findOne, { managerIds: @user._id }) + institution.should.equal @institution + done() diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee index a399496399..91706130f3 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee @@ -81,25 +81,6 @@ describe "SubscriptionGroupController", -> done() @Controller.removeUserFromGroup @req, res - describe "renderSubscriptionGroupAdminPage", -> - it "should redirect you if you don't have a group account", (done)-> - @subscription.groupPlan = false - - res = - redirect : (path)=> - path.should.equal("/user/subscription") - done() - @Controller.renderSubscriptionGroupAdminPage @req, res - - it "should redirect you don't have a subscription", (done)-> - @SubscriptionLocator.getUsersSubscription = sinon.stub().callsArgWith(1) - - res = - redirect : (path)=> - path.should.equal("/user/subscription") - done() - @Controller.renderSubscriptionGroupAdminPage @req, res - describe "exportGroupCsv", -> beforeEach -> diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index 78f6ac3805..0f2290f014 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee @@ -157,52 +157,6 @@ describe "SubscriptionGroupHandler", -> { $pull: { member_ids: @oldId } } ).should.equal true - describe "getPopulatedListOfMembers", -> - beforeEach -> - @subscription = {} - @SubscriptionLocator.getSubscription.callsArgWith(1, null, @subscription) - @UserGetter.getUser.callsArgWith(1, null, {_id:"31232"}) - - it "should locate the subscription", (done)-> - @UserGetter.getUser.callsArgWith(1, null, {_id:"31232"}) - @Handler.getPopulatedListOfMembers @subscriptionId, (err, users)=> - @SubscriptionLocator.getSubscription.calledWith(@subscriptionId).should.equal true - done() - - it "should get the users by id", (done)-> - @UserGetter.getUser.callsArgWith(1, null, {_id:"31232"}) - @subscription.member_ids = ["1234", "342432", "312312"] - @Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=> - @UserGetter.getUser.calledWith(@subscription.member_ids[0]).should.equal true - @UserGetter.getUser.calledWith(@subscription.member_ids[1]).should.equal true - @UserGetter.getUser.calledWith(@subscription.member_ids[2]).should.equal true - users.length.should.equal @subscription.member_ids.length - done() - - it "should just return the id if the user can not be found as they may have deleted their account", (done)-> - @UserGetter.getUser.callsArgWith(1) - @subscription.member_ids = ["1234", "342432", "312312"] - @Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=> - assert.deepEqual users[0], {_id:@subscription.member_ids[0]} - assert.deepEqual users[1], {_id:@subscription.member_ids[1]} - assert.deepEqual users[2], {_id:@subscription.member_ids[2]} - done() - - it "should return any invited users", (done) -> - @subscription.invited_emails = [ "jo@example.com" ] - - @subscription.teamInvites = [ - { email: "charlie@example.com" } - ] - - @Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=> - users[0].email.should.equal "jo@example.com" - users[0].invite.should.equal true - users[1].email.should.equal "charlie@example.com" - users[1].invite.should.equal true - users.length.should.equal @subscription.teamInvites.length + @subscription.invited_emails.length - done() - describe "isUserPartOfGroup", -> beforeEach -> @subscription_id = "123ed13123" diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee index a66d7f5dec..1a67a183b4 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee @@ -17,6 +17,7 @@ describe "SubscriptionUpdater", -> _id: @adminuser_id = "5208dd34438843e2db000007" @otherUserId = "5208dd34438842e2db000005" @allUserIds = ["13213", "dsadas", "djsaiud89"] + @userStub = _id: 'mock-user-stub-id', email: 'mock-stub-email@baz.com' @subscription = subscription = _id: "111111111111111111111111" admin_id: @adminUser._id @@ -67,6 +68,7 @@ describe "SubscriptionUpdater", -> getUsers: (memberIds, projection, callback) -> users = memberIds.map (id) -> { _id: id } callback(null, users) + getUserOrUserStubById: sinon.stub() @ReferalFeatures = getBonusFeatures: sinon.stub().callsArgWith(1) @Modules = {hooks: {fire: sinon.stub().callsArgWith(2, null, null)}} @@ -190,6 +192,7 @@ describe "SubscriptionUpdater", -> describe "removeUserFromGroup", -> beforeEach -> @FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1) + @UserGetter.getUserOrUserStubById.yields(null, {}, false) it "should pull the users id from the group", (done)-> @SubscriptionUpdater.removeUserFromGroup @subscription._id, @otherUserId, => @@ -205,6 +208,12 @@ describe "SubscriptionUpdater", -> @FeaturesUpdater.refreshFeatures.calledWith(@otherUserId).should.equal true done() + it "should not update features for user stubs", (done)-> + @UserGetter.getUserOrUserStubById.yields(null, {}, true) + @SubscriptionUpdater.removeUserFromGroup @subscription._id, @userStub._id, => + @FeaturesUpdater.refreshFeatures.called.should.equal false + done() + describe "deleteSubscription", -> beforeEach (done) -> @subscription_id = ObjectId().toString() diff --git a/services/web/test/unit/coffee/UserMembership/UserMembershipControllerTests.coffee b/services/web/test/unit/coffee/UserMembership/UserMembershipControllerTests.coffee new file mode 100644 index 0000000000..f890abfefa --- /dev/null +++ b/services/web/test/unit/coffee/UserMembership/UserMembershipControllerTests.coffee @@ -0,0 +1,95 @@ +sinon = require('sinon') +assertCalledWith = sinon.assert.calledWith +assertNotCalled = sinon.assert.notCalled +chai = require('chai') +should = chai.should() +assert = chai.assert +expect = require('chai').expect +modulePath = "../../../../app/js/Features/UserMembership/UserMembershipController.js" +SandboxedModule = require('sandboxed-module') +MockRequest = require "../helpers/MockRequest" +MockResponse = require "../helpers/MockResponse" + +describe "UserMembershipController", -> + beforeEach -> + @req = new MockRequest() + @user = _id: 'mock-user-id' + @newUser = _id: 'mock-new-user-id', email: 'new-user-email@foo.bar' + @subscription = { _id: 'mock-subscription-id'} + @users = [{ _id: 'mock-member-id-1' }, { _id: 'mock-member-id-2' }] + + @AuthenticationController = + getLoggedInUserId: sinon.stub().returns(@user._id) + @UserMembershipHandler = + getEntity: sinon.stub().yields(null, @subscription) + getUsers: sinon.stub().yields(null, @users) + addUser: sinon.stub().yields(null, @newUser) + removeUser: sinon.stub().yields(null) + @UserMembershipController = SandboxedModule.require modulePath, requires: + '../Authentication/AuthenticationController': @AuthenticationController + './UserMembershipHandler': @UserMembershipHandler + "logger-sharelatex": + log: -> + err: -> + + describe 'index', -> + it 'get entity', (done) -> + @UserMembershipController.index 'group', @req, render: () => + sinon.assert.calledWith(@UserMembershipHandler.getEntity, 'group', @user._id) + done() + + it 'get users', (done) -> + @UserMembershipController.index 'group', @req, render: () => + sinon.assert.calledWith(@UserMembershipHandler.getUsers, 'group', @subscription) + done() + + it 'render group view', (done) -> + @UserMembershipController.index 'group', @req, render: (viewPath, viewParams) => + expect(viewPath).to.equal 'user_membership/index' + expect(viewParams.entity).to.deep.equal @subscription + expect(viewParams.users).to.deep.equal @users + expect(viewParams.translations.title).to.equal 'group_account' + expect(viewParams.paths.addMember).to.equal '/subscription/invites' + done() + + it 'render group managers view', (done) -> + @UserMembershipController.index 'groupManagers', @req, render: (viewPath, viewParams) => + expect(viewPath).to.equal 'user_membership/index' + expect(viewParams.translations.title).to.equal 'group_managers' + expect(viewParams.paths.exportMembers).to.be.undefined + done() + + it 'render institution view', (done) -> + @UserMembershipController.index 'institution', @req, render: (viewPath, viewParams) => + expect(viewPath).to.equal 'user_membership/index' + expect(viewParams.translations.title).to.equal 'institution_managers' + expect(viewParams.paths.exportMembers).to.be.undefined + done() + + describe 'add', -> + beforeEach -> + @req.body.email = @newUser.email + + it 'get entity', (done) -> + @UserMembershipController.add 'groupManagers', @req, json: () => + sinon.assert.calledWith(@UserMembershipHandler.getEntity, 'groupManagers', @user._id) + done() + + it 'add user', (done) -> + @UserMembershipController.add 'groupManagers', @req, json: () => + sinon.assert.calledWith(@UserMembershipHandler.addUser, 'groupManagers', @subscription, @newUser.email) + done() + + it 'return user object', (done) -> + @UserMembershipController.add 'groupManagers', @req, json: (payload) => + payload.user.should.equal @newUser + done() + + describe 'remove', -> + beforeEach -> + @req.params.userId = @newUser._id + + it 'remove user', (done) -> + @UserMembershipController.remove 'groupManagers', @req, send: () => + sinon.assert.calledWith(@UserMembershipHandler.removeUser, 'groupManagers', @subscription, @newUser._id) + done() diff --git a/services/web/test/unit/coffee/UserMembership/UserMembershipHandlerTests.coffee b/services/web/test/unit/coffee/UserMembership/UserMembershipHandlerTests.coffee new file mode 100644 index 0000000000..23598a82a1 --- /dev/null +++ b/services/web/test/unit/coffee/UserMembership/UserMembershipHandlerTests.coffee @@ -0,0 +1,172 @@ +chai = require('chai') +should = chai.should() +expect = require('chai').expect +sinon = require('sinon') +assertCalledWith = sinon.assert.calledWith +assertNotCalled = sinon.assert.notCalled +ObjectId = require("../../../../app/js/infrastructure/mongojs").ObjectId +modulePath = "../../../../app/js/Features/UserMembership/UserMembershipHandler" +SandboxedModule = require("sandboxed-module") +Errors = require("../../../../app/js/Features/Errors/Errors") + +describe 'UserMembershipHandler', -> + beforeEach -> + @user = _id: 'mock-user-id' + @newUser = _id: 'mock-new-user-id', email: 'new-user-email@foo.bar' + @subscription = + _id: 'mock-subscription-id' + groupPlan: true + membersLimit: 10 + member_ids: [ObjectId(), ObjectId()] + manager_ids: [ObjectId()] + invited_emails: ['mock-email-1@foo.com'] + teamInvites: [{ email: 'mock-email-1@bar.com' }] + update: sinon.stub().yields(null) + @institution = + _id: 'mock-institution-id' + v1Id: 123 + managerIds: [ObjectId(), ObjectId(), ObjectId()] + update: sinon.stub().yields(null) + + @SubscriptionLocator = + findManagedSubscription: sinon.stub().yields(null, @subscription) + @InstitutionsLocator = + findManagedInstitution: sinon.stub().yields(null, @institution) + @UserMembershipViewModel = + buildAsync: sinon.stub().yields(null, { _id: 'mock-member-id'}) + build: sinon.stub().returns(@newUser) + @UserGetter = + getUserByAnyEmail: sinon.stub().yields(null, @newUser) + @UserMembershipHandler = SandboxedModule.require modulePath, requires: + '../Subscription/SubscriptionLocator': @SubscriptionLocator + '../Institutions/InstitutionsLocator': @InstitutionsLocator + './UserMembershipViewModel': @UserMembershipViewModel + '../User/UserGetter': @UserGetter + '../Errors/Errors': Errors + 'logger-sharelatex': + log: -> + err: -> + + describe 'getEntty', -> + it 'validate type', (done) -> + @UserMembershipHandler.getEntity 'foo', null, (error) -> + should.exist(error) + expect(error.message).to.match /No such entity/ + done() + + describe 'group subscriptions', -> + it 'get subscription', (done) -> + @UserMembershipHandler.getEntity 'group', @user._id, (error, subscription) => + should.not.exist(error) + assertCalledWith(@SubscriptionLocator.findManagedSubscription, @user._id) + expect(subscription).to.equal @subscription + expect(subscription.membersLimit).to.equal 10 + done() + + it 'check subscription is a group', (done) -> + @SubscriptionLocator.findManagedSubscription.yields(null, { groupPlan: false }) + @UserMembershipHandler.getEntity 'group', @user._id, (error, subscription) -> + should.exist(error) + done() + + it 'handle error', (done) -> + @SubscriptionLocator.findManagedSubscription.yields(new Error('some error')) + @UserMembershipHandler.getEntity 'group', @user._id, (error, subscription) => + should.exist(error) + done() + + describe 'group managers', -> + it 'has no members limit', (done) -> + @UserMembershipHandler.getEntity 'groupManagers', @user._id, (error, subscription) => + should.not.exist(error) + assertCalledWith(@SubscriptionLocator.findManagedSubscription, @user._id) + expect(subscription.membersLimit).to.equal null + done() + + describe 'institutions', -> + it 'get institution', (done) -> + @UserMembershipHandler.getEntity 'institution', @user._id, (error, institution) => + should.not.exist(error) + assertCalledWith(@InstitutionsLocator.findManagedInstitution, @user._id) + expect(institution).to.equal @institution + done() + + it 'handle institution not found', (done) -> + @InstitutionsLocator.findManagedInstitution.yields(null, null) + @UserMembershipHandler.getEntity 'institution', @user._id, (error, institution) => + should.exist(error) + expect(error).to.be.an.instanceof(Errors.NotFoundError) + done() + + it 'handle errors', (done) -> + @InstitutionsLocator.findManagedInstitution.yields(new Error('nope')) + @UserMembershipHandler.getEntity 'institution', @user._id, (error, institution) => + should.exist(error) + expect(error).to.not.be.an.instanceof(Errors.NotFoundError) + done() + + describe 'getUsers', -> + describe 'group', -> + it 'build view model for all users', (done) -> + @UserMembershipHandler.getUsers 'group', @subscription, (error, users) => + expectedCallcount = + @subscription.member_ids.length + + @subscription.invited_emails.length + + @subscription.teamInvites.length + expect(@UserMembershipViewModel.buildAsync.callCount).to.equal expectedCallcount + done() + + describe 'group mamagers', -> + it 'build view model for all managers', (done) -> + @UserMembershipHandler.getUsers 'groupManagers', @subscription, (error, users) => + expectedCallcount = @subscription.manager_ids.length + expect(@UserMembershipViewModel.buildAsync.callCount).to.equal expectedCallcount + done() + + describe 'institution', -> + it 'build view model for all managers', (done) -> + @UserMembershipHandler.getUsers 'institution', @institution, (error, users) => + expectedCallcount = @institution.managerIds.length + expect(@UserMembershipViewModel.buildAsync.callCount).to.equal expectedCallcount + done() + + describe 'addUser', -> + beforeEach -> + @email = @newUser.email + + describe 'group', -> + it 'fails', (done) -> + @UserMembershipHandler.addUser 'group', @subscription, @email, (error) => + expect(error).to.exist + done() + + describe 'institution', -> + it 'get user', (done) -> + @UserMembershipHandler.addUser 'institution', @institution, @email, (error, user) => + assertCalledWith(@UserGetter.getUserByAnyEmail, @email) + done() + + it 'handle user not found', (done) -> + @UserGetter.getUserByAnyEmail.yields(null, null) + @UserMembershipHandler.addUser 'institution', @institution, @email, (error) => + expect(error).to.exist + expect(error).to.be.an.instanceof(Errors.NotFoundError) + done() + + it 'add user to institution', (done) -> + @UserMembershipHandler.addUser 'institution', @institution, @email, (error, user) => + assertCalledWith(@institution.update, { $addToSet: managerIds: @newUser._id }) + done() + + it 'return user view', (done) -> + @UserMembershipHandler.addUser 'institution', @institution, @email, (error, user) => + user.should.equal @newUser + done() + + describe 'removeUser', -> + describe 'institution', -> + it 'remove user from institution', (done) -> + @UserMembershipHandler.removeUser 'institution', @institution, @newUser._id, (error, user) => + lastCall = @institution.update.lastCall + assertCalledWith(@institution.update, { $pull: managerIds: @newUser._id }) + done() diff --git a/services/web/test/unit/coffee/UserMembership/UserMembershipViewModelTests.coffee b/services/web/test/unit/coffee/UserMembership/UserMembershipViewModelTests.coffee new file mode 100644 index 0000000000..7dc7dbeff0 --- /dev/null +++ b/services/web/test/unit/coffee/UserMembership/UserMembershipViewModelTests.coffee @@ -0,0 +1,83 @@ +chai = require('chai') +should = chai.should() +expect = require('chai').expect +sinon = require('sinon') +assertCalledWith = sinon.assert.calledWith +assertNotCalled = sinon.assert.notCalled +mongojs = require('mongojs') +ObjectId = mongojs.ObjectId +modulePath = "../../../../app/js/Features/UserMembership/UserMembershipViewModel" +SandboxedModule = require("sandboxed-module") + +describe 'UserMembershipViewModel', -> + beforeEach -> + @UserGetter = + getUserOrUserStubById: sinon.stub() + @UserMembershipViewModel = SandboxedModule.require modulePath, requires: + 'mongojs': mongojs + '../User/UserGetter': @UserGetter + @email = 'mock-email@bar.com' + @user = _id: 'mock-user-id', email: 'mock-email@baz.com', first_name: 'Name' + @userStub = _id: 'mock-user-stub-id', email: 'mock-stub-email@baz.com' + + describe 'build', -> + it 'build email', -> + viewModel = @UserMembershipViewModel.build(@email) + expect(viewModel).to.deep.equal + email: @email + invite: true + first_name: null + last_name: null + _id: null + + it 'build user', -> + viewModel = @UserMembershipViewModel.build(@user) + expect(viewModel._id).to.equal @user._id + expect(viewModel.email).to.equal @user.email + expect(viewModel.invite).to.equal false + + describe 'build async', -> + beforeEach -> + @UserMembershipViewModel.build = sinon.stub() + + it 'build email', (done) -> + @UserMembershipViewModel.buildAsync @email, (error, viewModel) => + assertCalledWith(@UserMembershipViewModel.build, @email) + done() + + it 'build user', (done) -> + @UserMembershipViewModel.buildAsync @user, (error, viewModel) => + assertCalledWith(@UserMembershipViewModel.build, @user) + done() + + it 'build user id', (done) -> + @UserGetter.getUserOrUserStubById.yields(null, @user, false) + @UserMembershipViewModel.buildAsync ObjectId(), (error, viewModel) => + should.not.exist(error) + assertNotCalled(@UserMembershipViewModel.build) + expect(viewModel._id).to.equal @user._id + expect(viewModel.email).to.equal @user.email + expect(viewModel.first_name).to.equal @user.first_name + expect(viewModel.invite).to.equal false + should.exist(viewModel.email) + done() + + it 'build user stub id', (done) -> + @UserGetter.getUserOrUserStubById.yields(null, @userStub, true) + @UserMembershipViewModel.buildAsync ObjectId(), (error, viewModel) => + should.not.exist(error) + assertNotCalled(@UserMembershipViewModel.build) + expect(viewModel._id).to.equal @userStub._id + expect(viewModel.email).to.equal @userStub.email + expect(viewModel.invite).to.equal true + done() + + it 'build user id with error', (done) -> + @UserGetter.getUserOrUserStubById.yields(new Error('nope')) + userId = ObjectId() + @UserMembershipViewModel.buildAsync userId, (error, viewModel) => + should.not.exist(error) + assertNotCalled(@UserMembershipViewModel.build) + expect(viewModel._id).to.equal userId.toString() + should.not.exist(viewModel.email) + done() From 7652e808007c34d081899182dbdd46f1425a5bce Mon Sep 17 00:00:00 2001 From: Ersun Warncke Date: Tue, 9 Oct 2018 06:55:14 -0400 Subject: [PATCH 37/43] add account merge error --- .../app/coffee/Features/Errors/ErrorController.coffee | 8 ++++++++ services/web/app/coffee/Features/Errors/Errors.coffee | 8 ++++++++ .../web/app/views/general/account-merge-error.pug | 11 +++++++++++ 3 files changed, 27 insertions(+) create mode 100644 services/web/app/views/general/account-merge-error.pug diff --git a/services/web/app/coffee/Features/Errors/ErrorController.coffee b/services/web/app/coffee/Features/Errors/ErrorController.coffee index 50f0ba3c06..25760bb1c3 100644 --- a/services/web/app/coffee/Features/Errors/ErrorController.coffee +++ b/services/web/app/coffee/Features/Errors/ErrorController.coffee @@ -13,6 +13,11 @@ module.exports = ErrorController = res.render 'general/500', title: "Server Error" + accountMergeError: (req, res)-> + res.status(500) + res.render 'general/account-merge-error', + title: "Account Access Error" + handleError: (error, req, res, next) -> user = AuthenticationController.getSessionUser(req) if error?.code is 'EBADCSRFTOKEN' @@ -33,6 +38,9 @@ module.exports = ErrorController = logger.warn {err: error, url: req.url}, "invalid name error" res.status(400) res.send(error.message) + else if error instanceof Errors.AccountMergeError + logger.error err: error, "account merge error" + ErrorController.accountMergeError req, res else logger.error err: error, url:req.url, method:req.method, user:user, "error passed to top level next middlewear" ErrorController.serverError req, res diff --git a/services/web/app/coffee/Features/Errors/Errors.coffee b/services/web/app/coffee/Features/Errors/Errors.coffee index 3239bbbb58..0b20992faa 100644 --- a/services/web/app/coffee/Features/Errors/Errors.coffee +++ b/services/web/app/coffee/Features/Errors/Errors.coffee @@ -89,6 +89,13 @@ InvalidError = (message) -> return error InvalidError.prototype.__proto__ = Error.prototype +AccountMergeError = (message) -> + error = new Error(message) + error.name = "AccountMergeError" + error.__proto__ = AccountMergeError.prototype + return error +AccountMergeError.prototype.__proto__ = Error.prototype + module.exports = Errors = NotFoundError: NotFoundError ServiceNotConfiguredError: ServiceNotConfiguredError @@ -103,3 +110,4 @@ module.exports = Errors = UnconfirmedEmailError: UnconfirmedEmailError EmailExistsError: EmailExistsError InvalidError: InvalidError + AccountMergeError: AccountMergeError diff --git a/services/web/app/views/general/account-merge-error.pug b/services/web/app/views/general/account-merge-error.pug new file mode 100644 index 0000000000..70c5113781 --- /dev/null +++ b/services/web/app/views/general/account-merge-error.pug @@ -0,0 +1,11 @@ +extends ../layout + +block content + .content.content-alt + .container + .row + .col-md-6.col-md-offset-3 + .card + .page-header + h1 Account Access Error + p.text-danger Sorry, an error occurred accessing your account. Please #[a(href="" ng-controller="ContactModal" ng-click="contactUsModal()") contact support] for assistance. From 8719eff1d773fbc4afdc9bfb8e0e2454a04d7e2b Mon Sep 17 00:00:00 2001 From: Ersun Warncke Date: Tue, 9 Oct 2018 11:44:59 -0400 Subject: [PATCH 38/43] update error message --- services/web/app/views/general/account-merge-error.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/views/general/account-merge-error.pug b/services/web/app/views/general/account-merge-error.pug index 70c5113781..68af40a267 100644 --- a/services/web/app/views/general/account-merge-error.pug +++ b/services/web/app/views/general/account-merge-error.pug @@ -8,4 +8,4 @@ block content .card .page-header h1 Account Access Error - p.text-danger Sorry, an error occurred accessing your account. Please #[a(href="" ng-controller="ContactModal" ng-click="contactUsModal()") contact support] for assistance. + p.text-danger Sorry, an error occurred accessing your account. Please #[a(href="" ng-controller="ContactModal" ng-click="contactUsModal()") contact support] and provide any email addresses that you have used to sign in to Overleaf and/or ShareLaTeX for assistance. From 32149e652ff1a46e057bd9e3dae2da1f3494422e Mon Sep 17 00:00:00 2001 From: Simon Detheridge Date: Tue, 9 Oct 2018 17:09:49 +0100 Subject: [PATCH 39/43] Handle 'invalid element name' error in project list ui When invalid filenames are found during project-copy, the somewhat obscure (and non-localised) 'invalid element name' error is returned. Add a special case to handle this particular error and display something more descriptive to the user. Added a modal error handler for when this error is generated by clicking the 'copy' icon in the project list, instead of using the 'more' dropdown which opens a modal copy dialog bug: overleaf/sharelatex#908 Signed-off-by: Simon Detheridge --- .../web/app/views/project/list/modals.pug | 20 ++++++++++++--- .../project-list/modal-controllers.coffee | 14 +++++++---- .../main/project-list/project-list.coffee | 25 +++++++++++++------ 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/services/web/app/views/project/list/modals.pug b/services/web/app/views/project/list/modals.pug index 50d7f8d2ef..11e8da29ea 100644 --- a/services/web/app/views/project/list/modals.pug +++ b/services/web/app/views/project/list/modals.pug @@ -98,7 +98,7 @@ script(type='text/ng-template', id='renameProjectModalTemplate') ) × h3 #{translate("rename_project")} .modal-body - .alert.alert-danger(ng-show="state.error.message") {{ state.error.message}} + .alert.alert-danger(ng-show="state.error.message") {{state.error.message}} .alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")} form(name="renameProjectForm", novalidate) input.form-control( @@ -127,7 +127,7 @@ script(type='text/ng-template', id='cloneProjectModalTemplate') ) × h3 #{translate("copy_project")} .modal-body - .alert.alert-danger(ng-show="state.error.message") {{ state.error.message}} + .alert.alert-danger(ng-show="state.error.message") {{state.error.message === "invalid element name" ? translate("invalid_element_name") : state.error.message}} .alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")} form(name="cloneProjectForm", novalidate) .form-group @@ -161,7 +161,7 @@ script(type='text/ng-template', id='newProjectModalTemplate') ) × h3 #{translate("new_project")} .modal-body - .alert.alert-danger(ng-show="state.error.message") {{ state.error.message}} + .alert.alert-danger(ng-show="state.error.message") {{state.error.message}} .alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")} form(novalidate, name="newProjectForm") input.form-control( @@ -262,6 +262,20 @@ script(type="text/ng-template", id="uploadProjectModalTemplate") .modal-footer button.btn.btn-default(ng-click="cancel()") #{translate("cancel")} +script(type="text/ng-template", id="showErrorModalTemplate") + .modal-header + button.close( + type="button" + data-dismiss="modal" + ng-click="cancel()" + ) × + h3 #{translate("generic_something_went_wrong")} + .modal-body + .alert.alert-danger(ng-show="error.message") {{error.message === "invalid element name" ? translate("invalid_element_name") : error.message}} + .alert.alert-danger(ng-show="error && !error.message") #{translate("generic_something_went_wrong")} + .modal-footer + button.btn.btn-default(ng-click="cancel()") #{translate("cancel")} + script(type="text/ng-template", id="userProfileModalTemplate") .modal-header button.close( diff --git a/services/web/public/coffee/main/project-list/modal-controllers.coffee b/services/web/public/coffee/main/project-list/modal-controllers.coffee index 27124e0951..3d15797940 100644 --- a/services/web/public/coffee/main/project-list/modal-controllers.coffee +++ b/services/web/public/coffee/main/project-list/modal-controllers.coffee @@ -2,9 +2,9 @@ define [ "base" ], (App) -> App.controller 'RenameProjectModalController', ($scope, $modalInstance, $timeout, project, queuedHttp) -> - $scope.inputs = + $scope.inputs = projectName: project.name - + $scope.state = inflight: false error: false @@ -35,7 +35,7 @@ define [ $modalInstance.dismiss('cancel') App.controller 'CloneProjectModalController', ($scope, $modalInstance, $timeout, project) -> - $scope.inputs = + $scope.inputs = projectName: project.name + " (Copy)" $scope.state = inflight: false @@ -66,7 +66,7 @@ define [ $modalInstance.dismiss('cancel') App.controller 'NewProjectModalController', ($scope, $modalInstance, $timeout, template) -> - $scope.inputs = + $scope.inputs = projectName: "" $scope.state = inflight: false @@ -123,7 +123,6 @@ define [ $scope.cancel = () -> $modalInstance.dismiss('cancel') - App.controller 'UploadProjectModalController', ($scope, $modalInstance, $timeout) -> $scope.cancel = () -> $modalInstance.dismiss('cancel') @@ -137,3 +136,8 @@ define [ $scope.dismiss = () -> $modalInstance.dismiss('cancel') + + App.controller 'ShowErrorModalController', ($scope, $modalInstance, error) -> + $scope.error = error + $scope.cancel = () -> + $modalInstance.dismiss('cancel') diff --git a/services/web/public/coffee/main/project-list/project-list.coffee b/services/web/public/coffee/main/project-list/project-list.coffee index 9a8d7f7a73..8bdec6f42a 100644 --- a/services/web/public/coffee/main/project-list/project-list.coffee +++ b/services/web/public/coffee/main/project-list/project-list.coffee @@ -13,7 +13,7 @@ define [ $scope.predicate = "lastUpdated" $scope.nUntagged = 0 $scope.reverse = true - $scope.searchText = + $scope.searchText = value : "" $timeout () -> @@ -37,7 +37,7 @@ define [ angular.element($window).bind "resize", () -> recalculateProjectListHeight() $scope.$apply() - + # Allow tags to be accessed on projects as well projectsById = {} for project in $scope.projects @@ -56,7 +56,7 @@ define [ tag.selected = true else tag.selected = false - + $scope.changePredicate = (newPredicate)-> if $scope.predicate == newPredicate $scope.reverse = !$scope.reverse @@ -145,7 +145,7 @@ define [ # We don't want hidden selections project.selected = false - localStorage("project_list", JSON.stringify({ + localStorage("project_list", JSON.stringify({ filter: $scope.filter, selectedTagId: selectedTag?._id })) @@ -461,7 +461,7 @@ define [ resolve: project: () -> project ) - + if storedUIOpts?.filter? if storedUIOpts.filter == "tag" and storedUIOpts.selectedTagId? markTagAsSelected(storedUIOpts.selectedTagId) @@ -505,7 +505,16 @@ define [ $scope.project.isTableActionInflight = true $scope.cloneProject($scope.project, "#{$scope.project.name} (Copy)") .then () -> $scope.project.isTableActionInflight = false - .catch () -> $scope.project.isTableActionInflight = false + .catch (response) -> + { data, status } = response + error = if status == 400 then message: data else true + modalInstance = $modal.open( + templateUrl: "showErrorModalTemplate" + controller: "ShowErrorModalController" + resolve: + error: () -> error + ) + $scope.project.isTableActionInflight = false $scope.download = (e) -> e.stopPropagation() @@ -535,11 +544,11 @@ define [ url: "/project/#{$scope.project.id}?forever=true" headers: "X-CSRF-Token": window.csrfToken - }).then () -> + }).then () -> $scope.project.isTableActionInflight = false $scope._removeProjectFromList $scope.project for tag in $scope.tags $scope._removeProjectIdsFromTagArray(tag, [ $scope.project.id ]) $scope.updateVisibleProjects() - .catch () -> + .catch () -> $scope.project.isTableActionInflight = false From 9dd965da3c82beb453d73598cd05ee426408f00b Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Wed, 10 Oct 2018 15:04:18 +0100 Subject: [PATCH 40/43] Prevent autocompile loop If an autocompile hits a rate limit, it would get into a bad state where it would continuely loop making compile requests. This is because the compile response resolving would disable auto compile (because the rate limit was hit) but also trigger calculation of uncompiled changes, which would not check if autocompile was disabled. The fix is to just check if autocompile is disabled. --- .../web/public/coffee/ide/pdf/controllers/PdfController.coffee | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 001178c5dd..acd6b4e349 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -125,6 +125,9 @@ define [ $scope.uncompiledChanges = false recalculateUncompiledChanges = () -> + if !$scope.autocompile_enabled + # Auto-compile was disabled + $scope.uncompiledChanges = false if $scope.ui.pdfHidden # Don't bother auto-compiling if pdf isn't visible $scope.uncompiledChanges = false From e7506489d1580fb61d9caf177f3d061c82f1c7c5 Mon Sep 17 00:00:00 2001 From: Simon Detheridge Date: Wed, 10 Oct 2018 15:04:17 +0100 Subject: [PATCH 41/43] Fix missing string in modals.pug Add escape to execute translate() server-side when handling error message for invalid filenames in project on copy. Signed-off-by: Simon Detheridge --- services/web/app/views/project/list/modals.pug | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/app/views/project/list/modals.pug b/services/web/app/views/project/list/modals.pug index 11e8da29ea..f89b26925c 100644 --- a/services/web/app/views/project/list/modals.pug +++ b/services/web/app/views/project/list/modals.pug @@ -127,7 +127,7 @@ script(type='text/ng-template', id='cloneProjectModalTemplate') ) × h3 #{translate("copy_project")} .modal-body - .alert.alert-danger(ng-show="state.error.message") {{state.error.message === "invalid element name" ? translate("invalid_element_name") : state.error.message}} + .alert.alert-danger(ng-show="state.error.message") {{state.error.message === "invalid element name" ? "#{translate("invalid_element_name")}" : state.error.message}} .alert.alert-danger(ng-show="state.error && !state.error.message") #{translate("generic_something_went_wrong")} form(name="cloneProjectForm", novalidate) .form-group @@ -271,7 +271,7 @@ script(type="text/ng-template", id="showErrorModalTemplate") ) × h3 #{translate("generic_something_went_wrong")} .modal-body - .alert.alert-danger(ng-show="error.message") {{error.message === "invalid element name" ? translate("invalid_element_name") : error.message}} + .alert.alert-danger(ng-show="error.message") {{error.message === "invalid element name" ? "#{translate("invalid_element_name")}" : error.message}} .alert.alert-danger(ng-show="error && !error.message") #{translate("generic_something_went_wrong")} .modal-footer button.btn.btn-default(ng-click="cancel()") #{translate("cancel")} From 96d7d83b90b2da6fb22ee56cc4c3d4c3ab84d286 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 11 Oct 2018 09:30:21 +0100 Subject: [PATCH 42/43] Add guard against autocompile being disabled --- .../web/public/coffee/ide/pdf/controllers/PdfController.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index acd6b4e349..040d536b48 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -71,7 +71,7 @@ define [ autoCompileInterval = null autoCompileIfReady = () -> - if $scope.pdf.compiling + if $scope.pdf.compiling or !$scope.autocompile_enabled return # Only checking linting if syntaxValidation is on and visible to the user From 16db3a080623ed08f8977072e7e1b87d4182fe54 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Tue, 9 Oct 2018 15:46:09 +0100 Subject: [PATCH 43/43] gracefully handle subscriptions without currency --- services/web/public/coffee/main/subscription-dashboard.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/public/coffee/main/subscription-dashboard.coffee b/services/web/public/coffee/main/subscription-dashboard.coffee index 28d5af013b..7ae01d1b52 100644 --- a/services/web/public/coffee/main/subscription-dashboard.coffee +++ b/services/web/public/coffee/main/subscription-dashboard.coffee @@ -36,7 +36,7 @@ define [ $scope.pricing = MultiCurrencyPricing # $scope.plans = MultiCurrencyPricing.plans - $scope.currencySymbol = MultiCurrencyPricing.plans[MultiCurrencyPricing.currencyCode].symbol + $scope.currencySymbol = MultiCurrencyPricing.plans[MultiCurrencyPricing.currencyCode]?.symbol $scope.currencyCode = MultiCurrencyPricing.currencyCode