diff options
22 files changed, 756 insertions, 242 deletions
diff --git a/.drone.yml b/.drone.yml index b210282efe7..3a0d5ade9ee 100644 --- a/.drone.yml +++ b/.drone.yml @@ -773,12 +773,12 @@ matrix: - TESTS: carddavtester-old-endpoint - TESTS: object-store OBJECT_STORE: s3 - - TESTS: object-store - OBJECT_STORE: swift - SWIFT-AUTH: v2.0 - - TESTS: object-store - OBJECT_STORE: swift - SWIFT-AUTH: v3 +# - TESTS: object-store +# OBJECT_STORE: swift +# SWIFT-AUTH: v2.0 +# - TESTS: object-store +# OBJECT_STORE: swift +# SWIFT-AUTH: v3 - TESTS: sqlite-php7.0-samba-native - TESTS: sqlite-php7.0-samba-non-native - TEST: memcache-memcached diff --git a/apps/comments/css/comments.css b/apps/comments/css/comments.scss index 311eeebe4db..c2bc136ba66 100644 --- a/apps/comments/css/comments.css +++ b/apps/comments/css/comments.scss @@ -13,53 +13,40 @@ } #commentsTabView .newCommentForm { - position: relative; - margin-bottom: 20px; + margin-left: 36px; } #commentsTabView .newCommentForm .message { - width: calc(100% - 81px); /* 36 (left margin) + 30 (right padding) + 15 (right padding of surrounding box) */ - margin-left: 36px; - padding-right: 30px; - display: block; + /* width = 100% - (width of submit button (44px) + margin (3px) + inline-block gap) */ + width: calc(100% - 52px); + display: inline-block; } #commentsTabView .newCommentForm .submit { - position: absolute; - bottom: 0px; - right: 8px; - width: 30px; + width: 44px; margin: 0; - padding: 7px 9px; + padding: 13px; background-color: transparent; border: none; opacity: .3; + vertical-align: bottom; } -#commentsTabView .newCommentForm .submit:not(:disabled):hover, -#commentsTabView .newCommentForm .submit:not(:disabled):focus { - opacity: 1; -} - -#commentsTabView .newCommentForm .submitLoading { - background-position: left; - /* Match rules for '#commentsTabView .newCommentForm .submit' to place the - loading icon at the same position as the confirm icon */ - position: absolute; - bottom: 0px; - right: 8px; - width: 30px; - margin: 0; - padding: 7px 9px; +#commentsTabView .deleteLoading { + float: right; + padding: 14px; + vertical-align: middle; +} - /* Match rules for 'input[type="submit"]' to place the loading icon at the - same position as the confirm icon */ - min-height: 34px; - box-sizing: border-box; +#commentsTabView .submitLoading { + vertical-align: bottom; + display: inline-block; + padding: 14px; } -#commentsTabView .newCommentForm .cancel { - margin-right: 6px; +#commentsTabView .newCommentForm .submit:not(:disabled):hover, +#commentsTabView .newCommentForm .submit:not(:disabled):focus { + opacity: 1; } #commentsTabView .newCommentForm div.message { @@ -73,11 +60,16 @@ #commentsTabView .comment { position: relative; - margin-bottom: 30px; + /** padding bottom is little more so that the top and bottom gap look uniform **/ + padding: 10px 0px 15px; word-wrap: break-word; overflow-wrap: break-word; } +#commentsTabView .comments .comment { + border-top: 1px solid $color-border; +} + #commentsTabView .comment .avatar, .atwho-view-ul * .avatar{ width: 32px; @@ -123,19 +115,22 @@ background: -o-linear-gradient(rgba(255,255,255,0), rgba(255,255,255,1)); background: -ms-linear-gradient(rgba(255,255,255,0), rgba(255,255,255,1)); background: linear-gradient(rgba(255,255,255,0), rgba(255,255,255,1)); - filter: progid:DXImageTransform.Microsoft.gradient(GradientType=0,startColorstr='#00FFFFFF', endColorstr='#FFFFFFFF'); background-repeat: no-repeat; } -#commentsTabView .authorRow>div:not(.contactsmenu-popover) { - display: inline-block; - vertical-align: middle; -} - -#commentsTabView .authorRow>div.hidden { +#commentsTabView .hidden { display: none !important; } +/** set min-height as 44px to ensure that it fits the button sizes. **/ +#commentsTabView .comment .authorRow { + min-height: 44px; +} +#commentsTabView .comment .authorRow .tooltip { + /** because of the padding on the element, the tooltip appear too far up, + adding this brings them closer to the element**/ + margin-top: 5px; +} #commentsTabView .comments li .message .avatar-name-wrapper, .atwho-view-ul * .avatar-name-wrapper, #commentsTabView .comment .authorRow { @@ -164,47 +159,48 @@ .atwho-view-ul * .avatar-name-wrapper { white-space: nowrap; } - #commentsTabView .comment .author, #commentsTabView .comment .date { opacity: .5; } +#commentsTabView .comment .author { + max-width: 210px; + text-overflow: ellipsis; + overflow: hidden; + white-space: nowrap; +} #commentsTabView .comment .date { margin-left: auto; + /** this is to fix the tooltip being too close due to the margin-top applied + to bring the tooltip closer for the action icons **/ + padding: 10px 0px; } -#commentsTabView .comments li .message { +#commentsTabView .comments > li:not(.newCommentRow) .message { padding-left: 40px; - display: inline-flex; - flex-wrap: wrap; - align-items: center; } #commentsTabView .comment .action { - opacity: 0; - padding: 5px; -} - -#commentsTabView .comment:hover .action { opacity: 0.3; + padding: 14px; + display: block; } -#commentsTabView .comment .action:hover { +#commentsTabView .comment .action:hover, +#commentsTabView .comment .action:focus { opacity: 1; } -#commentsTabView .comment .action.delete, -#commentsTabView .comment .deleteLoading { - position: absolute; - right: 0; +#commentsTabView .newCommentRow .action-container { + margin-left: auto; } -#commentsTabView .comment.disabled { +#commentsTabView .comment.disabled .message { opacity: 0.3; } #commentsTabView .comment.disabled .action { - visibility: hidden; + display: none; } #commentsTabView .message.error { @@ -215,4 +211,4 @@ .app-files .action-comment { padding: 16px 14px; -} +}
\ No newline at end of file diff --git a/apps/comments/js/commentsmodifymenu.js b/apps/comments/js/commentsmodifymenu.js new file mode 100644 index 00000000000..4b17cbbfbf0 --- /dev/null +++ b/apps/comments/js/commentsmodifymenu.js @@ -0,0 +1,124 @@ +/* + * Copyright (c) 2018 + * + * This file is licensed under the Affero General Public License version 3 + * or later. + * + * See the COPYING-README file. + * + */ + +/* global Handlebars */ +(function() { + var TEMPLATE_MENU = + '<ul>' + + '{{#each items}}' + + '<li>' + + '<a href="#" class="menuitem action {{name}} permanent" data-action="{{name}}">' + + '{{#if iconClass}}' + + '<span class="icon {{iconClass}}"></span>' + + '{{else}}' + + '<span class="no-icon"></span>' + + '{{/if}}' + + '<span>{{displayName}}</span>' + + '</li>' + + '{{/each}}' + + '</ul>'; + + /** + * Construct a new CommentsModifyMenuinstance + * @constructs CommentsModifyMenu + * @memberof OC.Comments + */ + var CommentsModifyMenu = OC.Backbone.View.extend({ + tagName: 'div', + className: 'commentsModifyMenu popovermenu bubble menu', + _scopes: [ + { + name: 'edit', + displayName: t('comments', 'Edit comment'), + iconClass: 'icon-rename' + }, + { + name: 'delete', + displayName: t('comments', 'Delete comment'), + iconClass: 'icon-delete' + } + ], + initialize: function() { + + }, + events: { + 'click a.action': '_onClickAction' + }, + + template: Handlebars.compile(TEMPLATE_MENU), + + /** + * Event handler whenever an action has been clicked within the menu + * + * @param {Object} event event object + */ + _onClickAction: function(event) { + var $target = $(event.currentTarget); + if (!$target.hasClass('menuitem')) { + $target = $target.closest('.menuitem'); + } + + OC.hideMenus(); + + this.trigger('select:menu-item-clicked', event, $target.data('action')); + }, + + /** + * Renders the menu with the currently set items + */ + render: function() { + this.$el.html(this.template({ + items: this._scopes + })); + }, + + /** + * Displays the menu + */ + show: function(context) { + this._context = context; + + for(var i in this._scopes) { + this._scopes[i].active = false; + } + + + var $el = $(context.target); + var offsetIcon = $el.offset(); + var offsetContainer = $el.closest('.authorRow').offset(); + + // adding some extra top offset to push the menu below the button. + var position = { + top: offsetIcon.top - offsetContainer.top + 48, + left: '', + right: '' + }; + + position.left = offsetIcon.left - offsetContainer.left; + + if (position.left > 200) { + // we need to position the menu to the right. + position.left = ''; + position.right = this.$el.closest('.comment').find('.date').width(); + this.$el.removeClass('menu-left').addClass('menu-right'); + } else { + this.$el.removeClass('menu-right').addClass('menu-left'); + } + this.$el.css(position); + this.render(); + this.$el.removeClass('hidden'); + + OC.showMenu(null, this.$el); + } + }); + + OCA.Comments = OCA.Comments || {}; + OCA.Comments.CommentsModifyMenu = CommentsModifyMenu; +})(OC, OCA);
\ No newline at end of file diff --git a/apps/comments/js/commentstabview.js b/apps/comments/js/commentstabview.js index 20f1f590a28..9477cb0c301 100644 --- a/apps/comments/js/commentstabview.js +++ b/apps/comments/js/commentstabview.js @@ -21,24 +21,22 @@ '<div class="loading hidden" style="height: 50px"></div>'; var EDIT_COMMENT_TEMPLATE = - '<div class="newCommentRow comment" data-id="{{id}}">' + + '<{{tag}} class="newCommentRow comment" data-id="{{id}}">' + ' <div class="authorRow">' + ' <div class="avatar currentUser" data-username="{{actorId}}"></div>' + ' <div class="author currentUser">{{actorDisplayName}}</div>' + '{{#if isEditMode}}' + - ' <a href="#" class="action delete icon icon-delete has-tooltip" title="{{deleteTooltip}}"></a>' + - ' <div class="deleteLoading icon-loading-small hidden"></div>'+ + ' <div class="action-container">' + + ' <a href="#" class="action cancel icon icon-close has-tooltip" title="{{cancelText}}"></a>' + + ' </div>' + '{{/if}}' + ' </div>' + ' <form class="newCommentForm">' + ' <div contentEditable="true" class="message" data-placeholder="{{newMessagePlaceholder}}">{{message}}</div>' + - ' <input class="submit icon-confirm" type="submit" value="" />' + - '{{#if isEditMode}}' + - ' <input class="cancel pull-right" type="button" value="{{cancelText}}" />' + - '{{/if}}' + + ' <input class="submit icon-confirm has-tooltip" type="submit" value="" title="{{submitText}}"/>' + ' <div class="submitLoading icon-loading-small hidden"></div>'+ ' </form>' + - '</div>'; + '</{{tag}}>'; var COMMENT_TEMPLATE = '<li class="comment{{#if isUnread}} unread{{/if}}{{#if isLong}} collapsed{{/if}}" data-id="{{id}}">' + @@ -46,7 +44,8 @@ ' <div class="avatar{{#if isUserAuthor}} currentUser{{/if}}" {{#if actorId}}data-username="{{actorId}}"{{/if}}> </div>' + ' <div class="author{{#if isUserAuthor}} currentUser{{/if}}">{{actorDisplayName}}</div>' + '{{#if isUserAuthor}}' + - ' <a href="#" class="action edit icon icon-rename has-tooltip" title="{{editTooltip}}"></a>' + + ' <a href="#" class="action more icon icon-more has-tooltip"></a>' + + ' <div class="deleteLoading icon-loading-small hidden"></div>' + '{{/if}}' + ' <div class="date has-tooltip live-relative-timestamp" data-timestamp="{{timestamp}}" title="{{altDate}}">{{date}}</div>' + ' </div>' + @@ -64,12 +63,11 @@ id: 'commentsTabView', className: 'tab commentsTabView', _autoCompleteData: undefined, + _commentsModifyMenu: undefined, events: { 'submit .newCommentForm': '_onSubmitComment', 'click .showMore': '_onClickShowMore', - 'click .action.edit': '_onClickEditComment', - 'click .action.delete': '_onClickDeleteComment', 'click .cancel': '_onClickCloseComment', 'click .comment': '_onClickComment', 'keyup div.message': '_onTextChange', @@ -114,9 +112,9 @@ actorId: currentUser.uid, actorDisplayName: currentUser.displayName, newMessagePlaceholder: t('comments', 'New comment …'), - deleteTooltip: t('comments', 'Delete comment'), submitText: t('comments', 'Post'), - cancelText: t('comments', 'Cancel') + cancelText: t('comments', 'Cancel'), + tag: 'li' }, params)); }, @@ -166,7 +164,7 @@ emptyResultLabel: t('comments', 'No comments yet, start the conversation!'), moreLabel: t('comments', 'More comments …') })); - this.$el.find('.comments').before(this.editCommentTemplate({})); + this.$el.find('.comments').before(this.editCommentTemplate({ tag: 'div'})); this.$el.find('.has-tooltip').tooltip(); this.$container = this.$el.find('ul.comments'); this.$el.find('.avatar').avatar(OC.getCurrentUser().uid, 32); @@ -401,6 +399,24 @@ // it is the case when writing a comment and mentioning a person $message = $el; } + + + if (!editionMode) { + var self = this; + // add the dropdown menu to display the edit and delete option + var modifyCommentMenu = new OCA.Comments.CommentsModifyMenu(); + $el.find('.authorRow').append(modifyCommentMenu.$el); + $el.find('.more').on('click', _.bind(modifyCommentMenu.show, modifyCommentMenu)); + + self.listenTo(modifyCommentMenu, 'select:menu-item-clicked', function(ev, action) { + if (action === 'edit') { + self._onClickEditComment(ev); + } else if (action === 'delete') { + self._onClickDeleteComment(ev); + } + }); + } + this._postRenderMessage($message, editionMode); }, @@ -567,15 +583,13 @@ var $comment = $(ev.target).closest('.comment'); var commentId = $comment.data('id'); var $loading = $comment.find('.deleteLoading'); - var $commentField = $comment.find('.message'); - var $submit = $comment.find('.submit'); - var $cancel = $comment.find('.cancel'); + var $moreIcon = $comment.find('.more'); - $commentField.prop('contenteditable', false); - $submit.prop('disabled', true); - $cancel.prop('disabled', true); $comment.addClass('disabled'); $loading.removeClass('hidden'); + $moreIcon.addClass('hidden'); + + $comment.data('commentEl', $comment); this.collection.get(commentId).destroy({ success: function() { @@ -584,10 +598,8 @@ }, error: function() { $loading.addClass('hidden'); + $moreIcon.removeClass('hidden'); $comment.removeClass('disabled'); - $commentField.prop('contenteditable', true); - $submit.prop('disabled', false); - $cancel.prop('disabled', false); OC.Notification.showTemporary(t('comments', 'Error occurred while retrieving comment with ID {id}', {id: commentId})); } @@ -668,6 +680,12 @@ }, { success: function(model) { self._onSubmitSuccess(model, $form); + if(model.get('message').trim() === model.previous('message').trim()) { + // model change event doesn't trigger, manually remove the row. + var $row = $form.closest('.comment'); + $row.data('commentEl').removeClass('hidden'); + $row.remove(); + } }, error: function() { self._onSubmitError($form, commentId); diff --git a/apps/comments/js/merged.json b/apps/comments/js/merged.json index 0202c7ff55a..6e77d9cf80a 100644 --- a/apps/comments/js/merged.json +++ b/apps/comments/js/merged.json @@ -4,6 +4,7 @@ "commentcollection.js", "commentsummarymodel.js", "commentstabview.js", + "commentsmodifymenu.js", "filesplugin.js", "activitytabviewplugin.js", "vendor/Caret.js/dist/jquery.caret.min.js", diff --git a/apps/comments/tests/js/commentstabviewSpec.js b/apps/comments/tests/js/commentstabviewSpec.js index 0131bc7bce3..c90ad04e419 100644 --- a/apps/comments/tests/js/commentstabviewSpec.js +++ b/apps/comments/tests/js/commentstabviewSpec.js @@ -190,7 +190,7 @@ describe('OCA.Comments.CommentsTabView tests', function() { expect(fetchStub.notCalled).toEqual(true); - view.$el.find('.showMore').click(); + view.$el.find('.showMore').trigger('click'); expect(fetchStub.calledOnce).toEqual(true); }); @@ -398,10 +398,10 @@ describe('OCA.Comments.CommentsTabView tests', function() { $message = $newCommentForm.find('.message'); $submitButton = $newCommentForm.find('.submit'); }); - afterEach(function() { - tooltipStub.restore(); + afterEach(function() { + tooltipStub.restore(); }); - + it('does not displays tooltip when limit is far away', function() { $message.val(createMessageWithLength(3)); $message.trigger('change'); @@ -490,18 +490,21 @@ describe('OCA.Comments.CommentsTabView tests', function() { it('shows edit link for owner comments', function() { var $comment = view.$el.find('.comment[data-id=1]'); expect($comment.length).toEqual(1); + $comment.find('.action.more').trigger('click'); expect($comment.find('.action.edit').length).toEqual(1); }); it('does not show edit link for other user\'s comments', function() { var $comment = view.$el.find('.comment[data-id=2]'); expect($comment.length).toEqual(1); + $comment.find('.action.more').trigger('click'); expect($comment.find('.action.edit').length).toEqual(0); }); it('shows edit form when clicking edit', function() { var $comment = view.$el.find('.comment[data-id=1]'); - $comment.find('.action.edit').click(); + $comment.find('.action.more').trigger('click'); + $comment.find('.action.edit').trigger('click'); expect($comment.hasClass('hidden')).toEqual(true); var $formRow = view.$el.find('.newCommentRow.comment[data-id=1]'); @@ -510,7 +513,8 @@ describe('OCA.Comments.CommentsTabView tests', function() { it('saves message and updates comment item when clicking save', function() { var $comment = view.$el.find('.comment[data-id=1]'); - $comment.find('.action.edit').click(); + $comment.find('.action.more').trigger('click'); + $comment.find('.action.edit').trigger('click'); var $formRow = view.$el.find('.newCommentRow.comment[data-id=1]'); expect($formRow.length).toEqual(1); @@ -544,7 +548,8 @@ describe('OCA.Comments.CommentsTabView tests', function() { it('saves message and updates comment item with mentions when clicking save', function() { var $comment = view.$el.find('.comment[data-id=3]'); - $comment.find('.action.edit').click(); + $comment.find('.action.more').trigger('click'); + $comment.find('.action.edit').trigger('click'); var $formRow = view.$el.find('.newCommentRow.comment[data-id=3]'); expect($formRow.length).toEqual(1); @@ -591,13 +596,14 @@ describe('OCA.Comments.CommentsTabView tests', function() { it('restores original comment when cancelling', function() { var $comment = view.$el.find('.comment[data-id=1]'); - $comment.find('.action.edit').click(); + $comment.find('.action.more').trigger('click'); + $comment.find('.action.edit').trigger('click'); var $formRow = view.$el.find('.newCommentRow.comment[data-id=1]'); expect($formRow.length).toEqual(1); $formRow.find('textarea').val('modified\nmessage'); - $formRow.find('.cancel').click(); + $formRow.find('.cancel').trigger('click'); expect(saveStub.notCalled).toEqual(true); @@ -614,12 +620,8 @@ describe('OCA.Comments.CommentsTabView tests', function() { it('destroys model when clicking delete', function() { var destroyStub = sinon.stub(OCA.Comments.CommentModel.prototype, 'destroy'); var $comment = view.$el.find('.comment[data-id=1]'); - $comment.find('.action.edit').click(); - - var $formRow = view.$el.find('.newCommentRow.comment[data-id=1]'); - expect($formRow.length).toEqual(1); - - $formRow.find('.delete').click(); + $comment.find('.action.more').trigger('click'); + $comment.find('.action.delete').trigger('click'); expect(destroyStub.calledOnce).toEqual(true); expect(destroyStub.thisValues[0].id).toEqual(1); @@ -630,15 +632,11 @@ describe('OCA.Comments.CommentsTabView tests', function() { $comment = view.$el.find('.comment[data-id=1]'); expect($comment.length).toEqual(0); - // form row is gone - $formRow = view.$el.find('.newCommentRow.comment[data-id=1]'); - expect($formRow.length).toEqual(0); - destroyStub.restore(); }); it('does not submit comment if the field is empty', function() { var $comment = view.$el.find('.comment[data-id=1]'); - $comment.find('.action.edit').click(); + $comment.find('.action.edit').trigger('click'); $comment.find('.message').val(' '); $comment.find('form').submit(); @@ -646,7 +644,7 @@ describe('OCA.Comments.CommentsTabView tests', function() { }); it('does not submit comment if the field length is too large', function() { var $comment = view.$el.find('.comment[data-id=1]'); - $comment.find('.action.edit').click(); + $comment.find('.action.edit').trigger('click'); $comment.find('.message').val(createMessageWithLength(view._commentMaxLength * 2)); $comment.find('form').submit(); @@ -659,7 +657,7 @@ describe('OCA.Comments.CommentsTabView tests', function() { beforeEach(function() { updateMarkerStub = sinon.stub(OCA.Comments.CommentCollection.prototype, 'updateReadMarker'); }); - afterEach(function() { + afterEach(function() { updateMarkerStub.restore(); }); diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index 290d6027006..b28c8534aaa 100644 --- a/apps/dav/lib/CalDAV/CalDavBackend.php +++ b/apps/dav/lib/CalDAV/CalDavBackend.php @@ -2251,7 +2251,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription if (!$this->db->supports4ByteText()) { $value = preg_replace('/[\x{10000}-\x{10FFFF}]/u', "\xEF\xBF\xBD", $value); } - $value = substr($value, 0, 254); + $value = mb_substr($value, 0, 254); $query->setParameter('name', $property->name); $query->setParameter('parameter', null); @@ -2269,7 +2269,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription if ($this->db->supports4ByteText()) { $value = preg_replace('/[\x{10000}-\x{10FFFF}]/u', "\xEF\xBF\xBD", $value); } - $value = substr($value, 0, 254); + $value = mb_substr($value, 0, 254); $query->setParameter('name', $property->name); $query->setParameter('parameter', substr($key, 0, 254)); diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 6a467e9eff2..e577510b69e 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -332,7 +332,11 @@ class File extends Node implements IFile { // do a if the file did not exist throw new NotFound(); } - $res = $this->fileView->fopen(ltrim($this->path, '/'), 'rb'); + try { + $res = $this->fileView->fopen(ltrim($this->path, '/'), 'rb'); + } catch (\Exception $e) { + $this->convertToSabreException($e); + } if ($res === false) { throw new ServiceUnavailable("Could not open file"); } diff --git a/apps/dav/tests/unit/Connector/Sabre/ExceptionLoggerPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/ExceptionLoggerPluginTest.php index ff928ccede4..c1d48a7ce5d 100644 --- a/apps/dav/tests/unit/Connector/Sabre/ExceptionLoggerPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/ExceptionLoggerPluginTest.php @@ -24,6 +24,7 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre; +use OC\SystemConfig; use OCA\DAV\Connector\Sabre\Exception\InvalidPath; use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin as PluginToTest; use OC\Log; @@ -37,13 +38,9 @@ class TestLogger extends Log { public $message; public $level; - public function __construct($logger = null) { - //disable original constructor - } - - public function log(int $level, string $message, array $context = array()) { + public function writeLog(string $app, $entry, int $level) { $this->level = $level; - $this->message = $message; + $this->message = $entry; } } @@ -59,8 +56,20 @@ class ExceptionLoggerPluginTest extends TestCase { private $logger; private function init() { + $config = $this->createMock(SystemConfig::class); + $config->expects($this->any()) + ->method('getValue') + ->willReturnCallback(function($key, $default) { + switch ($key) { + case 'loglevel': + return 0; + default: + return $default; + } + }); + $this->server = new Server(); - $this->logger = new TestLogger(); + $this->logger = new TestLogger(Log\File::class, $config); $this->plugin = new PluginToTest('unit-test', $this->logger); $this->plugin->initialize($this->server); } @@ -73,7 +82,8 @@ class ExceptionLoggerPluginTest extends TestCase { $this->plugin->logException($exception); $this->assertEquals($expectedLogLevel, $this->logger->level); - $this->assertStringStartsWith('Exception: {"Exception":' . json_encode(get_class($exception)) . ',"Message":"' . $expectedMessage . '",', $this->logger->message); + $this->assertEquals(get_class($exception), $this->logger->message['Exception']); + $this->assertEquals($expectedMessage, $this->logger->message['Message']); } public function providesExceptions() { diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 420c09dfecb..7bbbe554838 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -206,7 +206,8 @@ class UsersController extends AUserData { string $email = '', array $groups = [], array $subadmin = [], - string $quota = ''): DataResponse { + string $quota = '', + string $language = 'en'): DataResponse { $user = $this->userSession->getUser(); $isAdmin = $this->groupManager->isAdmin($user->getUID()); $subAdminManager = $this->groupManager->getSubAdmin(); @@ -279,6 +280,10 @@ class UsersController extends AUserData { $this->editUser($userid, 'quota', $quota); } + if ($language !== '') { + $this->editUser($userid, 'language', $language); + } + // Send new user mail only if a mail is set if ($email !== '') { $newUser->setEMailAddress($email); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 76c1c0f7eef..167e3033bca 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -381,6 +381,7 @@ return array( 'OC\\App\\CodeChecker\\NodeVisitor' => $baseDir . '/lib/private/App/CodeChecker/NodeVisitor.php', 'OC\\App\\CodeChecker\\PrivateCheck' => $baseDir . '/lib/private/App/CodeChecker/PrivateCheck.php', 'OC\\App\\CodeChecker\\StrongComparisonCheck' => $baseDir . '/lib/private/App/CodeChecker/StrongComparisonCheck.php', + 'OC\\App\\CompareVersion' => $baseDir . '/lib/private/App/CompareVersion.php', 'OC\\App\\DependencyAnalyzer' => $baseDir . '/lib/private/App/DependencyAnalyzer.php', 'OC\\App\\InfoParser' => $baseDir . '/lib/private/App/InfoParser.php', 'OC\\App\\Platform' => $baseDir . '/lib/private/App/Platform.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 73c3bbf4d05..4bf613d1256 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -411,6 +411,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\App\\CodeChecker\\NodeVisitor' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/NodeVisitor.php', 'OC\\App\\CodeChecker\\PrivateCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/PrivateCheck.php', 'OC\\App\\CodeChecker\\StrongComparisonCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/StrongComparisonCheck.php', + 'OC\\App\\CompareVersion' => __DIR__ . '/../../..' . '/lib/private/App/CompareVersion.php', 'OC\\App\\DependencyAnalyzer' => __DIR__ . '/../../..' . '/lib/private/App/DependencyAnalyzer.php', 'OC\\App\\InfoParser' => __DIR__ . '/../../..' . '/lib/private/App/InfoParser.php', 'OC\\App\\Platform' => __DIR__ . '/../../..' . '/lib/private/App/Platform.php', diff --git a/lib/private/App/AppStore/Fetcher/AppFetcher.php b/lib/private/App/AppStore/Fetcher/AppFetcher.php index 6c6fa68429f..5203515e09b 100644 --- a/lib/private/App/AppStore/Fetcher/AppFetcher.php +++ b/lib/private/App/AppStore/Fetcher/AppFetcher.php @@ -27,24 +27,32 @@ namespace OC\App\AppStore\Fetcher; use OC\App\AppStore\Version\VersionParser; +use OC\App\CompareVersion; use OC\Files\AppData\Factory; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\ILogger; +use OCP\Util; class AppFetcher extends Fetcher { + + /** @var CompareVersion */ + private $compareVersion; + /** * @param Factory $appDataFactory * @param IClientService $clientService * @param ITimeFactory $timeFactory * @param IConfig $config + * @param CompareVersion $compareVersion * @param ILogger $logger */ public function __construct(Factory $appDataFactory, IClientService $clientService, ITimeFactory $timeFactory, IConfig $config, + CompareVersion $compareVersion, ILogger $logger) { parent::__construct( $appDataFactory, @@ -56,6 +64,7 @@ class AppFetcher extends Fetcher { $this->fileName = 'apps.json'; $this->setEndpoint(); + $this->compareVersion = $compareVersion; } /** @@ -70,8 +79,6 @@ class AppFetcher extends Fetcher { /** @var mixed[] $response */ $response = parent::fetch($ETag, $content); - $ncVersion = $this->getVersion(); - $ncMajorVersion = explode('.', $ncVersion)[0]; foreach($response['data'] as $dataKey => $app) { $releases = []; @@ -81,15 +88,20 @@ class AppFetcher extends Fetcher { if($release['isNightly'] === false && strpos($release['version'], '-') === false) { // Exclude all versions not compatible with the current version - $versionParser = new VersionParser(); - $version = $versionParser->getVersion($release['rawPlatformVersionSpec']); - if ( - // Major version is bigger or equals to the minimum version of the app - version_compare($ncMajorVersion, $version->getMinimumVersion(), '>=') - // Major version is smaller or equals to the maximum version of the app - && version_compare($ncMajorVersion, $version->getMaximumVersion(), '<=') - ) { - $releases[] = $release; + try { + $versionParser = new VersionParser(); + $version = $versionParser->getVersion($release['rawPlatformVersionSpec']); + $ncVersion = $this->getVersion(); + $min = $version->getMinimumVersion(); + $max = $version->getMaximumVersion(); + $minFulfilled = $this->compareVersion->isCompatible($ncVersion, $min, '>='); + $maxFulfilled = $max !== '' && + $this->compareVersion->isCompatible($ncVersion, $max, '<='); + if ($minFulfilled && $maxFulfilled) { + $releases[] = $release; + } + } catch (\InvalidArgumentException $e) { + $this->logger->logException($e, ['app' => 'appstoreFetcher', 'level' => Util::WARN]); } } } diff --git a/lib/private/App/CompareVersion.php b/lib/private/App/CompareVersion.php new file mode 100644 index 00000000000..ee25a8b9460 --- /dev/null +++ b/lib/private/App/CompareVersion.php @@ -0,0 +1,97 @@ +<?php + +/** + * @copyright 2018 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2018 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +namespace OC\App; + +use InvalidArgumentException; + +class CompareVersion { + + const REGEX_MAJOR = '/^\d+$/'; + const REGEX_MAJOR_MINOR = '/^\d+.\d+$/'; + const REGEX_MAJOR_MINOR_PATCH = '/^\d+.\d+.\d+$/'; + const REGEX_SERVER = '/^\d+.\d+.\d+(.\d+)?$/'; + + /** + * Checks if the given server version fulfills the given (app) version requirements. + * + * Version requirements can be 'major.minor.patch', 'major.minor' or just 'major', + * so '13.0.1', '13.0' and '13' are valid. + * + * @param string $actual version as major.minor.patch notation + * @param string $required version where major is requried and minor and patch are optional + * @param string $comparator passed to `version_compare` + * @return bool whether the requirement is fulfilled + * @throws InvalidArgumentException if versions specified in an invalid format + */ + public function isCompatible(string $actual, string $required, + string $comparator = '>='): bool { + + if (!preg_match(self::REGEX_SERVER, $actual)) { + throw new InvalidArgumentException('server version is invalid'); + } + + if (preg_match(self::REGEX_MAJOR, $required) === 1) { + return $this->compareMajor($actual, $required, $comparator); + } else if (preg_match(self::REGEX_MAJOR_MINOR, $required) === 1) { + return $this->compareMajorMinor($actual, $required, $comparator); + } else if (preg_match(self::REGEX_MAJOR_MINOR_PATCH, $required) === 1) { + return $this->compareMajorMinorPatch($actual, $required, $comparator); + } else { + throw new InvalidArgumentException('required version is invalid'); + } + } + + private function compareMajor(string $actual, string $required, + string $comparator) { + $actualMajor = explode('.', $actual)[0]; + $requiredMajor = explode('.', $required)[0]; + + return version_compare($actualMajor, $requiredMajor, $comparator); + } + + private function compareMajorMinor(string $actual, string $required, + string $comparator) { + $actualMajor = explode('.', $actual)[0]; + $actualMinor = explode('.', $actual)[1]; + $requiredMajor = explode('.', $required)[0]; + $requiredMinor = explode('.', $required)[1]; + + return version_compare("$actualMajor.$actualMinor", + "$requiredMajor.$requiredMinor", $comparator); + } + + private function compareMajorMinorPatch($actual, $required, $comparator) { + $actualMajor = explode('.', $actual)[0]; + $actualMinor = explode('.', $actual)[1]; + $actualPatch = explode('.', $actual)[2]; + $requiredMajor = explode('.', $required)[0]; + $requiredMinor = explode('.', $required)[1]; + $requiredPatch = explode('.', $required)[2]; + + return version_compare("$actualMajor.$actualMinor.$actualPatch", + "$requiredMajor.$requiredMinor.$requiredPatch", $comparator); + } + +} diff --git a/lib/private/Log.php b/lib/private/Log.php index e47f68807d3..85e439359fa 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -37,6 +37,7 @@ namespace OC; use InterfaSys\LogNormalizer\Normalizer; +use OC\Log\File; use OCP\ILogger; use OCP\Support\CrashReport\IRegistry; use OCP\Util; @@ -50,7 +51,6 @@ use OCP\Util; * * MonoLog is an example implementing this interface. */ - class Log implements ILogger { /** @var string */ @@ -78,7 +78,7 @@ class Log implements ILogger { 'updatePrivateKeyPassword', 'validateUserPass', 'loginWithToken', - '\{closure\}', + '{closure}', // TokenProvider 'getToken', @@ -120,14 +120,14 @@ class Log implements ILogger { */ public function __construct($logger = null, SystemConfig $config = null, $normalizer = null, IRegistry $registry = null) { // FIXME: Add this for backwards compatibility, should be fixed at some point probably - if($config === null) { + if ($config === null) { $config = \OC::$server->getSystemConfig(); } $this->config = $config; // FIXME: Add this for backwards compatibility, should be fixed at some point probably - if($logger === null) { + if ($logger === null) { $logType = $this->config->getValue('log_type', 'file'); $this->logger = static::getLogClass($logType); call_user_func([$this->logger, 'init']); @@ -251,61 +251,50 @@ class Log implements ILogger { * @return void */ public function log(int $level, string $message, array $context = []) { - $minLevel = min($this->config->getValue('loglevel', Util::WARN), Util::FATAL); - $logCondition = $this->config->getValue('log.condition', []); + $minLevel = $this->getLogLevel($context); array_walk($context, [$this->normalizer, 'format']); - if (isset($context['app'])) { - $app = $context['app']; + $app = $context['app'] ?? 'no app in context'; - /** - * check log condition based on the context of each log message - * once this is met -> change the required log level to debug - */ - if(!empty($logCondition) - && isset($logCondition['apps']) - && in_array($app, $logCondition['apps'], true)) { - $minLevel = Util::DEBUG; - } - - } else { - $app = 'no app in context'; - } // interpolate $message as defined in PSR-3 $replace = []; foreach ($context as $key => $val) { $replace['{' . $key . '}'] = $val; } - - // interpolate replacement values into the message and return $message = strtr($message, $replace); + if ($level >= $minLevel) { + $this->writeLog($app, $message, $level); + } + } + + private function getLogLevel($context) { /** * check for a special log condition - this enables an increased log on * a per request/user base */ - if($this->logConditionSatisfied === null) { + if ($this->logConditionSatisfied === null) { // default to false to just process this once per request $this->logConditionSatisfied = false; - if(!empty($logCondition)) { + if (!empty($logCondition)) { // check for secret token in the request - if(isset($logCondition['shared_secret'])) { + if (isset($logCondition['shared_secret'])) { $request = \OC::$server->getRequest(); // if token is found in the request change set the log condition to satisfied - if($request && hash_equals($logCondition['shared_secret'], $request->getParam('log_secret', ''))) { + if ($request && hash_equals($logCondition['shared_secret'], $request->getParam('log_secret', ''))) { $this->logConditionSatisfied = true; } } // check for user - if(isset($logCondition['users'])) { + if (isset($logCondition['users'])) { $user = \OC::$server->getUserSession()->getUser(); // if the user matches set the log condition to satisfied - if($user !== null && in_array($user->getUID(), $logCondition['users'], true)) { + if ($user !== null && in_array($user->getUID(), $logCondition['users'], true)) { $this->logConditionSatisfied = true; } } @@ -313,45 +302,104 @@ class Log implements ILogger { } // if log condition is satisfied change the required log level to DEBUG - if($this->logConditionSatisfied) { - $minLevel = Util::DEBUG; + if ($this->logConditionSatisfied) { + return Util::DEBUG; } - if ($level >= $minLevel) { - $logger = $this->logger; - call_user_func([$logger, 'write'], $app, $message, $level); + if (isset($context['app'])) { + $logCondition = $this->config->getValue('log.condition', []); + $app = $context['app']; + + /** + * check log condition based on the context of each log message + * once this is met -> change the required log level to debug + */ + if (!empty($logCondition) + && isset($logCondition['apps']) + && in_array($app, $logCondition['apps'], true)) { + return Util::DEBUG; + } } + + return min($this->config->getValue('loglevel', Util::WARN), Util::FATAL); } - /** - * Logs an exception very detailed - * - * @param \Exception|\Throwable $exception - * @param array $context - * @return void - * @since 8.2.0 - */ - public function logException(\Throwable $exception, array $context = []) { - $level = Util::ERROR; - if (isset($context['level'])) { - $level = $context['level']; - unset($context['level']); + private function filterTrace(array $trace) { + $sensitiveValues = []; + $trace = array_map(function (array $traceLine) use (&$sensitiveValues) { + foreach ($this->methodsWithSensitiveParameters as $sensitiveMethod) { + if (strpos($traceLine['function'], $sensitiveMethod) !== false) { + $sensitiveValues = array_merge($sensitiveValues, $traceLine['args']); + $traceLine['args'] = ['*** sensitive parameters replaced ***']; + return $traceLine; + } + } + return $traceLine; + }, $trace); + return array_map(function (array $traceLine) use ($sensitiveValues) { + $traceLine['args'] = $this->removeValuesFromArgs($traceLine['args'], $sensitiveValues); + return $traceLine; + }, $trace); + } + + private function removeValuesFromArgs($args, $values) { + foreach($args as &$arg) { + if (in_array($arg, $values, true)) { + $arg = '*** sensitive parameter replaced ***'; + } else if (is_array($arg)) { + $arg = $this->removeValuesFromArgs($arg, $values); + } } + return $args; + } + + private function serializeException(\Throwable $exception) { $data = [ 'Exception' => get_class($exception), 'Message' => $exception->getMessage(), 'Code' => $exception->getCode(), - 'Trace' => $exception->getTraceAsString(), + 'Trace' => $this->filterTrace($exception->getTrace()), 'File' => $exception->getFile(), 'Line' => $exception->getLine(), ]; - $data['Trace'] = preg_replace('!(' . implode('|', $this->methodsWithSensitiveParameters) . ')\(.*\)!', '$1(*** sensitive parameters replaced ***)', $data['Trace']); + if ($exception instanceof HintException) { $data['Hint'] = $exception->getHint(); } - $msg = isset($context['message']) ? $context['message'] : 'Exception'; - $msg .= ': ' . json_encode($data); - $this->log($level, $msg, $context); + + if ($exception->getPrevious()) { + $data['Previous'] = $this->serializeException($exception->getPrevious()); + } + + return $data; + } + + /** + * Logs an exception very detailed + * + * @param \Exception|\Throwable $exception + * @param array $context + * @return void + * @since 8.2.0 + */ + public function logException(\Throwable $exception, array $context = []) { + $app = $context['app'] ?? 'no app in context'; + $level = $context['level'] ?? Util::ERROR; + + $data = $this->serializeException($exception); + $data['CustomMessage'] = $context['message'] ?? '--'; + + $minLevel = $this->getLogLevel($context); + + array_walk($context, [$this->normalizer, 'format']); + + if ($level >= $minLevel) { + if ($this->logger !== File::class) { + $data = json_encode($data, JSON_PARTIAL_OUTPUT_ON_ERROR); + } + $this->writeLog($app, $data, $level); + } + $context['level'] = $level; if (!is_null($this->crashReporters)) { $this->crashReporters->delegateReport($exception, $context); @@ -359,6 +407,15 @@ class Log implements ILogger { } /** + * @param string $app + * @param string|array $entry + * @param int $level + */ + protected function writeLog(string $app, $entry, int $level) { + call_user_func([$this->logger, 'write'], $app, $entry, $level); + } + + /** * @param string $logType * @return string * @internal diff --git a/lib/private/Log/File.php b/lib/private/Log/File.php index ba5027251d7..2d7e4b6c14f 100644 --- a/lib/private/Log/File.php +++ b/lib/private/Log/File.php @@ -72,7 +72,7 @@ class File { /** * write a message in the log * @param string $app - * @param string $message + * @param string|array $message * @param int $level */ public static function write($app, $message, $level) { diff --git a/lib/private/Security/Hasher.php b/lib/private/Security/Hasher.php index c6c9109b336..e20de729f4f 100644 --- a/lib/private/Security/Hasher.php +++ b/lib/private/Security/Hasher.php @@ -51,11 +51,9 @@ class Hasher implements IHasher { /** @var IConfig */ private $config; /** @var array Options passed to password_hash and password_needs_rehash */ - private $options = array(); + private $options = []; /** @var string Salt used for legacy passwords */ private $legacySalt = null; - /** @var int Current version of the generated hash */ - private $currentVersion = 1; /** * @param IConfig $config @@ -78,7 +76,11 @@ class Hasher implements IHasher { * @return string Hash of the message with appended version parameter */ public function hash(string $message): string { - return $this->currentVersion . '|' . password_hash($message, PASSWORD_DEFAULT, $this->options); + if (\defined('PASSWORD_ARGON2I')) { + return 2 . '|' . password_hash($message, PASSWORD_ARGON2I, $this->options); + } else { + return 1 . '|' . password_hash($message, PASSWORD_BCRYPT, $this->options); + } } /** @@ -90,7 +92,7 @@ class Hasher implements IHasher { $explodedString = explode('|', $prefixedHash, 2); if(\count($explodedString) === 2) { if((int)$explodedString[0] > 0) { - return array('version' => (int)$explodedString[0], 'hash' => $explodedString[1]); + return ['version' => (int)$explodedString[0], 'hash' => $explodedString[1]]; } } @@ -111,8 +113,8 @@ class Hasher implements IHasher { // Verify whether it matches a legacy PHPass or SHA1 string $hashLength = \strlen($hash); - if($hashLength === 60 && password_verify($message.$this->legacySalt, $hash) || - $hashLength === 40 && hash_equals($hash, sha1($message))) { + if(($hashLength === 60 && password_verify($message.$this->legacySalt, $hash)) || + ($hashLength === 40 && hash_equals($hash, sha1($message)))) { $newHash = $this->hash($message); return true; } @@ -121,7 +123,7 @@ class Hasher implements IHasher { } /** - * Verify V1 hashes + * Verify V1 (blowfish) hashes * @param string $message Message to verify * @param string $hash Assumed hash of the message * @param null|string &$newHash Reference will contain the updated hash if necessary. Update the existing hash with this one. @@ -129,7 +131,30 @@ class Hasher implements IHasher { */ protected function verifyHashV1(string $message, string $hash, &$newHash = null): bool { if(password_verify($message, $hash)) { - if(password_needs_rehash($hash, PASSWORD_DEFAULT, $this->options)) { + $algo = PASSWORD_BCRYPT; + if (\defined('PASSWORD_ARGON2I')) { + $algo = PASSWORD_ARGON2I; + } + + if(password_needs_rehash($hash, $algo, $this->options)) { + $newHash = $this->hash($message); + } + return true; + } + + return false; + } + + /** + * Verify V2 (argon2i) hashes + * @param string $message Message to verify + * @param string $hash Assumed hash of the message + * @param null|string &$newHash Reference will contain the updated hash if necessary. Update the existing hash with this one. + * @return bool Whether $hash is a valid hash of $message + */ + protected function verifyHashV2(string $message, string $hash, &$newHash = null) : bool { + if(password_verify($message, $hash)) { + if(password_needs_rehash($hash, PASSWORD_ARGON2I, $this->options)) { $newHash = $this->hash($message); } return true; @@ -149,6 +174,8 @@ class Hasher implements IHasher { if(isset($splittedHash['version'])) { switch ($splittedHash['version']) { + case 2: + return $this->verifyHashV2($message, $splittedHash['hash'], $newHash); case 1: return $this->verifyHashV1($message, $splittedHash['hash'], $newHash); } diff --git a/tests/karma.config.js b/tests/karma.config.js index 0254d6a3335..3758636f074 100644 --- a/tests/karma.config.js +++ b/tests/karma.config.js @@ -93,6 +93,7 @@ module.exports = function(config) { 'apps/comments/js/commentmodel.js', 'apps/comments/js/commentcollection.js', 'apps/comments/js/commentsummarymodel.js', + 'apps/comments/js/commentsmodifymenu.js', 'apps/comments/js/commentstabview.js', 'apps/comments/js/filesplugin.js' ], @@ -223,7 +224,7 @@ module.exports = function(config) { // serve images to avoid warnings files.push({pattern: 'core/img/**/*', watched: false, included: false, served: true}); files.push({pattern: 'core/css/images/*', watched: false, included: false, served: true}); - + // include core CSS files.push({pattern: 'core/css/*.css', watched: true, included: true, served: true}); files.push({pattern: 'tests/css/*.css', watched: true, included: true, served: true}); diff --git a/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php b/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php index 4549b05935c..59dc7366cc0 100644 --- a/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php +++ b/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php @@ -22,6 +22,7 @@ namespace Test\App\AppStore\Fetcher; use OC\App\AppStore\Fetcher\AppFetcher; +use OC\App\CompareVersion; use OC\Files\AppData\Factory; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\IAppData; @@ -33,18 +34,21 @@ use OCP\Http\Client\IClientService; use OCP\Http\Client\IResponse; use OCP\IConfig; use OCP\ILogger; +use PHPUnit_Framework_MockObject_MockObject; use Test\TestCase; class AppFetcherTest extends TestCase { - /** @var IAppData|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IAppData|PHPUnit_Framework_MockObject_MockObject */ protected $appData; - /** @var IClientService|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IClientService|PHPUnit_Framework_MockObject_MockObject */ protected $clientService; - /** @var ITimeFactory|\PHPUnit_Framework_MockObject_MockObject */ + /** @var ITimeFactory|PHPUnit_Framework_MockObject_MockObject */ protected $timeFactory; - /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IConfig|PHPUnit_Framework_MockObject_MockObject */ protected $config; - /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ + /** @var CompareVersion|PHPUnit_Framework_MockObject_MockObject */ + protected $compareVersion; + /** @var ILogger|PHPUnit_Framework_MockObject_MockObject */ protected $logger; /** @var AppFetcher */ protected $fetcher; @@ -57,7 +61,7 @@ EOD; public function setUp() { parent::setUp(); - /** @var Factory|\PHPUnit_Framework_MockObject_MockObject $factory */ + /** @var Factory|PHPUnit_Framework_MockObject_MockObject $factory */ $factory = $this->createMock(Factory::class); $this->appData = $this->createMock(IAppData::class); $factory->expects($this->once()) @@ -67,6 +71,7 @@ EOD; $this->clientService = $this->createMock(IClientService::class); $this->timeFactory = $this->createMock(ITimeFactory::class); $this->config = $this->createMock(IConfig::class); + $this->compareVersion = new CompareVersion(); $this->logger = $this->createMock(ILogger::class); $this->config @@ -79,6 +84,7 @@ EOD; $this->clientService, $this->timeFactory, $this->config, + $this->compareVersion, $this->logger ); } diff --git a/tests/lib/App/CompareVersionTest.php b/tests/lib/App/CompareVersionTest.php new file mode 100644 index 00000000000..60309fdeae4 --- /dev/null +++ b/tests/lib/App/CompareVersionTest.php @@ -0,0 +1,91 @@ +<?php + +/** + * @copyright 2018 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2018 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +namespace Test\App; + +use InvalidArgumentException; +use OC\App\CompareVersion; +use Test\TestCase; + +class CompareVersionTest extends TestCase { + + /** @var CompareVersion */ + private $compare; + + protected function setUp() { + parent::setUp(); + + $this->compare = new CompareVersion(); + } + + public function comparisonData() { + return [ + // Compatible versions + ['13.0.0.3', '13.0.0', '>=', true], + ['13.0.0.3', '13.0.0', '<=', true], + ['13.0.0', '13.0.0', '>=', true], + ['13.0.0', '13.0', '<=', true], + ['13.0.0', '13', '>=', true], + ['13.0.1', '13', '>=', true], + ['13.0.1', '13', '<=', true], + // Incompatible major versions + ['13.0.0.3', '13.0.0', '<', false], + ['12.0.0', '13.0.0', '>=', false], + ['12.0.0', '13.0', '>=', false], + ['12.0.0', '13', '>=', false], + // Incompatible minor and patch versions + ['13.0.0', '13.0.1', '>=', false], + ['13.0.0', '13.1', '>=', false], + // Compatible minor and patch versions + ['13.0.1', '13.0.0', '>=', true], + ['13.2.0', '13.1', '>=', true], + ]; + } + + /** + * @dataProvider comparisonData + */ + public function testComparison(string $actualVersion, string $requiredVersion, + string $comparator, bool $expected) { + $isCompatible = $this->compare->isCompatible($actualVersion, $requiredVersion, + $comparator); + + $this->assertEquals($expected, $isCompatible); + } + + public function testInvalidServerVersion() { + $actualVersion = '13'; + $this->expectException(InvalidArgumentException::class); + + $this->compare->isCompatible($actualVersion, '13.0.0'); + } + + public function testInvalidRequiredVersion() { + $actualVersion = '13.0.0'; + $this->expectException(InvalidArgumentException::class); + + $this->compare->isCompatible($actualVersion, '13.0.0.9'); + } + +} diff --git a/tests/lib/LoggerTest.php b/tests/lib/LoggerTest.php index 6f528bd6fab..9fdbccc24dc 100644 --- a/tests/lib/LoggerTest.php +++ b/tests/lib/LoggerTest.php @@ -96,9 +96,12 @@ class LoggerTest extends TestCase { $logLines = $this->getLogs(); foreach($logLines as $logLine) { + if (is_array($logLine)) { + $logLine = json_encode($logLine); + } $this->assertNotContains($user, $logLine); $this->assertNotContains($password, $logLine); - $this->assertContains('login(*** sensitive parameters replaced ***)', $logLine); + $this->assertContains('*** sensitive parameters replaced ***', $logLine); } } @@ -115,9 +118,12 @@ class LoggerTest extends TestCase { $logLines = $this->getLogs(); foreach($logLines as $logLine) { + if (is_array($logLine)) { + $logLine = json_encode($logLine); + } $this->assertNotContains($user, $logLine); $this->assertNotContains($password, $logLine); - $this->assertContains('checkPassword(*** sensitive parameters replaced ***)', $logLine); + $this->assertContains('*** sensitive parameters replaced ***', $logLine); } } @@ -134,9 +140,12 @@ class LoggerTest extends TestCase { $logLines = $this->getLogs(); foreach($logLines as $logLine) { + if (is_array($logLine)) { + $logLine = json_encode($logLine); + } $this->assertNotContains($user, $logLine); $this->assertNotContains($password, $logLine); - $this->assertContains('validateUserPass(*** sensitive parameters replaced ***)', $logLine); + $this->assertContains('*** sensitive parameters replaced ***', $logLine); } } @@ -153,9 +162,12 @@ class LoggerTest extends TestCase { $logLines = $this->getLogs(); foreach($logLines as $logLine) { + if (is_array($logLine)) { + $logLine = json_encode($logLine); + } $this->assertNotContains($user, $logLine); $this->assertNotContains($password, $logLine); - $this->assertContains('tryLogin(*** sensitive parameters replaced ***)', $logLine); + $this->assertContains('*** sensitive parameters replaced ***', $logLine); } } @@ -177,13 +189,16 @@ class LoggerTest extends TestCase { $logLines = $this->getLogs(); foreach($logLines as $logLine) { + if (is_array($logLine)) { + $logLine = json_encode($logLine); + } $log = explode('\n', $logLine); unset($log[1]); // Remove `testDetectclosure(` because we are not testing this here, but the closure on stack trace 0 $logLine = implode('\n', $log); $this->assertNotContains($user, $logLine); $this->assertNotContains($password, $logLine); - $this->assertContains('{closure}(*** sensitive parameters replaced ***)', $logLine); + $this->assertContains('*** sensitive parameters replaced ***', $logLine); } } diff --git a/tests/lib/Security/HasherTest.php b/tests/lib/Security/HasherTest.php index 86d4ef6ca01..c994b68f781 100644 --- a/tests/lib/Security/HasherTest.php +++ b/tests/lib/Security/HasherTest.php @@ -21,52 +21,74 @@ class HasherTest extends \Test\TestCase { */ public function versionHashProvider() { - return array( - array('asf32äà$$a.|3', null), - array('asf32äà$$a.|3|5', null), - array('1|2|3|4', array('version' => 1, 'hash' => '2|3|4')), - array('1|我看|这本书。 我看這本書', array('version' => 1, 'hash' => '我看|这本书。 我看這本書')) - ); + return [ + ['asf32äà$$a.|3', null], + ['asf32äà$$a.|3|5', null], + ['1|2|3|4', ['version' => 1, 'hash' => '2|3|4']], + ['1|我看|这本书。 我看這本書', ['version' => 1, 'hash' => '我看|这本书。 我看這本書']], + ['2|newhash', ['version' => 2, 'hash' => 'newhash']], + ]; } /** * @return array */ - public function allHashProviders() + public function hashProviders70_71() { - return array( + return [ // Valid SHA1 strings - array('password', '5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8', true), - array('owncloud.com', '27a4643e43046c3569e33b68c1a4b15d31306d29', true), + ['password', '5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8', true], + ['owncloud.com', '27a4643e43046c3569e33b68c1a4b15d31306d29', true], // Invalid SHA1 strings - array('InvalidString', '5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8', false), - array('AnotherInvalidOne', '27a4643e43046c3569e33b68c1a4b15d31306d29', false), + ['InvalidString', '5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8', false], + ['AnotherInvalidOne', '27a4643e43046c3569e33b68c1a4b15d31306d29', false], // Valid legacy password string with password salt "6Wow67q1wZQZpUUeI6G2LsWUu4XKx" - array('password', '$2a$08$emCpDEl.V.QwPWt5gPrqrOhdpH6ailBmkj2Hd2vD5U8qIy20HBe7.', true), - array('password', '$2a$08$yjaLO4ev70SaOsWZ9gRS3eRSEpHVsmSWTdTms1949mylxJ279hzo2', true), - array('password', '$2a$08$.jNRG/oB4r7gHJhAyb.mDupNUAqTnBIW/tWBqFobaYflKXiFeG0A6', true), - array('owncloud.com', '$2a$08$YbEsyASX/hXVNMv8hXQo7ezreN17T8Jl6PjecGZvpX.Ayz2aUyaZ2', true), - array('owncloud.com', '$2a$11$cHdDA2IkUP28oNGBwlL7jO/U3dpr8/0LIjTZmE8dMPA7OCUQsSTqS', true), - array('owncloud.com', '$2a$08$GH.UoIfJ1e.qeZ85KPqzQe6NR8XWRgJXWIUeE1o/j1xndvyTA1x96', true), + ['password', '$2a$08$emCpDEl.V.QwPWt5gPrqrOhdpH6ailBmkj2Hd2vD5U8qIy20HBe7.', true], + ['password', '$2a$08$yjaLO4ev70SaOsWZ9gRS3eRSEpHVsmSWTdTms1949mylxJ279hzo2', true], + ['password', '$2a$08$.jNRG/oB4r7gHJhAyb.mDupNUAqTnBIW/tWBqFobaYflKXiFeG0A6', true], + ['owncloud.com', '$2a$08$YbEsyASX/hXVNMv8hXQo7ezreN17T8Jl6PjecGZvpX.Ayz2aUyaZ2', true], + ['owncloud.com', '$2a$11$cHdDA2IkUP28oNGBwlL7jO/U3dpr8/0LIjTZmE8dMPA7OCUQsSTqS', true], + ['owncloud.com', '$2a$08$GH.UoIfJ1e.qeZ85KPqzQe6NR8XWRgJXWIUeE1o/j1xndvyTA1x96', true], // Invalid legacy passwords - array('password', '$2a$08$oKAQY5IhnZocP.61MwP7xu7TNeOb7Ostvk3j6UpacvaNMs.xRj7O2', false), + ['password', '$2a$08$oKAQY5IhnZocP.61MwP7xu7TNeOb7Ostvk3j6UpacvaNMs.xRj7O2', false], // Valid passwords "6Wow67q1wZQZpUUeI6G2LsWUu4XKx" - array('password', '1|$2a$05$ezAE0dkwk57jlfo6z5Pql.gcIK3ReXT15W7ITNxVS0ksfhO/4E4Kq', true), - array('password', '1|$2a$05$4OQmloFW4yTVez2MEWGIleDO9Z5G9tWBXxn1vddogmKBQq/Mq93pe', true), - array('password', '1|$2a$11$yj0hlp6qR32G9exGEXktB.yW2rgt2maRBbPgi3EyxcDwKrD14x/WO', true), - array('owncloud.com', '1|$2a$10$Yiss2WVOqGakxuuqySv5UeOKpF8d8KmNjuAPcBMiRJGizJXjA2bKm', true), - array('owncloud.com', '1|$2a$10$v9mh8/.mF/Ut9jZ7pRnpkuac3bdFCnc4W/gSumheQUi02Sr.xMjPi', true), - array('owncloud.com', '1|$2a$05$ST5E.rplNRfDCzRpzq69leRzsTGtY7k88h9Vy2eWj0Ug/iA9w5kGK', true), + ['password', '1|$2a$05$ezAE0dkwk57jlfo6z5Pql.gcIK3ReXT15W7ITNxVS0ksfhO/4E4Kq', true], + ['password', '1|$2a$05$4OQmloFW4yTVez2MEWGIleDO9Z5G9tWBXxn1vddogmKBQq/Mq93pe', true], + ['password', '1|$2a$11$yj0hlp6qR32G9exGEXktB.yW2rgt2maRBbPgi3EyxcDwKrD14x/WO', true], + ['owncloud.com', '1|$2a$10$Yiss2WVOqGakxuuqySv5UeOKpF8d8KmNjuAPcBMiRJGizJXjA2bKm', true], + ['owncloud.com', '1|$2a$10$v9mh8/.mF/Ut9jZ7pRnpkuac3bdFCnc4W/gSumheQUi02Sr.xMjPi', true], + ['owncloud.com', '1|$2a$05$ST5E.rplNRfDCzRpzq69leRzsTGtY7k88h9Vy2eWj0Ug/iA9w5kGK', true], // Invalid passwords - array('password', '0|$2a$08$oKAQY5IhnZocP.61MwP7xu7TNeOb7Ostvk3j6UpacvaNMs.xRj7O2', false), - array('password', '1|$2a$08$oKAQY5IhnZocP.61MwP7xu7TNeOb7Ostvk3j6UpacvaNMs.xRj7O2', false), - array('password', '2|$2a$08$oKAQY5IhnZocP.61MwP7xu7TNeOb7Ostvk3j6UpacvaNMs.xRj7O2', false), - ); + ['password', '0|$2a$08$oKAQY5IhnZocP.61MwP7xu7TNeOb7Ostvk3j6UpacvaNMs.xRj7O2', false], + ['password', '1|$2a$08$oKAQY5IhnZocP.61MwP7xu7TNeOb7Ostvk3j6UpacvaNMs.xRj7O2', false], + ['password', '2|$2a$08$oKAQY5IhnZocP.61MwP7xu7TNeOb7Ostvk3j6UpacvaNMs.xRj7O2', false], + ]; + } + + + /** + * @return array + */ + public function hashProviders72() { + return [ + // Valid ARGON2 hashes + ['password', '2|$argon2i$v=19$m=1024,t=2,p=2$T3JGcEkxVFNOVktNSjZUcg$4/hyLtSejxNgAuzSFFV/HLM3qRQKBwEtKw61qPN4zWA', true], + ['password', '2|$argon2i$v=19$m=1024,t=2,p=2$Zk52V24yNjMzTkhyYjJKOQ$vmqHkCaOD6SiiiFKD1GeKLg/D1ynWpyZbx4XA2yed34', true], + ['password', '2|$argon2i$v=19$m=1024,t=2,p=2$R1pRcUZKamVlNndBc3l5ag$ToRhR8SiZc7fGMpOYfSc5haS5t9+Y00rljPJV7+qLkM', true], + ['nextcloud.com', '2|$argon2i$v=19$m=1024,t=2,p=2$NC9xM0FFaDlzM01QM3kudg$fSfndwtO2mKMZlKdsT8XAtPY51cSS6pLSGS3xMqeJhg', true], + ['nextcloud.com', '2|$argon2i$v=19$m=1024,t=2,p=2$UjkvUjEuL042WWl1cmdHOA$FZivLkBdZnloQsW6qq/jqWK95JSYUHW9rwQC4Ff9GN0', true], + ['nextcloud.com', '2|$argon2i$v=19$m=1024,t=2,p=2$ZnpNdUlzMEpUTW40OVpiMQ$c+yHT9dtSYsjtVGsa7UKOsxxgQAMiUc781d9WsFACqs', true], + + //Invalid ARGON2 hashes + ['password', '2|$argon2i$v=19$m=1024,t=2,p=2$UjFDUDg3cjBvM3FkbXVOWQ$7Y5xqFxSERnYn+2+7WChUpWZWMa5BEIhSHWnDgJ71Jk', false], + ['password', '2|$argon2i$v=19$m=1024,t=2,p=2$ZUxSUi5aQklXdkcyMG1uVA$sYjoSvXg/CS/aS6Xnas/o9a/OPVcGKldzzmuiCD1Fxo', false], + ['password', '2|$argon2i$v=19$m=1024,t=2,p=2$ZHQ5V0xMOFNmUC52by44Sg$DzQFk3bJTX0J4PVGwW6rMvtnBJRalBkbtpDIXR+d4A0', false], + ]; } /** @var Hasher */ @@ -78,13 +100,12 @@ class HasherTest extends \Test\TestCase { protected function setUp() { parent::setUp(); - $this->config = $this->getMockBuilder(IConfig::class) - ->disableOriginalConstructor()->getMock(); + $this->config = $this->createMock(IConfig::class); $this->hasher = new Hasher($this->config); } - function testHash() { + public function testHash() { $hash = $this->hasher->hash('String To Hash'); $this->assertNotNull($hash); } @@ -92,16 +113,16 @@ class HasherTest extends \Test\TestCase { /** * @dataProvider versionHashProvider */ - function testSplitHash($hash, $expected) { - $relativePath = self::invokePrivate($this->hasher, 'splitHash', array($hash)); + public function testSplitHash($hash, $expected) { + $relativePath = self::invokePrivate($this->hasher, 'splitHash', [$hash]); $this->assertSame($expected, $relativePath); } /** - * @dataProvider allHashProviders + * @dataProvider hashProviders70_71 */ - function testVerify($password, $hash, $expected) { + public function testVerify($password, $hash, $expected) { $this->config ->expects($this->any()) ->method('getSystemValue') @@ -112,4 +133,33 @@ class HasherTest extends \Test\TestCase { $this->assertSame($expected, $result); } + /** + * @dataProvider hashProviders72 + */ + public function testVerifyArgon2i($password, $hash, $expected) { + if (!\defined('PASSWORD_ARGON2I')) { + $this->markTestSkipped('Need ARGON2 support to test ARGON2 hashes'); + } + + $result = $this->hasher->verify($password, $hash); + $this->assertSame($expected, $result); + } + + public function testUpgradeHashBlowFishToArgon2i() { + if (!\defined('PASSWORD_ARGON2I')) { + $this->markTestSkipped('Need ARGON2 support to test ARGON2 hashes'); + } + + $message = 'mysecret'; + + $blowfish = 1 . '|' . password_hash($message, PASSWORD_BCRYPT, []); + $argon2i = 2 . '|' . password_hash($message, PASSWORD_ARGON2I, []); + + $this->assertTrue($this->hasher->verify($message, $blowfish,$newHash)); + $this->assertTrue($this->hasher->verify($message, $argon2i)); + + $relativePath = self::invokePrivate($this->hasher, 'splitHash', [$newHash]); + + $this->assertFalse(password_needs_rehash($relativePath['hash'], PASSWORD_ARGON2I, [])); + } } |