Previously we had a partial fix in place where non human users
were not allowed draft sequences, this left edges around where non
human users asked for drafts yet had none.
For example system could already have a few drafts in place.
This also removes and extensibility point we added that is not in use
This reverts commit 20780a1eee.
* SECURITY: re-adds accidentally reverted commit:
03d26cd6: ensure embed_url contains valid http(s) uri
* when the merge commit e62a85cf was reverted, git chose the 2660c2e2 parent to land on
instead of the 03d26cd6 parent (which contains security fixes)
* We now have a site setting "topic_excerpt_maxlength" that is used when the OP is created or revised to generate a topic excerpt.
* However, posts created before this setting was introduced cannot benefit from this change unless they are revised, and if the topic excerpt length setting is changed that situation is also not covererd.
* This PR makes a change to rebake! to update the topic excerpt IF the post is the OP.
Adds a new topic_excerpt_maxlength site setting.
* When topic excerpt is requested for a post, use the new topic_excerpt_maxlength site setting to limit the size of the excerpt
* Remove code for getting/setting Post.excerpt_size as it is not used anywhere
* FIX: Emit web hooks for flags
* FEATURE: Remove 'flag' web hook in favor of 'reviewable' web hook
* FEATURE: Remove 'queued post' web hook in favor of 'reviewable' web hook
* FIX: Do not set a default value for web hooks with no events
This commit reorganises the delete dependencies on users and make sure
all are covered. We forgot some on bookmarks, security keys, anon users
and so on.
If image uploads are missing a width/height, we re-download them to update the width/height in the database. However, this takes significant resources so we don't want to run this logic during topic serialization.
* DEV: Add framework for filtered plugin registers
Plugins often need to add values to a list, and we need to filter those lists at runtime to ignore values from disabled plugins. This commit provides a re-usable way to do that, which should make it easier to add new registers in future, and also reduce repeated code.
Follow-up commits will migrate existing registers to use this new system
* DEV: Migrate user and group custom field APIs to plugin registry
This gives us a consistent system for checking plugin enabled state, so we are repeating less logic. API changes are backwards compatible
Previously we would raise a warning in the logs if downloading
a file (from s3) takes longer than 60 seconds.
At scale this happens reasonably frequently.
1. Raised the duration to 3 minutes
2. Pulled the resizing mutex out of the downloading mutex
so we have less and clearer error logs
* PERF: Dematerialize topic_reply_count
It's only ever used for trust level promotions that run daily, or compared to 0. We don't need to track it on every post creation.
* UX: Add symbol in TL3 report if topic reply count is capped
* DEV: Drop user_stats.topic_reply_count column
Previously the code was very race condition prone leading to
odd failures in production
It was re-written in raw SQL to avoid conditions where rows
conflict on inserts
There is no clean way in ActiveRecord to do:
Insert, on conflict do nothing and return existing id.
This also increases test coverage, we were previously not testing
the code responsible for crawling external sites directly
We were getting errors like this in Reviewables in some cases:
```
ActiveRecord::StatementInvalid (PG::AmbiguousColumn: ERROR: column reference "category_id" is ambiguous
LINE 4: ...TRUE) OR (reviewable_by_group_id IN (NULL))) AND (category_i...
```
The problem that was making everything go boom is that plugins can add their own custom filters for Reviewables. If one is doing an INNER JOIN on topics, which has its own category_id column, we would get the above AmbiguousColumn error. The solution here is to just make all references to the reviewable columns in the list_for and viewable_by code prefixed by the table name e.g. reviewables.category_id.
This is so that, on a multisite cluster, when we handle a CDN request,
the hostname that is requested corresponds to one of the sites -
specifically the default site.
Previously we only changed sequence on ownership change, this
cause a race condition between tabs where user could type for a
long time without being warned of an out of date draft.
This change is a radical change and we should watch closely.
Code was already in place to track sequence on the client so no
changes are needed there.
We now show an options gear icon next to the bookmark name.
When expanded we show the "delete bookmark when reminder sent" option. The value of this checkbox is saved in local storage for the user.
If this is ticked, when a reminder is sent for the bookmark the bookmark itself is deleted. This is so people can use the reminder functionality by itself.
Also remove the blue alert reminder section from the "Edit Bookmark" modal as it just added clutter, because the user can already see they had a reminder set:
Adds a default false boolean column `delete_when_reminder_sent` to bookmarks.
Previously the image was imported from a Discourse hosted CDN but the
URL has since become invalid. However, it was not caught since all
errors are rescued. This commit fixes the issue by shipping the user
avatar with the plugin.
We were sharing `Discourse` both as an application object and a
namespace which complicated things for Ember CLI. This patch
moves raw templates into `__DISCOURSE_RAW_TEMPLATES` and adds
a couple helper methods to create/remove them.
This introduces new APIs for obtaining optimized thumbnails for topics. There are a few building blocks required for this:
- Introduces new `image_upload_id` columns on the `posts` and `topics` table. This replaces the old `image_url` column, which means that thumbnails are now restricted to uploads. Hotlinked thumbnails are no longer possible. In normal use (with pull_hotlinked_images enabled), this has no noticeable impact
- A migration attempts to match existing urls to upload records. If a match cannot be found then the posts will be queued for rebake
- Optimized thumbnails are generated during post_process_cooked. If thumbnails are missing when serializing a topic list, then a sidekiq job is queued
- Topic lists and topics now include a `thumbnails` key, which includes all the available images:
```
"thumbnails": [
{
"max_width": null,
"max_height": null,
"url": "//example.com/original-image.png",
"width": 1380,
"height": 1840
},
{
"max_width": 1024,
"max_height": 1024,
"url": "//example.com/optimized-image.png",
"width": 768,
"height": 1024
}
]
```
- Themes can request additional thumbnail sizes by using a modifier in their `about.json` file:
```
"modifiers": {
"topic_thumbnail_sizes": [
[200, 200],
[800, 800]
],
...
```
Remember that these are generated asynchronously, so your theme should include logic to fallback to other available thumbnails if your requested size has not yet been generated
- Two new raw plugin outlets are introduced, to improve the customisability of the topic list. `topic-list-before-columns` and `topic-list-before-link`
Recently, we added feature that we are sending `/muted` to users who muted specific topic just before `/latest` so the client knows to ignore those messages - https://github.com/discourse/discourse/pull/9482
Same `/muted` message should be included when the post is edited
* FEATURE: add category banner for why a user cannot post
Adds a category banner for why a user is unable to post in a category.
Also adds an extra alert for the user when a user is unable to create a topic in a
category and they still try and click on the disabled-looking new topic
button.
We have the `# frozen_string_literal: true` comment on all our
files. This means all string literals are frozen. There is no need
to call #freeze on any literals.
For files with `# frozen_string_literal: true`
```
puts %w{a b}[0].frozen?
=> true
puts "hi".frozen?
=> true
puts "a #{1} b".frozen?
=> true
puts ("a " + "b").frozen?
=> false
puts (-("a " + "b")).frozen?
=> true
```
For more details see: https://samsaffron.com/archive/2018/02/16/reducing-string-duplication-in-ruby
avg_time on posts and topics have not been used in a year.
This uses a re-runnable ddl transaction diasabled migration to
drop the column, cause it touchs very high traffic table and may
deadlock
Previously we relied entirely on levenshtein_distance_spammer_emails site
setting to handle "similar looking" emails.
This commit improves the situation by always preferring to block (and check)
canonical emails.
This means that if:
`samevil+test@domain.com` is blocked the system will block `samevil@domain.com`
This means that `samevil+2@domain.com` (ad infinitum) will be blocked
This reverts commit 6f9177e2ed.
We decided on a completely different approach to the problem.
Instead we will let blocked emails be treated as canonical.
* When copying the markdown for an image between posts, we were not adding the srcset and data-small-image attributes which are done by calling optimize_image! in cooked post processor
* Refactored the code which was confusing in its current state (the consider_for_reuse method was super confusing) and fixed the issue
* FEATURE: don't display new/unread notification for muted topics
Currently, even if user mute topic, when a new reply to that topic arrives, the user will get "See 1 new or updated topic" message. After clicking on that link, nothing is visible (because the topic is muted)
To solve that problem, we will send background message to all users who recently muted that topic that update is coming and they can ignore the next message about that topic.
The main thrust of this PR is to take all the conditional checks based on the `enable_bookmarks_with_reminders` away and only keep the code from the `true` path, making bookmarks with reminders the core bookmarks feature. There is also a migration to create `Bookmark` records out of `PostAction` bookmarks for a site.
### Summary
* Remove logic based on whether enable_bookmarks_with_reminders is true. This site setting is now obsolete, the old bookmark functionality is being removed. Retain the setting and set the value to `true` in a migration.
* Use the code from the rake task to create a database migration that creates bookmarks from post actions.
* Change the bookmark report to read from the new table.
* Get rid of old endpoints for bookmarks
* Link to the new bookmarks list from the user summary page
* DEV: Use `render_json_error` (Adds specs for Admin::GroupsController)
* DEV: Use a specific error on blank category slug (Fixes a `render_json_error` warning)
* DEV: Use a specific error on reviewable claim conflict (Fixes a `render_json_error` warning)
* DEV: Use specific errors in Admin::UsersController (Fixes `render_json_error` warnings)
* FIX: PublishedPages error responses
* FIX: TopicsController error responses (There was an issue of two separate `Topic` instances for the same record. This makes sure there's only one up-to-date instance.)
Within 24 hours of signing up, new users were losing their
default trust level of 3. With this fix, demotions from
trust level 3 won't happen when the "default trust level"
setting is 3 or 4.
* Count user summary bookmarks from new Bookmark table if bookmarks with reminders enabled
* Update topic user bookmarked column when new topic bookmark changed
* Make in:bookmarks search work with new bookmarks
* Fix batch inserts for bookmark rake task (and thus migration). We were only inserting one bookmark at a time, completely defeating the purpose of batching!
The new `enforce_canonical_emails` site setting ensures that emails in the
canonical form are unique.
This mean that if `s.a.m+1@gmail.com` is registered `sam@gmail.com` will
not be allowed.
The commit contains a blanket "tag strip" (stripping everything after +)
it also contains special handling of a "dot strip" for googlemail and gmail.
The setting only impacts new registrations after `enforce_canonical_emails`
The setting is default false so it will not impact any existing installs.
If the feature is enabled, staff members can construct a URL and publish a
topic for others to browse without the regular Discourse chrome.
This is useful if you want to use Discourse like a CMS and publish
topics as articles, which can then be embedded into other systems.
Previously we were only updating group membership when a user record was
first created in an SSO setting.
This corrects it so we also update it if SSO changes the email
It wasn't intended that people should be able to earn trust level
3 without participating in public topics. When counting topic
views and likes given/received, don't count private topics.
Users noticed that sometimes, avatar from Gravatar is not correctly updated - https://meta.discourse.org/t/updated-image-on-gravatar-not-seeing-it-update-on-site/54357
A potential reason for that is that even if you update your avatar in Gravatar, URL stays the same and if the cache is involved, service is still receiving the old photo.
For example. In my case, when I click the button to refresh avatar the
new Upload record is created with `origin` URL to new avatar, and `url` to
old one
I made some tests in the rails console and adding random param to Gravatar URL is deceiving cache and correct, the newest avatar is downloaded
Previously all topic posters would be added which could lead to major performance issues. Now if there are too many posters, only the acting user will be added as a participant.
New Reminder Types
-------------------------------------
* Add a "later this week" reminder which is today + 2 days, will not show if we are on the days Thu-Sun
* Add a "start of next business week" reminder which is 8am Monday
Bugfixes and Tweaks
--------------------------------------
* Move dates out of translation for reminder types and yield HTML for tap-tile for more customizable content and styling
* Make sure double clicking the bookmark icon in quick access takes users to the new bookmarks-with-reminders page
* Sane default to 8am (start of day) for custom reminder with no time
This is so users with huge amount of bookmarks do not have to wait a long time to see results.
* Add a bookmark list and list serializer to server-side to be able to handle paging and load more URL
* Use load-more component to load more bookmark items, 20 at a time in user activity
* Change the way current user is loaded for bookmark ember models because it was breaking/losing resolvedTimezone when loading more items
Introduce the concept of "high priority notifications" which include PM and bookmark reminder notifications. Now bookmark reminder notifications act in the same way as PM notifications (float to top of recent list, show in the green bubble) and most instances of unread_private_messages in the UI have been replaced with unread_high_priority_notifications.
The user email digest is changed to just have a section about unread high priority notifications, the unread PM section has been removed.
A high_priority boolean column has been added to the Notification table and relevant indices added to account for it.
unread_private_messages has been kept on the User model purely for backwards compat, but now just returns unread_high_priority_notifications count so this may cause some inconsistencies in the UI.
* FEATURE: add setting `auto_approve_email_domains` to auto approve users
This commit adds a new site setting `auto_approve_email_domains` to
auto approve users based on their email address domain.
Note that if a domain already exists in `email_domains_whitelist` then
`auto_approve_email_domains` needs to be duplicated there as well,
since users won’t be able to register with email address that is
not allowed in `email_domains_whitelist`.
* Update config/locales/server.en.yml
Co-Authored-By: Robin Ward <robin.ward@gmail.com>
Moves the new draft creation concurrency handling to PostgreSQL
so the database doesn't error out when the draft is being created
by multiple backends.
Also removes `retry_not_unique` parameter from Draft#set` which is
not called anywhere.
Also fixes a draft update not bumping the `updated_at` column.
New `duration` attribute is introduced for the `set_or_create_timer` method in the commit aad12822b7 for "based on last post" and "auto delete replies" topic timers.
* DEV: Replace User.unstage and User#unstage API with User#unstage!
Quoting @SamSaffron:
> User.unstage mixes concerns of both unstaging users and updating params which is fragile/surprising.
> u.unstage destroys notifications and raises a user_unstaged event prior to the user becoming unstaged and the user object being saved.
User#unstage! no longer updates user attributes and saves the object before triggering the `user_unstaged` event.
* Update one more spec
* Assign attributes after unstaging
On some sites when bootstrapping communities it is helpful to bootstrap
with a "light weight" invite code.
Use the site setting `invite_code` to set a global invite code.
In this case the administrator can share the code with
a community which is very easy to remember and then anyone who has
that code can easily register accounts.
People without the invite code are not allowed account registration.
Global invite codes are less secure than indevidual codes, in that they
tend to leak in the community however in some cases when starting a brand
new community the security guarantees of invites are not needed.
Having a tag be a member of a tag group and the group's parent tag at
the same time causes some unexpected behavior. When a tag is assigned
as the parent, remove it from the group.
Adds 3 config values that allow to set a custom provider of Gravatar-like API accessible from gravatar_base_url. The gravatar_name is purely cosmetic, but helps with associating name with the service that actually provides the avatars. gravatar_login_url is a link relative to gravatar_base_url, which provides the user with the login to the Gravatar service
* This PR implements the scheduling and notification system for bookmark reminders. Every 5 minutes a schedule runs to check any reminders that need to be sent before now, limited to **300** reminders at a time. Any leftover reminders will be sent in the next run. This is to avoid having to deal with fickle sidekiq and reminders in the far-flung future, which would necessitate having a background job anyway to clean up any missing `enqueue_at` reminders.
* If a reminder is sent its `reminder_at` time is cleared and the `reminder_last_sent_at` time is filled in. Notifications are only user-level notifications for now.
* All JavaScript and frontend code related to displaying the bookmark reminder notification is contained here. The reminder functionality is now re-enabled in the bookmark modal as well.
* This PR also implements the "Remind me next time I am at my desktop" bookmark reminder functionality. When the user is on a mobile device they are able to select this option. When they choose this option we set a key in Redis saying they have a pending at desktop reminder. The next time they change devices we check if the new device is desktop, and if it is we send reminders using a DistributedMutex. There is also a job to ensure consistency of these reminders in Redis (in case Redis drops the ball) and the at desktop reminders expire after 20 days.
* Also in this PR is a fix to delete all Bookmarks for a user via `UserDestroyer`
There are three modifiers:
- serialize_topic_excerpts (boolean)
- csp_extensions (array of strings)
- svg_icons (array of strings)
When multiple themes are active, the values will be combined. The combination method varies based on the setting. CSP/SVG arrays will be combined. serialize_topic_excerpts will use `Enumerable#any`.
* Do not grant badges for posts with no user
* Ensure instructions are correct in Change Owner modal
* Hide user-dependent actions from posts with no user
* Make PostRevisor work with posts with no user
* Ensure posts with no user can be deleted
* discourse-narrative-bot should ignore posts with no user
* Skip TopicLink creation for posts with no user
This commit ensures that an error is thrown when a user fails to be
removed from a group instead of silently failing.
This means when using the api you will receive a 400 instead of a 200 if
there is a failure. The remove group endpoint allows the removal of
multiple users, this change means that if you try to delete 10 users,
but 1 of them fails you will receive a 400 instead of 200 even though
the other 9 were removed successfully. Rather than adding a bunch more
complexity I think this is more than adequate for most use cases.
* DEV: Reserve webhook event types to be used in plugins
Based on feedback on the following PR's:
https://github.com/discourse/discourse-solved/pull/85https://github.com/discourse/discourse-assign/pull/61
This commit reserves ID's to be used for webhook event types to ensure
that some other webhook or plugin doesn't end up using the same ID.
* Fix broken test
I don't think this test has to test ALL event types to verify that this
feature is working. Now that we added some event types that plugins are
using this test was failing for missing fabricators that exist in the
respective plugins.
* remove loop and just test first record
* Add uploads:sync_s3_acls rake task to ensure the ACLs in S3 are the correct (public-read or private) setting based on upload security
* Improved uploads:disable_secure_media to be more efficient and provide better messages to the user.
* Rename uploads:ensure_correct_acl task to uploads:secure_upload_analyse_and_update as it does more than check the ACL
* Many improvements to uploads:secure_upload_analyse_and_update
* Make sure that upload.access_control_post is unscoped so deleted posts are still fetched, because they still affect the security of the upload.
* Add escape hatch for capture_stdout in the form of RAILS_ENABLE_TEST_STDOUT. If provided the capture_stdout code will be ignored, so you can see the output if you need.
* PERF: Allow passing an existing list of user field ids when loading
This avoids the need for running `UserField.pluck(:id)` for each user that is serialized
* Memoize user_fields to avoid rebuilding hash ever time
The server and client used two different formats for preload keys. The
server was using 'topic_list_c/SLUG/l/latest', but the client was using
'topic_list_c/SLUG/ID/l/latest'.
This commit is an addition to 374534f00e.
Introduces a new site setting `max_notifications_per_user`.
Out-of-the-box this is set to 10,000. If a user exceeds this number of
notifications, we will delete the oldest notifications keeping only 10,000.
To disable this safeguard set the setting to 0.
Enforcement happens weekly.
This is in place to protect the system from pathological states where a
single user has enormous amounts of notifications causing various queries
to time out. In practice nobody looks back more than a few hundred notifications.
This is because the TOTP gem identifies as a colon as an addressable
protocol. The solution for now is to remove the colon in the issuer
name.
Changing the issuer changes the token values, but now it was completely
broken for colons so this should not be breaking anyone new.
Previously we had many places in the app that called `hostname` to get
hostname of a server. This commit replaces the pattern in 2 ways
1. We cache the result in `Discourse.os_hostname` so it is only ever called once
2. We prefer to use Socket.gethostname which avoids making a shell command
This improves performance as we are not spawning hostname processes throughout
the app lifetime
* Because custom emoji count as post "uploads" we were
marking them as secure when updating the secure status for post uploads.
* We were also giving them an access control post id, which meant
broken image previews from 403 errors in the admin custom emoji list.
* We now check if an upload is used as a custom emoji and do not
assign the access control post + never mark as secure.
### UI Changes
If `SiteSetting.enable_bookmarks_with_reminders` is enabled:
* Clicking "Bookmark" on a topic will create a new Bookmark record instead of a post + user action
* Clicking "Clear Bookmarks" on a topic will delete all the new Bookmark records on a topic
* The topic bookmark buttons control the post bookmark flags correctly and vice-versa
Disabled selecting the "reminder type" for bookmarks in the UI because the backend functionality is not done yet (of sending users notifications etc.)
### Other Changes
* Added delete bookmark route (but no UI yet)
* Added a rake task to sync the old PostAction bookmarks to the new Bookmark table, which can be run as many times as we want for a site (it will not create duplicates).
This is not used in core or official plugins, and has been printing a deprecation notice since v2.3.0beta4. All OpenID 2.0 code and dependencies have been dropped. The user_open_ids table remains for now, in case anyone has missed the deprecation notice, and needs to migrate their data.
Context at https://meta.discourse.org/t/-/113249
* DEV: Use Ember 3.12.2
* Add Ember version to ThemeField's DEPENDENT_CONSTANTS
* DEV: Use `id` instead of `elementId` (See: https://github.com/emberjs/ember.js/issues/18147)
* FIX: Don't leak event listeners (bug introduced in 999e2ff)
This fix ensures that the site setting `post_edit_time_limit` does not
bypass the limit of the site setting `min_trust_to_edit_post`. This
prevents a bug where users that did not meet the minimum trust level to
edit could edit the title of topics.
Previously you'd get a server side generic error due to a password check
failing. Now the input element has a maxlength attribute and the server
side will respond with a nicer error message if the value is too long.
If our reply tree somehow ends up with cycles or other odd
structures, we only want to consider a reply once, at the first
level in the tree that it appears.
* DEV: Add data-notification-level attribute to category UI
* Show muted categories on the category page by default
This reverts commit ed9c21e42c.
* Remove redundant spec - muted categories are now visible by default
It seems in some situations replies have been moved to other topics but
the `PostReply` table has not been updated. I will try and fix this in a
follow up PR, but for now this fix ensures that every time we ask a post
for its replies that we restrict it to the same topic.
For example /t/ URLs were being replaced if they contained secure-media-uploads so if you made a topic called "Secure Media Uploads Are Cool" the View Topic link in the user notifications would be stripped out.
Refactored code so this secure URL detection happens in one place.
Basically, say you had already downloaded a certain image from a certain URL
using pull_hotlinked_images and the onebox. The upload would be stored
by its sha as an upload record. Whenever you linked to the same URL again
in a post (e.g. in our case an og:image on review.discourse) we would
would reuse the original upload record because of the sha1.
However when you turned on secure media this could cause problems as
the first post that uses that upload after secure media is enabled
will set the access control post for the upload to the new post.
Then if the post is deleted every single onebox/link to that same image
URL will fail forever with 403 as the secure-media-uploads URL fails
if the access control post has been deleted.
To fix this when cooking posts and pulling hotlinked images, we only
allow using an original upload by URL if its access control post
matches the current post, and if the original_sha1 is filled in,
meaning it was uploaded AFTER secure media was enabled. otherwise
we just redownload the media again to be safe, as the URL will always
be new then.
When pull_hotlinked_images tried to run on posts with secure media (which had already been downloaded from external sources) we were getting a 404 when trying to download the image because the secure endpoint doesn't allow anon downloads.
Also, we were getting into an infinite loop of pull_hotlinked_images because the job didn't consider the secure media URLs as "downloaded" already so it kept trying to download them over and over.
In this PR I have also refactored secure-media-upload URL checks and mutations into single source of truth in Upload, adding a SECURE_MEDIA_ROUTE constant to check URLs against too.
Add TopicUploadSecurityManager to handle post moves. When a post moves around or a topic changes between categories and public/private message status the uploads connected to posts in the topic need to have their secure status updated, depending on the security context the topic now lives in.
ReviewableScore#types extend the PostActionTypes with their own, storing the result inside a class variable. To avoid overwriting an existing flag, we need to calculate the next flag ID using these types instead of the PostAction ones. Since we first call the score types to calculate the id, this list gets memoized, leaving us with an outdated list.
To fix this, we now reload ReviewableScore#types after replacing flags.
### General Changes and Duplication
* We now consider a post `with_secure_media?` if it is in a read-restricted category.
* When uploading we now set an upload's secure status straight away.
* When uploading if `SiteSetting.secure_media` is enabled, we do not check to see if the upload already exists using the `sha1` digest of the upload. The `sha1` column of the upload is filled with a `SecureRandom.hex(20)` value which is the same length as `Upload::SHA1_LENGTH`. The `original_sha1` column is filled with the _real_ sha1 digest of the file.
* Whether an upload `should_be_secure?` is now determined by whether the `access_control_post` is `with_secure_media?` (if there is no access control post then we leave the secure status as is).
* When serializing the upload, we now cook the URL if the upload is secure. This is so it shows up correctly in the composer preview, because we set secure status on upload.
### Viewing Secure Media
* The secure-media-upload URL will take the post that the upload is attached to into account via `Guardian.can_see?` for access permissions
* If there is no `access_control_post` then we just deliver the media. This should be a rare occurrance and shouldn't cause issues as the `access_control_post` is set when `link_post_uploads` is called via `CookedPostProcessor`
### Removed
We no longer do any of these because we do not reuse uploads by sha1 if secure media is enabled.
* We no longer have a way to prevent cross-posting of a secure upload from a private context to a public context.
* We no longer have to set `secure: false` for uploads when uploading for a theme component.
FIX: raised a proper NotFound exception when filtering groups by username with invalid username.
FIX: properly filter the groups based on current user visibility when viewing another user's groups.
DEV: Guardian.can_see_group?(group) is now using Guardian.can_see_groups(groups) instead of duplicating the same code.
FIX: spec for groups_controller#index when group directory is disabled for logged in user.
FIX: groups_controller.sortable specs to actually test all sorting combinations.
DEV: s/response_body/body/g for slightly shorter spec code.
FIX: rewrote the "view another user's groups" specs to test all group_visibility and members_group_visibility combinations.
DEV: Various refactoring for cleaner and more consistent code.
The previous concurrency-safe implementation relied on catching an
index conflict and following through appropriately. Unfortunately
those conflicts were logged to Postgres and there is no easy way
to turn them off.
This solution approaches the problem differently. It should still
be safe under concurrency and not log errors.
I was playing with groups locally and saw this line. I suspect this method isn't needed at all because I don't see any reference to it anywhere in the code, and as far as I know ActiveRecord objects don't have an `id!` method so if this method is called dynamically somewhere it's most likely failing.
6e1fe22 introduced the possiblity for category_users to have a NULL notification_level, so that we can store `last_seen_at` dates without locking the notification level. At the time, this did not affect the topic-tracking-state query. However, the query changes in f434de2 introduced a slight change in behavior.
Previously, a subquery would look for a category_user with notification_level=mute. f434de2 refactored this to remove the subquery, and inverted some of the logic to suit.
The new query checked for `notification_level <> :muted`. If `notification_level` is NULL, this comparison will return NULL. In this scenario, notification_level=NULL means that we should fall back to the default tracking level (regular), and so we want the expression to resolve as true, not false. There was already a check for the existence of the category_users row, but it did not check for the existence of a NOT NULL notification_level.
This commit amends the expression so that the notification_level will only be compared if it is non-null.
Under rare conditions due to bad HTTP timing and so on a draft could be
set at the exact same time from 2 unicorn workers.
When this happened and all the stars aligned, one of the sets would win
and the other would raise an error.
This transparently handles the situation without adding any cost to the
draft system.
The alternative is to add a distributed mutex, tricky DB transaction or handle
the error in the controller. However this seems like a reasonable way to
work around a pretty big edge case.
Previously, categories without any topics were being excluded from the UPDATE query. This means the counter gets stuck, and the category cannot be deleted. This change ensures that the counters get correctly set to zero.
The bug mentioned here
https://meta.discourse.org/t/badge-not-triggering/135896/8
Basically, descriptions for 3 badges: "Out of Love", "Higher Love" and
"Crazy in Love" are granted based on on "max_likes_per_day" and the
description should reflect that.
Restore tl3 reachability by calculating penalty counts vs unsuspend/unsilence.
Only count unsuspend or unsilences that have been made by a user other than
the Discourse system user.
- Using h4 instead of h3 for sub-categories.
- Show category description if it does not have subcategories.
- Implemented equivalent for mobile-view.
- Include description_excerpt in basic serializer. This is needed for
displaying second-level categories in category list.
Follow-up to 9253cb79e3.
The ROTP gem is only used in a very small amount of places in the app, we don't need to globally require it.
Also set the Addressable gem to not have a specific version range, as it has not been a problem yet.
Some slight refactoring of UserSecondFactor here too to use SecondFactorManager to avoid code repetition
API keys are now only visible when first created. After that, only the first four characters are stored in the database for identification, along with an sha256 hash of the full key. This makes key usage easier to audit, and ensures attackers would not have access to the live site in the event of a database leak.
This makes the merge lower risk, because we have some time to revert if needed. Once the change is confirmed to be working, we will add a second commit to drop the `key` column.
The following methods have long been deprecated in ruby due to flaws in their implementation per http://blade.nagaokaut.ac.jp/cgi-bin/vframe.rb/ruby/ruby-core/29293?29179-31097:
URI.escape
URI.unescape
URI.encode
URI.unencode
escape/encode are just aliases for one another. This PR uses the Addressable gem to replace these methods with its own encode, unencode, and encode_component methods where appropriate.
I have put all references to Addressable::URI here into the UrlHelper to keep them corralled in one place to make changes to this implementation easier.
Addressable is now also an explicit gem dependency.
Note: All of this functionality is hidden behind a hidden, default false, site setting called `enable_bookmarks_with_reminders`. Also, any feedback on Ember code would be greatly appreciated!
This is part 1 of the bookmark improvements. The next PR will address the backend logic to send reminder notifications for bookmarked posts to users. This PR adds the following functionality:
* We are adding a new `bookmarks` table and `Bookmark` model to make the bookmarks a first-class citizen and to allow attaching reminders to them.
* Posts now have a new button in their actions menu that has the icon of an actual book
* Clicking the button opens the new bookmark modal.
* Both name and the reminder type are optional.
* If you close the modal without doing anything, the bookmark is saved with no reminder.
* If you click the Cancel button, no bookmark is saved at all.
* All of the reminder type tiles are dynamic and the times they show will be based on your user timezone set in your profile (this should already be set for you).
* If for some reason a user does not have their timezone set they will not be able to set a reminder, but they will still be able to create a bookmark.
* A bookmark can be deleted by clicking on the book icon again which will be red if the post is bookmarked.
This PR does NOT do anything to migrate or change existing bookmarks in the form of `PostActions`, the two features live side-by-side here. Also this does nothing to the topic bookmarking.
We like to stay as close as possible to latest with rubocop cause the cops
get better.
This update required some code changes, specifically the default is to avoid
explicit returns where implicit is done
Also this renames a few rules
When the tag is muted and topic contains that tag, we should not mark that message as NEW.
There are 3 possible settings which site admin can set.
remove_muted_tags_from_latest - always
It means that if the topic got at least one muted tag, we should not mark that topic as NEW
remove_muted_tags_from_latest - only muted
Similar to above, however, if at least one tag is not muted, the topic is marked as NEW
remove_muted_tags_from_latest - never
Basically, mute tag setting is ignored and all topics are set as NEW
This feature adds the ability to define synonyms for tags, and the ability to merge one tag into another while keeping it as a synonym. For example, tags named "js" and "java-script" can be synonyms of "javascript". When searching and creating topics using synonyms, they will be mapped to the base tag.
Along with this change is a new UI found on each tag's page (for example, `/tags/javascript`) where more information about the tag can be shown. It will list the synonyms, which categories it's restricted to (if any), and which tag groups it belongs to (if tag group names are public on the `/tags` page by enabling the "tags listed by group" setting). Staff users will be able to manage tags in this UI, merge tags, and add/remove synonyms.
* FEATURE: Ability to add components to all themes
This is the first and functional step from that topic https://dev.discourse.org/t/adding-a-theme-component-is-too-much-work/15398/16
The idea here is that when a new component is added, the user can easily assign it to all themes (parents).
To achieve that, I needed to change a site-setting component to accept `setDefaultValues` action and `setDefaultValuesLabel` translated label.
Also, I needed to add `allowAny` option to disable that for theme selector.
I also refactored backend to accept both parent and child ids with one method to avoid duplication (Renamed `add_child_theme!` to more general `add_relative_theme!`)
* FIX: Improvement after code review
* FIX: Improvement after code review2
* FIX: use mapBy and filterBy directly
Note:
```
def foo(bar: 1)
end
foo({bar: 2})
# raises a deprecation, instead use:
foo(**{bar: 2})
```
Additionally when matching regexes always use strings. It does not make
sense to match a non string to a regex.
When uploading a file to a theme component, and that file is existing and has already been marked as secure, we now automatically mark the file as secure: false, change the ACL, and log the action as the user (also rebake the posts for the upload)
If a user has more than 60 active sessions, the oldest sessions will be terminated automatically. This protects performance when logging in and when loading the list of recently used devices.
This is a bottom up rewrite of Discourse cache to support faster performance
and a limited surface area.
ActiveSupport::Cache::Store accepts many options we do not use, this partial
implementation only picks the bits out that we do use and want to support.
Additionally params are named which avoids typos such as "expires_at" vs "expires_in"
This also moves a few spots in Discourse to use Discourse.cache over setex
Performance of setex and Discourse.cache.write is similar.
Discourse.cache is a more consistent method to use and offers clean fallback
if you are skipping redis
This is part of a larger change that both optimizes Discoruse.cache and omits
use of setex on $redis in favor of consistently using discourse cache
Bench does reveal that use of Rails.cache and Discourse.cache is 1.25x slower
than redis.setex / get so a re-implementation will follow prior to porting
* Add timezone to user_options table
* Also migrate existing timezone values from UserCustomField,
which is where the discourse-calendar plugin is storing them
* Allow user to change their core timezone from Profile
* Auto guess & set timezone on login & invite accept & signup
* Serialize user_options.timezone for group members. this is so discourse-group-timezones can access the core user timezone, as it is being removed in discourse-calendar.
* Annotate user_option with timezone
* Validate timezone values
When category is dismissed, `dismiss_new` message is sent to fronted to clean state.
In addition, I noticed that when old dismiss new button is clicked, no message is sent so I decided to kill two birds with one stone.
In non-login-required sites, we prevent secure uploads already used in PMs from being used in public topics.
In login_required sites, secure uploads should be reusable in any topic, PM or not.
- Avoid using User#find and then fetching the user_stat association just to update a counter - We did this for every post in the topic.
- Avoid having to query the topic_allowed_user table everytime we want add a new topic_allowed_user
- Use pluck to fetch just what we need.
This PR introduces a new secure media setting. When enabled, it prevent unathorized access to media uploads (files of type image, video and audio). When the `login_required` setting is enabled, then all media uploads will be protected from unauthorized (anonymous) access. When `login_required`is disabled, only media in private messages will be protected from unauthorized access.
A few notes:
- the `prevent_anons_from_downloading_files` setting no longer applies to audio and video uploads
- the `secure_media` setting can only be enabled if S3 uploads are already enabled and configured
- upload records have a new column, `secure`, which is a boolean `true/false` of the upload's secure status
- when creating a public post with an upload that has already been uploaded and is marked as secure, the post creator will raise an error
- when enabling or disabling the setting on a site with existing uploads, the rake task `uploads:ensure_correct_acl` should be used to update all uploads' secure status and their ACL on S3
Previous versions of the mail-receiver used query based api credentials,
if we detect this we will show a message in the admin panel to update
the mail receiver.
* Fix user title logic when badge name customized
* Fix an issue where a user's title was not considered a badge granted title when the user used a badge for their title and the badge name was customized. this affected the effectiveness of revoke_ungranted_titles! which only operates on badge_granted_titles.
* When a user's title is set now it is considered a badge_granted_title if the badge name OR the badge custom name from TranslationOverride is the same as the title
* When a user's badge is revoked we now also revoke their title if the user's title matches the badge name OR the badge custom name from TranslationOverride
* Add a user history log when the title is revoked to remove confusion about why titles are revoked
* Add granted_title_badge_id to user_profile, now when we set badge_granted_title on a user profile when updating a user's title based on a badge, we also remember which badge matched the title
* When badge name (or custom text) changes update titles of users in a background job
* When the name of a badge changes, or in the case of system badges when their custom translation text changes, then we need to update the title of all corresponding users who have a badge_granted_title and matching granted_title_badge_id. In the case of system badges we need to first get the proper badge ID based on the translation key e.g. badges.regular.name
* Add migration to backfill all granted_title_badge_ids for both normal badge name titles and titles using custom badge text.
Instead of enabling `suppress_from_latest` setting on many categories now we can enable `mute_all_categories_by_default` site setting. Then users should opt-in to categories for them to appear in the latest and categories pages.
Previously our index was non unique, causing situations where a user could
have multiple drafts stored in the table for the same exact entity.
This does not properly reflect reality and needed to change as in certain
cases duplicate drafts could be created causing internal data inconsistency
This change adds a message to the admin panel if it detects an api
requests that doesn't use the new header based authentication method.
The message is to warn people to switch to header based auth and links
to the api documention topic on meta for more info.
- Allow revoking keys without deleting them
- Auto-revoke keys after a period of no use (default 6 months)
- Allow multiple keys per user
- Allow attaching a description to each key, for easier auditing
- Log changes to keys in the staff action log
- Move all key management to one place, and improve the UI
Previously theme translations were loaded along with other plugin API scripts. These run after pre-initializers and initializers when the app boots. This commit moves theme translation loading into pre-initializers, so their behaviour matches core translations more closely.
This ensures we can very quickly figure out which topics are banners if
a banner is set.
Previously you would have to scan an entire table to find banners
In a category's settings, the Tags tab has two new fields to
specify the number of tags that must be added to a topic
from a tag group. When creating a new topic, an error will be
shown to the user if the requirement isn't met.
The routes for categories are changing. The scheme that I intend to move
us to is:
/c/*slug_path/(:id)/ENDPOINT
/c/*slug_path/(:id)
This commit adds support for the new scheme to the server side without
dropping support for existing URLs. It is necessary to support existing
URLs for two reasons:
* This commit does not change any client side routing code,
* Posts that contain category hashtags that refer to a root category
are baked into URLs that do not fit this new scheme, (/c/[id]-[slug])
This is a major change to draft internals. Previously there were quite a
few cases where the draft system would say "draft saved", when in fact
we just skipped saving.
This commit ensures the draft system deals with draft ownership handover in
a predictable way.
For example:
- Window 1 editing draft
- Window 2 editing same draft at the same time
Previously we would allow window 1 and 2 to just fight on the same draft
each window overwriting the same draft over an over.
This commit introduces an ownership concept where either window 1 or 2 win
and user is prompted on the loser window to reload screen to correct the issue
This also corrects edge cases where a user could have multiple browser windows
open and posts in 1 window, later to post in the second window. Previously
drafts would break in the second window, this corrects it.
Certain DBs have duplicates already, if we want to ensure uniqueness here
we need to decide first how to clean up existing data and confirm all the
plugins expect this.
This simplifies the code for refreshing notification counts. It now
unconditionally looks up the user object which protects against stale objects
in memory.
This also removes a pile of conditional logic we no longer need.
Doing .pluck(:column).first is a very common pattern in Discourse and in
most cases, a limit cause isn't being added. Instead of adding a limit
clause to all these callsites, this commit adds two new methods to
ActiveRecord::Relation:
pluck_first, equivalent to limit(1).pluck(*columns).first
and pluck_first! which, like other finder methods, raises an exception
when no record is found
- Increase size of the reviewable's conversation excerpt to prevent truncation of the new copy
- Remove the `domain` parameter from the `flag_linked_posts_as_spam` method in the user model since it is no longer needed
- Remove the `domain` interpolation variable from all translation files
- Add "All posts from this user that include links should be reviewed." to server.en.yml for added clarity on why the posts entered the queue
Under exceptional situations the automatic draft feature can fail.
This new **hidden, default off** site setting
`backup_drafts_to_pm_length` will automatically backup any draft that is
saved by the system to a dedicated PM (originating from self)
The body of that PM will contain the text of the reply.
We can enable this feature strategically on sites exhibiting issues to
diagnose issues with the draft system and offer a recourse to users who
appear to lose drafts. We automatically checkpoint these drafts every 5
minutes forcing a new revision each 5 minutes so you can revert to old
content.
Longer term we are considering automatically enabling this kind of feature
for extremely long drafts where the risk is really high one could lose
days of writing.
* FIX: Do not encode the URL twice
Now that we encode slugs in the server we don't need this anymore.
Reverts fe5na33
* FIX: More places do deal with encoded slugs
* the param is a string now, not a hash
* FIX: Handle the nil slug on /categories
* DEV: Add seeded? method to identity default categories
* DEV: Use SiteSetting to keep track of seeded categories
Slugs can be the empty string, but the added index didn't account for
that. This commit changes the migration, stopping it from being unique
so that it can be applied everywhere and adds another migration that
recreates the index properly.
When a category has a subcategory, we ensure that no one who can see the
subcategory cannot see the parent. However, we don't take into account
the fact that, when no CategoryGroups exist, the default is that
everyone has full permissions.
Moving posts also moves the read state (`topic_users` table) to the destination topic. This changes that behavior so that only users who posted in the destination topic will have the original notification level (probably "watching") of the original topic. The notification level for all other users will be set to "regular".
Dropping the temp table in an `ensure` block hides the actual exception. Creating the table with `ON COMMIT DROP` makes the temp table disappear automatically at the end of the transaction. We only need the explicit `DROP` in tests, because tests already run inside a transaction, so the temp table won't be dropped after each test which leads to spec failures.
Post timings are created by `topic_id` and `post_number` and it's possible that the destination topic already contains post timings for non-existent posts. For example, this can happen if the destination topic was previously split and Discourse recorded post timings for moved posts in the destination topic.
This commit ensures that all timings which reference non-existent posts are deleted from the destination topic before the posts are moved.
If the setting is turned on, then the user will receive information
about the subject: if it was deleted or requires some special access to
a group (only if the group is public). Otherwise, the user will receive
a generic #404 error message. For now, this change affects only the
topics and categories controller.
This commit also tries to refactor some of the code related to error
handling. To make error pages more consistent (design-wise), the actual
error page will be rendered server-side.
Added on Aug 21, 2015 in bef80633b1
The only usage removed on Aug 26, 2015 in 4ba89eec27
As far I can tell it isn't used by core or any official plugins.
* FEATURE: Adds an extra protection layer when decompressing files.
* Rename exporter/importer to zip importer. Update old locale
* Added a new composite class to decompress a file with multiple strategies
* Set max file size inside a site setting
* Ensure that file is deleted after compression
* Sanitize path and files before compressing/decompressing
Zeitwerk simplifies working with dependencies in dev and makes it easier reloading class chains.
We no longer need to use Rails "require_dependency" anywhere and instead can just use standard
Ruby patterns to require files.
This is a far reaching change and we expect some followups here.
Adds 2 factor authentication method via second factor security keys over [web authn](https://developer.mozilla.org/en-US/docs/Web/API/Web_Authentication_API).
Allows a user to authenticate a second factor on login, login-via-email, admin-login, and change password routes. Adds registration area within existing user second factor preferences to register multiple security keys. Supports both external (yubikey) and built-in (macOS/android fingerprint readers).
Currently, the topic is only validated for censored words and should be validated for blocked words as well.
Blocked word validation is now used by both Post and Topic. To avoid code duplication, I extracted blocked words validation code into separate Validator, and use it in both places.
The only downside is that even if the topic contains blocked words validation message is saying "Your post contains a word that's not allowed: tomato" but I think this is descriptive enough.
Some site settings (e.g. `unicode_username_character_whitelist`) depend on the default locale, so we need to reset the watched settings when the locale changes.
Forums without previously calculated scores would return the same values
for low/medium/high sensitivity. Now those are scaled based on the
default value.
The default value has also been changed from 10.0 to 12.5 based on
observing data from live discourse forums.
* FEATURE: Add tl2 threshold for editing new posts
* Adds a new setting and for tl2 editing posts (30 days same as old value)
* Sets the tl0/tl1 editing period as 1 day
* FIX: Spec uses wrong setting
* Fix site setting on guardian spec
* FIX: post editing period specs
* Avoid shared examples
* Use update_columns to avoid callbacks on user during tests
If you click a (?) icon beside the reviewable status a pop up will
appear with expanded informatio that explains how the reviewable got its
score, and how it compares to system thresholds.
Start tracking the date an api key was last used. This has already been
the case for user_api_keys.
This information can provide us with the ability to automatically expire
unused api keys after N days.
The URL '/images/transparent.png' will be used in the cooked content if upload record not found. In that case we have to use 'short_url' as image src in 'post.each_upload_url' method.
* FIX: User should get notified when a post is deleted
* FEATURE: Notify posters when restoring flagged posts
* Fix typo
Co-Authored-By: Régis Hanol <regis@hanol.fr>
* Improve tests
* The read indicator now shows up when no member has read the last post of the topic (written by a non-member)
* The read indicator works on mobile and receives live updates from message bus
* The icon we display in the topic list was changed
* Added a title to the indicator to indicate its purpose when hovering over it
The client-side theme-selector would always apply the first in a series of file change notifications. This has been fixed, so it now applies the most recent notification.
Duplicate notifications were being sent because
- The remote_theme autosave was causing every change notification to be doubled
- Color scheme change notifications were being sent every time a theme was uploaded, even if the colors were unchanged
These duplicate notifications have been fixed, and a spec added to ensure it does not regress in future
Databases can have a lot of user actions, self joining and running an
aggregate on millions of rows can be very costly
This optimisation will reduce the regular window of consistency down to 13
hours, this ensures the job runs much faster
When activating a user via an external provider, this would cause the "this account is not activated" message to show on the first attempt, even though the account had been activated correctly.
* Revert "Revert "FEATURE: Publish read state on group messages. (#7989) [Undo revert] (#8024)""
This reverts commit 36425eb9f0.
* Fix: Show who read only if the attribute is enabled
* PERF: Precalculate the last post readed by a group member
* Use book-reader icon instear of far-eye
* FIX: update topic groups correctly
* DEV: Tidy up read indicator update on write
This change allows themes and components access to theme assets.
This means that inside theme js you can now get the URL for an asset with:
```
settings.theme_uploads.name
```
* Reenable: "FEATURE: Publish read state on group messages. (#7989)"
This reverts commit 67f5cc1ce8.
* FIX: Read indicator only appears when the group setting is enabled
* Enable or disable read state based on group attribute
* When read state needs to be published, the minimum unread count is calculated in the topic query. This way, we can know if someone reads the last post
* The option can be enabled/disabled from the UI
* The read indicator will live-updated using message bus
* Show read indicator on every post
* The read indicator now shows read count and can be expanded to see user avatars
* Read count gets updated everytime someone reads a message
* Simplify topic-list read indicator logic
* Unsubscribe from message bus on willDestroyElement, removed unnecesarry values from post-menu, and added a comment to explain where does minimum_unread_count comes from
Previously we relied on side effects to set tracking state correctly
when inviting groups to messages
Also has a minor optimisation in that we use pluck instead of pulling in
full record
Previously we would unconditionally issue an "invited_to_pm" notification
to all non muting users.
New behavior
- Watching and Watching first post get notified
- Tracking get a new "summary" message
- The rest get nothing
This is consistent with topic creation and way clearer
If enabled, this will fire a webhook whenever a user's notification has
been created. This could potentially be a lot of data depending on your
forum, and should be used carefully since it includes everything all users
will see in their feeds.
There are 5 visibility levels (similar to group visibility)
public (default)
logged-in users
members only
staff
owners
Admins & group owners always have visibility to group members.
Sam Saffron
Osama Sayegh
Arpit Jalan
Guo Xiang Tan
Vinoth Kannan
Gerhard Schlager
Michael Brown
Jeff Atwood
Blake Erickson
Roman Rizzi
Martin Brennan
Robin Ward
Régis Hanol
Bianca Nenciu
Dan Ungureanu
David Taylor
Kane York
Mark VanLandingham
Daniel Waterworth
Krzysztof Kotlarek
Jeff Wong
Neil Lalonde
mentalstring
Jarek Radosz
Martin Brennan
Joffrey JAFFEUX
Rafael dos Santos Silva
Justin DiRose
Stasiek Michalski
Penar Musaraj
Stephen Chung
dependabot-preview[bot]
jwjwyoung
Vinoth Kannan
Leo McArdle
Simon Cossar
Mark VanLandingham
Michael Brown
tshenry
Martin Brennan
Rimian Perkins