From 464475778545a43e0f628e2a775ea78cfcf4ed53 Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Mon, 12 Feb 2018 13:39:52 -0200 Subject: [PATCH 001/109] FEATURE: Style new gfycat onebox --- .../stylesheets/common/base/onebox.scss | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/common/base/onebox.scss b/app/assets/stylesheets/common/base/onebox.scss index 61d92687fe2..c692cfceb7d 100644 --- a/app/assets/stylesheets/common/base/onebox.scss +++ b/app/assets/stylesheets/common/base/onebox.scss @@ -463,10 +463,22 @@ aside.onebox.stackexchange .onebox-body { .label2 { float: right; } - .site-icon { - width: 16px; - height: 16px; - margin-right: 3px; +} + +.onebox { + &.whitelistedgeneric, + &.gfycat { + .site-icon { + width: 16px; + height: 16px; + margin-right: 3px; + } + } +} + +.onebox.gfycat p { + span.label1 a { + white-space: nowrap; } } From ed114177e76bd996a12c9c64ad7d6e7eb5a19353 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 13 Feb 2018 19:41:03 +0100 Subject: [PATCH 002/109] Mini tag chooser tweaks --- .../select-kit/components/mini-tag-chooser.js.es6 | 13 ++++++++++++- config/locales/client.en.yml | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 b/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 index 0de5b95894e..abce533ffcd 100644 --- a/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 +++ b/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 @@ -14,6 +14,17 @@ export default ComboBox.extend({ filterable: true, noTags: Ember.computed.empty("computedTags"), allowAny: true, + caretUpIcon: Ember.computed.alias("caretIcon"), + caretDownIcon: Ember.computed.alias("caretIcon"), + + @computed("computedTags", "siteSettings.max_tags_per_topic") + caretIcon(computedTags, maxTagsPerTopic) { + if (computedTags.length >= maxTagsPerTopic) { + return null; + } + + return "plus"; + }, init() { this._super(); @@ -136,7 +147,7 @@ export default ComboBox.extend({ if (isEmpty(this.get("computedTags"))) { content.label = I18n.t("tagging.choose_for_topic"); } else { - content.label = this.get("computedTags").join(","); + content.label = this.get("computedTags").join(", "); } return content; diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 49d0be51dc3..fa7006c6cad 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1183,7 +1183,7 @@ en: default_header_text: Select... no_content: No matches found filter_placeholder: Search... - create: "Create {{content}}" + create: "Create: '{{content}}'" emoji_picker: filter_placeholder: Search for emoji From 734851384869f2886cae4ecfc73f221dec86c379 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 13 Feb 2018 15:33:15 -0500 Subject: [PATCH 003/109] FIX: Include post in staff action logs when silencing a user --- app/controllers/admin/users_controller.rb | 3 ++- app/services/staff_action_logger.rb | 13 +++++----- app/services/user_silencer.rb | 6 +++-- .../admin/users_controller_spec.rb | 25 +++++++++++++++++++ spec/services/user_silencer_spec.rb | 2 +- 5 files changed, 39 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 13aa89d3ca5..d948d5f5f09 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -289,7 +289,8 @@ class Admin::UsersController < Admin::AdminController silenced_till: params[:silenced_till], reason: params[:reason], message_body: message, - keep_posts: true + keep_posts: true, + post_id: params[:post_id] ) if silencer.silence && message.present? Jobs.enqueue( diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 6261e655809..b5a64c3f2f2 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -289,13 +289,14 @@ class StaffActionLogger def log_silence_user(user, opts = {}) raise Discourse::InvalidParameters.new(:user) unless user - UserHistory.create( - params(opts).merge( - action: UserHistory.actions[:silence_user], - target_user_id: user.id, - details: opts[:details] - ) + create_args = params(opts).merge( + action: UserHistory.actions[:silence_user], + target_user_id: user.id, + details: opts[:details] ) + create_args[:post_id] = opts[:post_id] if opts[:post_id] + + UserHistory.create(create_args) end def log_unsilence_user(user, opts = {}) diff --git a/app/services/user_silencer.rb b/app/services/user_silencer.rb index 5db360637b8..a9fff88dd0c 100644 --- a/app/services/user_silencer.rb +++ b/app/services/user_silencer.rb @@ -33,10 +33,12 @@ class UserSilencer SystemMessage.create(@user, message_type) if @by_user + log_params = { context: context, details: details } + log_params[:post_id] = @opts[:post_id].to_i if @opts[:post_id] + @user_history = StaffActionLogger.new(@by_user).log_silence_user( @user, - context: context, - details: details + log_params ) end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index e56fa93b3e6..1936233f701 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -615,6 +615,31 @@ describe Admin::UsersController do expect(@reg_user).to be_silenced end + it "can have an associated post" do + silence_post = Fabricate(:post, user: @reg_user) + + put :silence, params: { + user_id: @reg_user.id, + post_id: silence_post.id, + post_action: 'edit', + post_edit: "this is the new contents for the post" + }, format: :json + expect(response).to be_success + + silence_post.reload + expect(silence_post.raw).to eq("this is the new contents for the post") + + log = UserHistory.where( + target_user_id: @reg_user.id, + action: UserHistory.actions[:silence_user] + ).first + expect(log).to be_present + expect(log.post_id).to eq(silence_post.id) + + @reg_user.reload + expect(@reg_user).to be_silenced + end + it "will set a length of time if provided" do future_date = 1.month.from_now.to_date put( diff --git a/spec/services/user_silencer_spec.rb b/spec/services/user_silencer_spec.rb index 188bd0435fd..73bab2e19ff 100644 --- a/spec/services/user_silencer_spec.rb +++ b/spec/services/user_silencer_spec.rb @@ -60,7 +60,7 @@ describe UserSilencer do end it "logs it with context" do - SystemMessage.stubs(:create).returns(Fabricate.build(:post)) + SystemMessage.stubs(:create) expect { UserSilencer.silence(user, Fabricate(:admin)) }.to change { UserHistory.count }.by(1) From 713993d15048864946a46e74bf3f21549b6f0c1b Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 13 Feb 2018 15:58:32 -0500 Subject: [PATCH 004/109] UX: Link post ids in staff action logs to the post --- .../admin/components/staff-actions.js.es6 | 22 +++++++++++++++++++ .../admin/models/staff-action-log.js.es6 | 11 +++++++--- .../templates/logs/staff-action-logs.hbs | 4 ++-- 3 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 app/assets/javascripts/admin/components/staff-actions.js.es6 diff --git a/app/assets/javascripts/admin/components/staff-actions.js.es6 b/app/assets/javascripts/admin/components/staff-actions.js.es6 new file mode 100644 index 00000000000..9e742526afa --- /dev/null +++ b/app/assets/javascripts/admin/components/staff-actions.js.es6 @@ -0,0 +1,22 @@ +import DiscourseURL from 'discourse/lib/url'; + +export default Ember.Component.extend({ + classNames: ['table', 'staff-actions'], + + willDestroyElement() { + this.$().off('click.discourse-staff-logs'); + }, + + didInsertElement() { + this._super(); + + this.$().on('click.discourse-staff-logs', '[data-link-post-id]', e => { + let postId = $(e.target).attr('data-link-post-id'); + + this.store.find('post', postId).then(p => { + DiscourseURL.routeTo(p.get('url')); + }); + return false; + }); + } +}); diff --git a/app/assets/javascripts/admin/models/staff-action-log.js.es6 b/app/assets/javascripts/admin/models/staff-action-log.js.es6 index e60d815f7c7..5c84e5c3940 100644 --- a/app/assets/javascripts/admin/models/staff-action-log.js.es6 +++ b/app/assets/javascripts/admin/models/staff-action-log.js.es6 @@ -1,6 +1,7 @@ import { ajax } from 'discourse/lib/ajax'; import AdminUser from 'admin/models/admin-user'; import { escapeExpression } from 'discourse/lib/utilities'; +import getUrl from 'discourse-common/lib/get-url'; const StaffActionLog = Discourse.Model.extend({ showFullDetails: false, @@ -10,7 +11,7 @@ const StaffActionLog = Discourse.Model.extend({ }.property('action_name'), formattedDetails: function() { - var formatted = ""; + let formatted = ""; formatted += this.format('email', 'email'); formatted += this.format('admin.logs.ip_address', 'ip_address'); formatted += this.format('admin.logs.topic_id', 'topic_id'); @@ -26,9 +27,13 @@ const StaffActionLog = Discourse.Model.extend({ return formatted; }.property('ip_address', 'email', 'topic_id', 'post_id', 'category_id'), - format: function(label, propertyName) { + format(label, propertyName) { if (this.get(propertyName)) { - return ('' + I18n.t(label) + ': ' + escapeExpression(this.get(propertyName)) + '
'); + let value = escapeExpression(this.get(propertyName)); + if (propertyName === 'post_id') { + value = `${value}`; + } + return `${I18n.t(label)}: ${value}
`; } else { return ''; } diff --git a/app/assets/javascripts/admin/templates/logs/staff-action-logs.hbs b/app/assets/javascripts/admin/templates/logs/staff-action-logs.hbs index 5df150c79fb..3445abf8df5 100644 --- a/app/assets/javascripts/admin/templates/logs/staff-action-logs.hbs +++ b/app/assets/javascripts/admin/templates/logs/staff-action-logs.hbs @@ -39,7 +39,7 @@
-
+{{#staff-actions}}
{{i18n 'admin.logs.staff_actions.staff_user'}}
{{i18n 'admin.logs.action'}}
@@ -86,4 +86,4 @@ {{i18n 'search.no_results'}} {{/each}} {{/conditional-loading-spinner}} -
+{{/staff-actions}} From 556ab8480ed044da616bb8a81bad134dcc0d947e Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 13 Feb 2018 16:06:40 -0500 Subject: [PATCH 005/109] FIX: ESlint --- app/assets/javascripts/admin/models/staff-action-log.js.es6 | 1 - 1 file changed, 1 deletion(-) diff --git a/app/assets/javascripts/admin/models/staff-action-log.js.es6 b/app/assets/javascripts/admin/models/staff-action-log.js.es6 index 5c84e5c3940..78d39869cf4 100644 --- a/app/assets/javascripts/admin/models/staff-action-log.js.es6 +++ b/app/assets/javascripts/admin/models/staff-action-log.js.es6 @@ -1,7 +1,6 @@ import { ajax } from 'discourse/lib/ajax'; import AdminUser from 'admin/models/admin-user'; import { escapeExpression } from 'discourse/lib/utilities'; -import getUrl from 'discourse-common/lib/get-url'; const StaffActionLog = Discourse.Model.extend({ showFullDetails: false, From 22f0b0096d33d918f7a4505a653b410e168d435c Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 13 Feb 2018 17:13:32 -0500 Subject: [PATCH 006/109] FEATURE: show avatar flair on user profile page --- .../javascripts/discourse/templates/user.hbs | 11 ++++++++++- .../stylesheets/common/base/topic-post.scss | 6 +++--- app/assets/stylesheets/common/base/user.scss | 16 ++++++++++++++++ app/assets/stylesheets/mobile/user.scss | 7 +++++++ 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/discourse/templates/user.hbs b/app/assets/javascripts/discourse/templates/user.hbs index f20660c64ac..a5858926760 100644 --- a/app/assets/javascripts/discourse/templates/user.hbs +++ b/app/assets/javascripts/discourse/templates/user.hbs @@ -39,7 +39,16 @@
- {{bound-avatar model "huge"}} +
    {{#if model.can_send_private_message_to_user}} diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss index 9614af03bc4..baee2ab77a7 100644 --- a/app/assets/stylesheets/common/base/topic-post.scss +++ b/app/assets/stylesheets/common/base/topic-post.scss @@ -263,7 +263,7 @@ aside.quote { } -.topic-avatar, .avatar-flair-preview, .user-card-avatar, .topic-map .poster { +.topic-avatar, .avatar-flair-preview, .user-card-avatar, .topic-map .poster, .user-profile-avatar { .avatar-flair { display: flex; align-items: center; @@ -275,7 +275,7 @@ aside.quote { right: -6px; } } -.topic-avatar .avatar-flair, .avatar-flair-preview .avatar-flair { +.topic-avatar .avatar-flair, .avatar-flair-preview .avatar-flair, .collapsed-info .user-profile-avatar .avatar-flair { background-size: 20px 20px; width: 20px; height: 20px; @@ -288,7 +288,7 @@ aside.quote { right: -8px; } } -.user-card-avatar .avatar-flair { +.user-card-avatar .avatar-flair, .user-profile-avatar .avatar-flair { background-size: 40px 40px; width: 40px; height: 40px; diff --git a/app/assets/stylesheets/common/base/user.scss b/app/assets/stylesheets/common/base/user.scss index 5efd7ed9ca2..1d0343cb569 100644 --- a/app/assets/stylesheets/common/base/user.scss +++ b/app/assets/stylesheets/common/base/user.scss @@ -120,6 +120,15 @@ } } } + + .user-profile-avatar { + position: relative; + float: left; + .avatar-flair { + bottom: 8px; + right: 16px; + } + } } .controls { @@ -176,6 +185,13 @@ } } } + + .user-profile-avatar { + .avatar-flair { + bottom: 8px; + right: 2px; + } + } } } diff --git a/app/assets/stylesheets/mobile/user.scss b/app/assets/stylesheets/mobile/user.scss index fccb2542ef4..7a652d99046 100644 --- a/app/assets/stylesheets/mobile/user.scss +++ b/app/assets/stylesheets/mobile/user.scss @@ -127,6 +127,10 @@ max-width: 700px; } } + + .user-profile-avatar .avatar-flair { + right: 2px; + } } .controls { @@ -177,6 +181,9 @@ } } } + .user-profile-avatar .avatar-flair { + bottom: 12px; + } } } From 5a56746610e218993fb18718cf0bdbc87b182b07 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Wed, 14 Feb 2018 00:28:16 +0100 Subject: [PATCH 007/109] FIX: Embedded topic was not found when URL contained query string --- app/models/topic_embed.rb | 2 +- spec/models/topic_embed_spec.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb index 8e3071ea4c8..2f2b26e1952 100644 --- a/app/models/topic_embed.rb +++ b/app/models/topic_embed.rb @@ -183,7 +183,7 @@ class TopicEmbed < ActiveRecord::Base def self.topic_id_for_embed(embed_url) embed_url = normalize_url(embed_url).sub(/^https?\:\/\//, '') - TopicEmbed.where("embed_url ~* '^https?://#{embed_url}$'").pluck(:topic_id).first + TopicEmbed.where("embed_url ~* '^https?://#{Regexp.escape(embed_url)}$'").pluck(:topic_id).first end def self.first_paragraph_from(html) diff --git a/spec/models/topic_embed_spec.rb b/spec/models/topic_embed_spec.rb index 447e8ff667f..1f00c699ec0 100644 --- a/spec/models/topic_embed_spec.rb +++ b/spec/models/topic_embed_spec.rb @@ -72,6 +72,11 @@ describe TopicEmbed do expect(TopicEmbed.topic_id_for_embed('http://example.com/post/24')).to eq(nil) expect(TopicEmbed.topic_id_for_embed('http://example.com/post')).to eq(nil) end + + it "finds the topic id when the embed_url contains a query string" do + topic_embed = Fabricate(:topic_embed, embed_url: "http://example.com/post/248?key=foo") + expect(TopicEmbed.topic_id_for_embed('http://example.com/post/248?key=foo')).to eq(topic_embed.topic_id) + end end describe '.find_remote' do From 548db91c76fd295a7369a27c75d5a614d68a0190 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 14 Feb 2018 00:30:09 +0100 Subject: [PATCH 008/109] FIX: displays an error when reaching tags limit --- .../components/mini-tag-chooser.js.es6 | 44 ++++++++++++++----- .../select-kit/components/select-kit.js.es6 | 9 ++-- .../templates/components/select-kit.hbs | 4 +- .../select-kit/select-kit-collection.hbs | 34 +++++++------- .../common/select-kit/select-kit.scss | 5 +++ config/locales/client.en.yml | 1 + 6 files changed, 67 insertions(+), 30 deletions(-) diff --git a/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 b/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 index abce533ffcd..9f12c5c58f9 100644 --- a/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 +++ b/app/assets/javascripts/select-kit/components/mini-tag-chooser.js.es6 @@ -14,16 +14,33 @@ export default ComboBox.extend({ filterable: true, noTags: Ember.computed.empty("computedTags"), allowAny: true, + maximumSelectionSize: Ember.computed.alias("siteSettings.max_tags_per_topic"), caretUpIcon: Ember.computed.alias("caretIcon"), caretDownIcon: Ember.computed.alias("caretIcon"), - @computed("computedTags", "siteSettings.max_tags_per_topic") - caretIcon(computedTags, maxTagsPerTopic) { - if (computedTags.length >= maxTagsPerTopic) { - return null; + @computed("limitReached", "maximumSelectionSize") + maxContentRow(limitReached, count) { + if (limitReached) { + return I18n.t("select_kit.max_content_reached", { count }); + } + }, + + mutateAttributes() { + this.set("value", null); + }, + + @computed("limitReached") + caretIcon(limitReached) { + return limitReached ? null : "plus"; + }, + + @computed("computedTags.[]", "maximumSelectionSize") + limitReached(computedTags, maximumSelectionSize) { + if (computedTags.length >= maximumSelectionSize) { + return true; } - return "plus"; + return false; }, init() { @@ -46,6 +63,10 @@ export default ComboBox.extend({ }, validateCreate(term) { + if (this.get("limitReached")) { + return false; + } + const filterRegexp = new RegExp(this.site.tags_filter_regexp, "g"); term = term.replace(filterRegexp, "").trim().toLowerCase(); @@ -65,6 +86,10 @@ export default ComboBox.extend({ this.site.get("can_create_tag"); }, + filterComputedContent(computedContent) { + return computedContent; + }, + didRender() { this._super(); @@ -143,13 +168,16 @@ export default ComboBox.extend({ computeHeaderContent() { let content = this.baseHeaderComputedContent(); + const joinedTags = this.get("computedTags").join(", "); if (isEmpty(this.get("computedTags"))) { content.label = I18n.t("tagging.choose_for_topic"); } else { - content.label = this.get("computedTags").join(", "); + content.label = joinedTags; } + content.title = content.name = content.value = joinedTags; + return content; }, @@ -183,10 +211,6 @@ export default ComboBox.extend({ } }, - muateAttributes() { - this.set("value", null); - }, - _searchTags(query) { this.startLoading(); diff --git a/app/assets/javascripts/select-kit/components/select-kit.js.es6 b/app/assets/javascripts/select-kit/components/select-kit.js.es6 index ab9852b1541..b8bdf5c6674 100644 --- a/app/assets/javascripts/select-kit/components/select-kit.js.es6 +++ b/app/assets/javascripts/select-kit/components/select-kit.js.es6 @@ -37,7 +37,6 @@ export default Ember.Component.extend(UtilsMixin, PluginApiMixin, DomHelpersMixi tabindex: 0, none: null, highlightedValue: null, - noContentLabel: "select_kit.no_content", valueAttribute: "id", nameProperty: "name", autoFilterable: false, @@ -70,6 +69,8 @@ export default Ember.Component.extend(UtilsMixin, PluginApiMixin, DomHelpersMixi allowContentReplacement: false, collectionHeader: null, allowAutoSelectFirst: true, + maximumSelectionSize: null, + maxContentRow: null, init() { this._super(); @@ -155,8 +156,10 @@ export default Ember.Component.extend(UtilsMixin, PluginApiMixin, DomHelpersMixi }, @computed("filter", "filteredComputedContent.[]") - shouldDisplayNoContentRow(filter, filteredComputedContent) { - return filter.length > 0 && filteredComputedContent.length === 0; + noContentRow(filter, filteredComputedContent) { + if (filter.length > 0 && filteredComputedContent.length === 0) { + return I18n.t("select_kit.no_content"); + } }, @computed("filter", "filterable", "autoFilterable", "renderedFilterOnce") diff --git a/app/assets/javascripts/select-kit/templates/components/select-kit.hbs b/app/assets/javascripts/select-kit/templates/components/select-kit.hbs index 5e6e936db4a..cd92fb83122 100644 --- a/app/assets/javascripts/select-kit/templates/components/select-kit.hbs +++ b/app/assets/javascripts/select-kit/templates/components/select-kit.hbs @@ -40,11 +40,11 @@ select=(action "select") highlight=(action "highlight") create=(action "create") - noContentLabel=noContentLabel highlightedValue=highlightedValue computedValue=computedValue - shouldDisplayNoContentRow=shouldDisplayNoContentRow rowComponentOptions=rowComponentOptions + noContentRow=noContentRow + maxContentRow=maxContentRow }} {{/if}}
diff --git a/app/assets/javascripts/select-kit/templates/components/select-kit/select-kit-collection.hbs b/app/assets/javascripts/select-kit/templates/components/select-kit/select-kit-collection.hbs index bb02559029f..35d46789dd3 100644 --- a/app/assets/javascripts/select-kit/templates/components/select-kit/select-kit-collection.hbs +++ b/app/assets/javascripts/select-kit/templates/components/select-kit/select-kit-collection.hbs @@ -30,22 +30,26 @@ }} {{/if}} -{{#each filteredComputedContent as |computedContent|}} - {{component rowComponent - computedContent=computedContent - highlightedValue=highlightedValue - computedValue=computedValue - templateForRow=templateForRow - select=select - highlight=highlight - options=rowComponentOptions - }} -{{/each}} - -{{#if shouldDisplayNoContentRow}} - {{#if noContentLabel}} +{{#if maxContentRow}} +
  • + {{maxContentRow}} +
    • +{{else}} + {{#if noContentRow}}
  • - {{i18n noContentLabel}} + {{noContentRow}}
    • + {{else}} + {{#each filteredComputedContent as |computedContent|}} + {{component rowComponent + computedContent=computedContent + highlightedValue=highlightedValue + computedValue=computedValue + templateForRow=templateForRow + select=select + highlight=highlight + options=rowComponentOptions + }} + {{/each}} {{/if}} {{/if}} diff --git a/app/assets/stylesheets/common/select-kit/select-kit.scss b/app/assets/stylesheets/common/select-kit/select-kit.scss index 91e7fe922f7..b5c1597ef32 100644 --- a/app/assets/stylesheets/common/select-kit/select-kit.scss +++ b/app/assets/stylesheets/common/select-kit/select-kit.scss @@ -165,6 +165,11 @@ white-space: nowrap; } + &.max-content { + white-space: nowrap; + color: $danger; + } + .name { margin: 0; overflow: hidden; diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index fa7006c6cad..383e3007066 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1184,6 +1184,7 @@ en: no_content: No matches found filter_placeholder: Search... create: "Create: '{{content}}'" + max_content_reached: "You can only select {{count}} items." emoji_picker: filter_placeholder: Search for emoji From f028ffaf29d871c54d3a50e25cae077e0ed7c591 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 14 Feb 2018 10:39:44 +1100 Subject: [PATCH 009/109] SECURITY: correct local onebox category checks Also removes ugly "source_topic_id" from cooked posts Patch was authored by @zogstrip Signed-off-by: Sam --- .../components/composer-editor.js.es6 | 2 +- .../components/composer-title.js.es6 | 8 +- .../engines/discourse-markdown/quotes.js.es6 | 1 - .../javascripts/pretty-text/oneboxer.js.es6 | 15 +- app/controllers/onebox_controller.rb | 8 +- app/models/post_analyzer.rb | 2 +- lib/cooked_post_processor.rb | 14 +- lib/discourse.rb | 8 +- lib/onebox/engine/discourse_local_onebox.rb | 129 -------------- lib/oneboxer.rb | 136 +++++++++++---- spec/components/cooked_post_processor_spec.rb | 3 +- .../engine/discourse_local_onebox_spec.rb | 161 ------------------ spec/components/oneboxer_spec.rb | 71 ++++++++ spec/controllers/onebox_controller_spec.rb | 54 ++++-- 14 files changed, 251 insertions(+), 361 deletions(-) delete mode 100644 lib/onebox/engine/discourse_local_onebox.rb delete mode 100644 spec/components/onebox/engine/discourse_local_onebox_spec.rb diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6 index 6b3ca5b4001..b0623158e4c 100644 --- a/app/assets/javascripts/discourse/components/composer-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6 @@ -372,7 +372,7 @@ export default Ember.Component.extend({ post.set('refreshedPost', true); } - $oneboxes.each((_, o) => load(o, refresh, ajax, this.currentUser.id)); + $oneboxes.each((_, o) => load({ elem: o, refresh, ajax, categoryId: this.get('composer.category.id') })); }, _warnMentionedGroups($preview) { diff --git a/app/assets/javascripts/discourse/components/composer-title.js.es6 b/app/assets/javascripts/discourse/components/composer-title.js.es6 index 2a828a9483a..86acf45c773 100644 --- a/app/assets/javascripts/discourse/components/composer-title.js.es6 +++ b/app/assets/javascripts/discourse/components/composer-title.js.es6 @@ -80,7 +80,13 @@ export default Ember.Component.extend({ const link = document.createElement('a'); link.href = this.get('composer.title'); - let loadOnebox = load(link, false, ajax, this.currentUser.id, true); + const loadOnebox = load({ + elem: link, + refresh: false, + ajax, + synchronous: true, + categoryId: this.get('composer.category.id'), + }); if (loadOnebox && loadOnebox.then) { loadOnebox.then( () => { diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 index 3a0523bef66..27873269ee1 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/quotes.js.es6 @@ -105,7 +105,6 @@ const rule = { let title = topicInfo.title; - if (options.enableEmoji) { title = performEmojiUnescape(topicInfo.title, { getURL: options.getURL, emojiSet: options.emojiSet diff --git a/app/assets/javascripts/pretty-text/oneboxer.js.es6 b/app/assets/javascripts/pretty-text/oneboxer.js.es6 index 245c87cbca1..90538994b7f 100644 --- a/app/assets/javascripts/pretty-text/oneboxer.js.es6 +++ b/app/assets/javascripts/pretty-text/oneboxer.js.es6 @@ -43,16 +43,15 @@ function loadNext(ajax) { let timeoutMs = 150; let removeLoading = true; - const { url, refresh, $elem, userId } = loadingQueue.shift(); + const { url, refresh, $elem, categoryId } = loadingQueue.shift(); // Retrieve the onebox return ajax("/onebox", { dataType: 'html', - data: { url, refresh }, + data: { url, refresh, category_id: categoryId }, cache: true }).then(html => { let $html = $(html); - localCache[normalize(url)] = $html; $elem.replaceWith($html); applySquareGenericOnebox($html, normalize(url)); @@ -60,7 +59,7 @@ function loadNext(ajax) { if (result && result.jqXHR && result.jqXHR.status === 429) { timeoutMs = 2000; removeLoading = false; - loadingQueue.unshift({ url, refresh, $elem, userId }); + loadingQueue.unshift({ url, refresh, $elem, categoryId }); } else { failedCache[normalize(url)] = true; } @@ -75,14 +74,14 @@ function loadNext(ajax) { // Perform a lookup of a onebox based an anchor $element. // It will insert a loading indicator and remove it when the loading is complete or fails. -export function load(e, refresh, ajax, userId, synchronous) { - const $elem = $(e); +export function load({ elem , refresh = true, ajax, synchronous = false, categoryId }) { + const $elem = $(elem); // If the onebox has loaded or is loading, return if ($elem.data('onebox-loaded')) return; if ($elem.hasClass('loading-onebox')) return; - const url = e.href; + const url = elem.href; // Unless we're forcing a refresh... if (!refresh) { @@ -99,7 +98,7 @@ export function load(e, refresh, ajax, userId, synchronous) { $elem.addClass('loading-onebox'); // Add to the loading queue - loadingQueue.push({ url, refresh, $elem, userId }); + loadingQueue.push({ url, refresh, $elem, categoryId }); // Load next url in queue if (synchronous) { diff --git a/app/controllers/onebox_controller.rb b/app/controllers/onebox_controller.rb index 7c0d16af322..dea29ce5368 100644 --- a/app/controllers/onebox_controller.rb +++ b/app/controllers/onebox_controller.rb @@ -14,13 +14,19 @@ class OneboxController < ApplicationController return render(body: nil, status: 429) if Oneboxer.is_previewing?(current_user.id) user_id = current_user.id + category_id = params[:category_id].to_i invalidate = params[:refresh] == 'true' url = params[:url] hijack do Oneboxer.preview_onebox!(user_id) - preview = Oneboxer.preview(url, invalidate_oneboxes: invalidate) + preview = Oneboxer.preview(url, + invalidate_oneboxes: invalidate, + user_id: user_id, + category_id: category_id + ) + preview.strip! if preview.present? Oneboxer.onebox_previewed!(user_id) diff --git a/app/models/post_analyzer.rb b/app/models/post_analyzer.rb index 919b7d42155..ddcfa2fd0c0 100644 --- a/app/models/post_analyzer.rb +++ b/app/models/post_analyzer.rb @@ -31,7 +31,7 @@ class PostAnalyzer cooked = PrettyText.cook(raw, opts) end - result = Oneboxer.apply(cooked, topic_id: @topic_id) do |url, _| + result = Oneboxer.apply(cooked) do |url| @found_oneboxes = true Oneboxer.invalidate(url) if opts[:invalidate_oneboxes] Oneboxer.cached_onebox(url) diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index e9637755f09..263d139e729 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -368,15 +368,13 @@ class CookedPostProcessor end def post_process_oneboxes - args = { - post_id: @post.id, - invalidate_oneboxes: !!@opts[:invalidate_oneboxes], - } - - # apply oneboxes - Oneboxer.apply(@doc, topic_id: @post.topic_id) do |url| + Oneboxer.apply(@doc) do |url| @has_oneboxes = true - Oneboxer.onebox(url, args) + Oneboxer.onebox(url, + invalidate_oneboxes: !!@opts[:invalidate_oneboxes], + user_id: @post&.user_id, + category_id: @post&.topic&.category_id + ) end oneboxed_images.each do |img| diff --git a/lib/discourse.rb b/lib/discourse.rb index 036836c6057..45b6416c6b1 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -236,15 +236,11 @@ module Discourse end def self.route_for(uri) - - uri = URI(uri) rescue nil unless (uri.is_a?(URI)) + uri = URI(uri) rescue nil unless uri.is_a?(URI) return unless uri path = uri.path || "" - if (uri.host == Discourse.current_hostname && - path.start_with?(Discourse.base_uri)) || - !uri.host - + if !uri.host || (uri.host == Discourse.current_hostname && path.start_with?(Discourse.base_uri)) path.slice!(Discourse.base_uri) return Rails.application.routes.recognize_path(path) end diff --git a/lib/onebox/engine/discourse_local_onebox.rb b/lib/onebox/engine/discourse_local_onebox.rb deleted file mode 100644 index b4add001ac0..00000000000 --- a/lib/onebox/engine/discourse_local_onebox.rb +++ /dev/null @@ -1,129 +0,0 @@ -module Onebox - module Engine - class DiscourseLocalOnebox - include Engine - - # Use this onebox before others - def self.priority - 1 - end - - def self.===(other) - url = other.to_s - return false unless url[Discourse.base_url] - - route = Discourse.route_for(url) - - !!(route[:controller] =~ /topics|uploads|users/) - rescue ActionController::RoutingError - false - end - - def to_html - uri = URI(@url) - path = uri.path || "" - route = Discourse.route_for(uri) - - case route[:controller] - when "uploads" then upload_html(path) - when "topics" then topic_html(route) - when "users" then user_html(route) - end - end - - private - - def upload_html(path) - case File.extname(path) - when /^\.(mov|mp4|webm|ogv)$/i - "" - when /^\.(mp3|ogg|wav|m4a)$/i - "" - end - end - - def topic_html(route) - link = "#{@url}" - source_topic_id = @url[/[&?]source_topic_id=(\d+)/, 1].to_i - source_topic = Topic.find_by(id: source_topic_id) if source_topic_id > 0 - - if route[:post_number].present? && route[:post_number].to_i > 1 - post = Post.find_by(topic_id: route[:topic_id], post_number: route[:post_number]) - return link unless can_see_post?(post, source_topic) - - topic = post.topic - slug = Slug.for(topic.title) - excerpt = post.excerpt(SiteSetting.post_onebox_maxlength) - excerpt.gsub!(/[\r\n]+/, " ") - excerpt.gsub!("[/quote]", "[quote]") # don't break my quote - - quote = "[quote=\"#{post.user.username}, topic:#{topic.id}, slug:#{slug}, post:#{post.post_number}\"]\n#{excerpt}\n[/quote]" - - args = {} - args[:topic_id] = source_topic_id if source_topic_id > 0 - - PrettyText.cook(quote, args) - else - topic = Topic.find_by(id: route[:topic_id]) - return link unless can_see_topic?(topic, source_topic) - - first_post = topic.ordered_posts.first - - args = { - topic_id: topic.id, - avatar: PrettyText.avatar_img(topic.user.avatar_template, "tiny"), - original_url: @url, - title: PrettyText.unescape_emoji(CGI::escapeHTML(topic.title)), - category_html: CategoryBadge.html_for(topic.category), - quote: first_post.excerpt(SiteSetting.post_onebox_maxlength), - } - - template = File.read("#{Rails.root}/lib/onebox/templates/discourse_topic_onebox.hbs") - Mustache.render(template, args) - end - end - - def user_html(route) - link = "#{@url}" - username = route[:username] || '' - user = User.find_by(username_lower: username.downcase) - - if user - args = { - user_id: user.id, - username: user.username, - avatar: PrettyText.avatar_img(user.avatar_template, "extra_large"), - name: user.name, - bio: user.user_profile.bio_excerpt(230), - location: user.user_profile.location, - joined: I18n.t('joined'), - created_at: user.created_at.strftime(I18n.t('datetime_formats.formats.date_only')), - website: user.user_profile.website, - website_name: UserSerializer.new(user).website_name, - original_url: @url - } - - template = File.read("#{Rails.root}/lib/onebox/templates/discourse_user_onebox.hbs") - Mustache.render(template, args) - else - return link - end - end - - def can_see_post?(post, source_topic) - return false if post.nil? || post.hidden || post.trashed? || post.topic.nil? - Guardian.new.can_see_post?(post) || same_category?(post.topic.category, source_topic) - end - - def can_see_topic?(topic, source_topic) - return false if topic.nil? || topic.trashed? || topic.private_message? - Guardian.new.can_see_topic?(topic) || same_category?(topic.category, source_topic) - end - - def same_category?(category, source_topic) - source_topic.try(:category_id) == category.try(:id) - end - - end - end -end diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb index 121984f680d..64e1ee8b3c6 100644 --- a/lib/oneboxer.rb +++ b/lib/oneboxer.rb @@ -28,13 +28,13 @@ module Oneboxer def self.preview(url, options = nil) options ||= {} invalidate(url) if options[:invalidate_oneboxes] - onebox_raw(url)[:preview] + onebox_raw(url, options)[:preview] end def self.onebox(url, options = nil) options ||= {} invalidate(url) if options[:invalidate_oneboxes] - onebox_raw(url)[:onebox] + onebox_raw(url, options)[:onebox] end def self.cached_onebox(url) @@ -76,41 +76,22 @@ module Oneboxer doc end - def self.append_source_topic_id(url, topic_id) - # hack urls to create proper expansions - if url =~ Regexp.new("^#{Discourse.base_url.gsub(".", "\\.")}.*$", true) - uri = URI.parse(url) rescue nil - if uri && uri.path - route = Rails.application.routes.recognize_path(uri.path) rescue nil - if route && route[:controller] == 'topics' - url += (url =~ /\?/ ? "&" : "?") + "source_topic_id=#{topic_id}" - end - end - end - url - end - def self.apply(string_or_doc, args = nil) doc = string_or_doc doc = Nokogiri::HTML::fragment(doc) if doc.is_a?(String) changed = false each_onebox_link(doc) do |url, element| - if args && args[:topic_id] - url = append_source_topic_id(url, args[:topic_id]) - end - onebox, _preview = yield(url, element) + onebox, _ = yield(url, element) if onebox parsed_onebox = Nokogiri::HTML::fragment(onebox) next unless parsed_onebox.children.count > 0 # special logic to strip empty p elements - if element.parent && - element.parent.node_name && - element.parent.node_name.downcase == "p" && - element.parent.children.count == 1 + if element&.parent&.node_name&.downcase == "p" && element&.parent&.children&.count == 1 element = element.parent end + changed = true element.swap parsed_onebox.to_html end @@ -149,7 +130,104 @@ module Oneboxer "onebox__#{url}" end - def self.onebox_raw(url) + def self.onebox_raw(url, opts = {}) + local_onebox(url, opts) || external_onebox(url) + rescue => e + # no point warning here, just cause we have an issue oneboxing a url + # we can later hunt for failed oneboxes by searching logs if needed + Rails.logger.info("Failed to onebox #{url} #{e} #{e.backtrace}") + # return a blank hash, so rest of the code works + blank_onebox + end + + def self.local_onebox(url, opts = {}) + return unless route = Discourse.route_for(url) + + html = + case route[:controller] + when "uploads" then local_upload_html(url) + when "topics" then local_topic_html(url, route, opts) + when "users" then local_user_html(url, route) + end + + html = html.presence || "#{url}" + { onebox: html, preview: html } + end + + def self.local_upload_html(url) + case File.extname(URI(url).path || "") + when /^\.(mov|mp4|webm|ogv)$/i + "" + when /^\.(mp3|ogg|wav|m4a)$/i + "" + end + end + + def self.local_topic_html(url, route, opts) + return unless current_user = User.find_by(id: opts[:user_id]) + return unless current_category = Category.find_by(id: opts[:category_id]) + return unless Guardian.new(current_user).can_see_category?(current_category) + + if route[:post_number].to_i > 1 + post = Post.find_by(topic_id: route[:topic_id], post_number: route[:post_number]) + + return unless post.present? && !post.hidden + return unless current_category.id == post.topic.category_id || Guardian.new.can_see_post?(post) + + topic = post.topic + excerpt = post.excerpt(SiteSetting.post_onebox_maxlength) + excerpt.gsub!(/[\r\n]+/, " ") + excerpt.gsub!("[/quote]", "[quote]") # don't break my quote + + quote = "[quote=\"#{post.user.username}, topic:#{topic.id}, post:#{post.post_number}\"]\n#{excerpt}\n[/quote]" + + PrettyText.cook(quote) + else + return unless topic = Topic.find_by(id: route[:topic_id]) + return unless current_category.id == topic.category_id || Guardian.new.can_see_topic?(topic) + + first_post = topic.ordered_posts.first + + args = { + topic_id: topic.id, + avatar: PrettyText.avatar_img(topic.user.avatar_template, "tiny"), + original_url: url, + title: PrettyText.unescape_emoji(CGI::escapeHTML(topic.title)), + category_html: CategoryBadge.html_for(topic.category), + quote: first_post.excerpt(SiteSetting.post_onebox_maxlength), + } + + template = File.read("#{Rails.root}/lib/onebox/templates/discourse_topic_onebox.hbs") + Mustache.render(template, args) + end + end + + def self.local_user_html(url, route) + username = route[:username] || "" + + if user = User.find_by(username_lower: username.downcase) + args = { + user_id: user.id, + username: user.username, + avatar: PrettyText.avatar_img(user.avatar_template, "extra_large"), + name: user.name, + bio: user.user_profile.bio_excerpt(230), + location: user.user_profile.location, + joined: I18n.t('joined'), + created_at: user.created_at.strftime(I18n.t('datetime_formats.formats.date_only')), + website: user.user_profile.website, + website_name: UserSerializer.new(user).website_name, + original_url: url + } + + template = File.read("#{Rails.root}/lib/onebox/templates/discourse_user_onebox.hbs") + Mustache.render(template, args) + else + nil + end + end + + def self.external_onebox(url) Rails.cache.fetch(onebox_cache_key(url), expires_in: 1.day) do fd = FinalDestination.new(url, ignore_redirects: ignore_redirects, force_get_hosts: force_get_hosts) uri = fd.resolve @@ -169,14 +247,8 @@ module Oneboxer r = Onebox.preview(uri.to_s, options) - { onebox: r.to_s, preview: r.try(:placeholder_html).to_s } + { onebox: r.to_s, preview: r&.placeholder_html.to_s } end - rescue => e - # no point warning here, just cause we have an issue oneboxing a url - # we can later hunt for failed oneboxes by searching logs if needed - Rails.logger.info("Failed to onebox #{url} #{e} #{e.backtrace}") - # return a blank hash, so rest of the code works - blank_onebox end end diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index d2b90f5e4d7..618b69a349d 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -479,10 +479,11 @@ describe CookedPostProcessor do before do Oneboxer.expects(:onebox) - .with("http://www.youtube.com/watch?v=9bZkp7q19f0", post_id: 123, invalidate_oneboxes: true) + .with("http://www.youtube.com/watch?v=9bZkp7q19f0", invalidate_oneboxes: true, user_id: nil, category_id: post.topic.category_id) .returns("
    GANGNAM STYLE
    ") cpp.post_process_oneboxes end + it "inserts the onebox without wrapping p" do expect(cpp).to be_dirty expect(cpp.html).to match_html "
    GANGNAM STYLE
    " diff --git a/spec/components/onebox/engine/discourse_local_onebox_spec.rb b/spec/components/onebox/engine/discourse_local_onebox_spec.rb deleted file mode 100644 index efc60b97501..00000000000 --- a/spec/components/onebox/engine/discourse_local_onebox_spec.rb +++ /dev/null @@ -1,161 +0,0 @@ -require 'rails_helper' - -describe Onebox::Engine::DiscourseLocalOnebox do - - before { SiteSetting.external_system_avatars_enabled = false } - - def build_link(url) - %|#{url}| - end - - context "for a link to a post" do - let(:post) { Fabricate(:post) } - let(:post2) { Fabricate(:post, topic: post.topic, post_number: 2) } - - it "returns a link if post isn't found" do - url = "#{Discourse.base_url}/t/not-exist/3/2" - expect(Onebox.preview(url).to_s).to eq(build_link(url)) - end - - it "returns a link if not allowed to see the post" do - url = "#{Discourse.base_url}#{post2.url}" - Guardian.any_instance.expects(:can_see_post?).returns(false) - expect(Onebox.preview(url).to_s).to eq(build_link(url)) - end - - it "returns a link if post is hidden" do - hidden_post = Fabricate(:post, topic: post.topic, post_number: 2, hidden: true, hidden_reason_id: Post.hidden_reasons[:flag_threshold_reached]) - url = "#{Discourse.base_url}#{hidden_post.url}" - expect(Onebox.preview(url).to_s).to eq(build_link(url)) - end - - it "returns some onebox goodness if post exists and can be seen" do - url = "#{Discourse.base_url}#{post2.url}?source_topic_id=#{post2.topic_id + 1}" - html = Onebox.preview(url).to_s - expect(html).to include(post2.excerpt) - expect(html).to include(post2.topic.title) - - url = "#{Discourse.base_url}#{post2.url}/?source_topic_id=#{post2.topic_id + 1}" - html = Onebox.preview(url).to_s - expect(html).to include(post2.excerpt) - expect(html).to include(post2.topic.title) - - html = Onebox.preview("#{Discourse.base_url}#{post2.url}").to_s - expect(html).to include(post2.user.username) - expect(html).to include(post2.excerpt) - end - end - - context "for a link to a topic" do - let(:post) { Fabricate(:post) } - let(:topic) { post.topic } - - it "returns a link if topic isn't found" do - url = "#{Discourse.base_url}/t/not-found/123" - expect(Onebox.preview(url).to_s).to eq(build_link(url)) - end - - it "returns a link if not allowed to see the topic" do - url = topic.url - Guardian.any_instance.expects(:can_see_topic?).returns(false) - expect(Onebox.preview(url).to_s).to eq(build_link(url)) - end - - it "replaces emoji in the title" do - topic.update_column(:title, "Who wants to eat a :hamburger:") - expect(Onebox.preview(topic.url).to_s).to match(/hamburger\.png/) - end - - it "returns some onebox goodness if topic exists and can be seen" do - html = Onebox.preview(topic.url).to_s - expect(html).to include(topic.ordered_posts.first.user.username) - expect(html).to include("
    ") - - html = Onebox.preview("#{topic.url}/?u=codinghorror").to_s - expect(html).to include(topic.ordered_posts.first.user.username) - expect(html).to include("
    ") - end - end - - context "for a link to a user profile" do - let(:user) { Fabricate(:user) } - - it "returns a link if user isn't found" do - url = "#{Discourse.base_url}/u/none" - expect(Onebox.preview(url).to_s).to eq(build_link(url)) - end - - it "returns some onebox goodness if user exists" do - html = Onebox.preview("#{Discourse.base_url}/u/#{user.username}").to_s - expect(html).to include(user.username) - expect(html).to include(user.name) - expect(html).to include(user.created_at.strftime("%B %-d, %Y")) - expect(html).to include('