This commit introduces two new APIs for handling unused uploads, one
can be used to exclude uploads in bulk when the data model allow and
the other one excludes uploads one by one.
We added this constraint in 5bd55acf83
but it is causing problems in hosted sites and is catching the
issue too far down the line. This commit removes the constraint
for now, and also fixes an issue found with PostDestroyer
which wasn't using the UserStatCountUpdater when updating post_count
and thus was causing negative numbers to occur.
Whenever we got a bounced email in the Email::Receiver we
previously would just set bounced: true on the EmailLog and
discard the status/diagnostic code. This commit changes this
flow to store the bounce error code (defined in the RFC at
https://www.iana.org/assignments/smtp-enhanced-status-codes/smtp-enhanced-status-codes.xhtml)
not just in the Email::Receiver, but also via webhook events
from other mail services and from SNS.
This commit does not surface the bounce error in the UI,
we can do that later if necessary.
To make this possible in development mode, the `sourceURL=` implementation needs to include something plugin-specific. This has no effect on production.
The asset version is bumped in order to trigger a re-compilation of plugin JS assets.
Without this parameter, requests for sourcemaps on shared-CDN multisites will not be routed to the correct database, resulting in a 404.
The stylesheet content now depends on the site hostname, so the hostname has been added to the digest.
The `assets:precompile` rake task loads the full Ruby app, which can consume around 500mb of RAM by itself. Using `exec` to run `ember build` allows us to free up the Ruby memory and make more space for `ember build`
This makes a small improvement to 'cold cache' ember-cli build times, and a large improvement to 'warm cache' build times
The ember-auto-import update means that vendor is now split into multiple files for efficiency. These are named `chunk.*`, and should be included immediately after the `vendor.js` file. This commit also updates the rails app to render script tags for these chunks.
This change was previously merged, and caused memory-related errors on RAM-constrained machines. This was because Webpack 5 switches from multiple worker processes to a single multi-threaded process. This meant that it was hitting node's default heap size limit (~500mb on a 1GB RAM server). Discourse's standard install procedure recommends adding 2GB swap to 1GB-RAM machines, so we can afford to override's Node's default via the `--max-old-space-size` flag.
The chat quoting mechanism will need to be able to generate
markdown for all kinds of uploads. The UploadMarkdown class
was missing generation for video and audio uploads. This
commit adds that in, and also expands the server-side regex
recognition of FileHelper types to match those in uploads.js,
and adds a spec for UploadMarkdown
When a site has secure media enabled and a post is with secure
media, we were incorrectly cooking custom emoji URLs and using the
secure URL for those emojis, even though they should not be
considered secure (their corresponding upload records in the
database are _not_ secure). Now instead of the blanket
post.with_secure_media? boolean for the secure: param, we also
want to make sure the image whose URL is being cooked is also
_not_ a custom emoji.
This will allow pretty text deprecations / errors / warnings to appear in the Rails logs, rather than disappearing silently.
(implementation adapted from `discourse_js_processor.rb`)
This is intended for use by plugins which are building their own
topic lists, and want to include PMs alongside regular topics (e.g.
discourse-assign). It does not get used directly in core.
Redis supports deleting multiple keys at once using the `del` command and so does the `redis` gem: 21ec1dec06/lib/redis/commands/keys.rb (L188-L193). However, our wrapper around the `del` method currently accepts only one argument and expects it to be a string so it's impossible to delete multiple keys at once.
This PR changes the signature of the `DiscourseRedis#del` method so it accepts any number of arguments and makes sure all keys are properly namespaced before calling the `del` implementation of the `redis` gem.
Breakdown of fixes in this commit:
* `UserStat#topic_count` was not updated when visibility of
the topic changed.
* `UserStat#post_count` was not updated when post was hidden or
unhidden.
* `TopicConverter` was only incrementing or decrementing the counts by 1
even if a user has multiple posts in the topic.
* The commit turns off the verbose logging by default as it is just
noise to normal users who are not debugging this problem.
Ember CLI gives sourcemaps their own digest. Our `s3.rake` logic assumes that the digest portion of sourcemap filenames remains the same.
The Ember CLI sourcemaps are included in the manifest file, so we can ensure they are uploaded by letting them past the MiniMime check.
Followup to abefb1beff
If no path is supplied, browsers will look for the map on the same path as the JS file itself. This fixes two problems that we see in production:
1. When compiling assets against one CDN, and then re-using them on a site with a different CDN, the sourceMappingUrls would be incorrect and print warnings in the console
2. If both an S3 CDN and an app CDN are configured, we were using the S3 CDN for the JS and the app CDN for the map. This commit will make sure we use the S3 CDN for both.
ember-cli already runs terser on its output. Running it through again provides no benefit, takes longer, and also breaks source mapping of these assets in production.
This commits adds a new advance_draft to PostCreator that controls if
the draft sequence will be advanced or not. If the draft sequence is
advanced then the old drafts will be cleared. This used to happen for
posts created by plugins or through the API and cleared user drafts
by mistake.
* FEATURE: Add external_id to topics
This commit allows for topics to be created and fetched by an
external_id. These changes are API only for now as there aren't any
front changes.
* add annotations
* add external_id to this spec
* Several PR feedback changes
- Add guardian to find topic
- 403 is returned for not found as well now
- add `include_external_id?`
- external_id is now case insensitive
- added test for posts_controller
- added test for topic creator
- created constant for max length
- check that it redirects to the correct path
- restrain external id in routes file
* remove puts
* fix tests
* only check for external_id in webhook if exists
* Update index to exclude external_id if null
* annotate
* Update app/controllers/topics_controller.rb
We need to check whether the topic is present first before passing it to the guardian.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
* Apply suggestions from code review
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
* FEATURE: RS512, RS384 and RS256 COSE algorithms
These algorithms are not implemented by cose-ruby, but used in the web
authentication API and were marked as supported.
* FEATURE: Use all algorithms supported by cose-ruby
Previously only a subset of the algorithms were allowed.
This can be disabled by setting `EMBER_CLI_PROD_ASSETS=0`, but this option will not be available for long. If your theme/plugin/site has issues under Ember CLI, please open a topic on https://meta.discourse.org
This allows plugins to override the permissions required to access
specific things like the Logster and Sidekiq web UI without the changes
leaking to the rest of Discourse routes.
Our discourse_public_exceptions middleware is designed to catch bubbled exceptions from lower in the stack, and then use `ApplicationController.rescue_with_handler` to render an appropriate error response.
When the request itself is invalid, we had an escape-hatch to skip re-dispatching the request to ApplicationController. However, it was possible to work around this by 'layering' the errors. For example, if you made a request which resulted in a 404, but **also** had some other invalidity, the escape hatch would not be triggered.
This commit ensures that these kind of 'layered' errors are properly handled, without logging warnings. It also adds detection for invalid JSON bodies and badly-formed multipart requests.
The user-facing behavior is unchanged. This commit simply prevents warnings being logged for invalid requests.
Ensures that `UserStat#post_count` and `UserStat#topic_count` does not
go below 0. When it does like it did now, we tend to have bugs in our
code since we're usually coding with the assumption that the count isn't
negative.
In order to support the constraints, our post and topic fabricators in
tests will now automatically increment the count for the respective
user's `UserStat` as well. We have to do this because our fabricators
bypasss `PostCreator` which holds the responsibility of updating `UserStat#post_count` and
`UserStat#topic_count`.
* Chinese segmenetation will continue to rely on cppjieba
* Japanese segmentation will use our port of TinySegmenter
* Korean currently does not rely on segmentation which was dropped in c677877e4f
* SiteSetting.search_tokenize_chinese_japanese_korean has been split
into SiteSetting.search_tokenize_chinese and
SiteSetting.search_tokenize_japanese respectively
Sometimes, the 'message' portion of an exception isn't enough to work out what's happening. In these cases, including the exception class name can help with debugging.
Job arguments go via JSON, and so symbols will appear as strings in the Job's `#execute` method. The latest version of Sidekiq has started warning about this to reduce developer confusion.
Most of our logging goes through Rails.logger, and therefore appears in Logster at `/logs` on a site. The Sidekiq logger was bypassing this and writing directly to STDERR.
Unfortunately it's not possible to do `Sidekiq.logger = Rails.logger` because `Sidekiq#logger=` applies a number of patches to the logger instance, causing our whole logging system to break.
Instead, this commit adds a dedicated Logger instance with no output, which is then patched to forward all messages directly to `Rails.logger`
- Limit bulk re-invite to 1 time per day
- Move bulk invite by csv behind a site setting (hidden by default)
- Bump invite expiry from 30 -> 90 days
## Updates to rate_limiter
When limiting reinvites I found that **staff** are never limited in any way. So I updated the **rate_limiter** model to allow for a few things:
- add an optional param of `staff_limit`, which (when included and passed values, and the user passes `.staff?`) will override the default `max` & `secs` values and apply them to the user.
- in the case you **do** pass values to `staff_limit` but the user **does not** pass `staff?` the standard `max` & `secs` values will be applied to the user.
This should give us enough flexibility to
1. continue to apply a strict rate limit to a standard user
2. but also apply a secondary (less strict) limit to staff
This adapter ensures that MiniSql locks the ActiveRecord mutex before using the raw PG connection. This ensures that multiple threads will not attempt to use the same connection simultaneously.
This commit also removes the schema_cache_concurrency freedom-patch, which is no longer required now that cross-thread connection use is controlled by the mutex.
For the original root cause of this issue, see https://github.com/rails/rails/pull/38577
When creating a direct message to a group with group SMTP
set up, and adding another person to that message in the OP,
we send an email to the second person in the OP via the group_smtp
job. This in turn creates an IncomingEmail record to guard against
IMAP double sync.
The issue with this was that this IncomingEmail (which is essentialy
a placeholder/dummy one) was having its Message-ID used as the canonical
References Message-ID for subsequent emails sent out to user_private_message
recipients (such as members of the group), causing threading issues in
the mail client. The canonical <topic/ID@HOST> format should be used
instead for these cases.
This commit fixes the issue by only using the IncomingEmail for the
OP's Message-ID if the OP was created via our handle_mail email receiver
pipeline. It does not make sense to use it in other cases.
In an earlier PR, we decided that we only want to block a domain if
the blocked domain in the SiteSetting is the final destination (/t/59305). That
PR used `FinalDestination#get`. `resolve` however is used several places
but blocks domains along the redirect chain when certain options are provided.
This commit changes the default options for `resolve` to not do that. Existing
users of `FinalDestination#resolve` are
- `Oneboxer#external_onebox`
- our onebox helper `fetch_html_doc`, which is used in amazon, standard embed
and youtube
- these folks already go through `Oneboxer#external_onebox` which already
blocks correctly
Sometimes plugins need to have additional data or options available
when rendering custom markdown features/rules that are not available
on the default opts.discourse object. These additional options should
be namespaced to the plugin adding them.
```
Site.markdown_additional_options["chat"] = { limited_pretty_text_markdown_rules: [] }
```
These are passed down to markdown rules on opts.discourse.additionalOptions.
The main motivation for adding this is the chat plugin, which currently stores
chat_pretty_text_features and chat_pretty_text_markdown_rules on
the Site object via additions to the serializer, and the Site object is
not accessible to import via markdown rules (either through
Site.current() or through container.lookup). So, to have this working
for both front + backend code, we need to attach these additional options
from the Site object onto the markdown options object.
Running `update_from_remote` and `save!` cause a number of side-effects, including instructing all clients to reload CSS files. If there are no changes, then this is wasteful, and can even cause a 'flicker' effect on clients as they reload CSS.
This commit checks if any updates are available before triggering `update_from_remote` / `save!`. This should be much faster, and stop the 'flickering' UX from happening on every themes:update run.
It also improves the output of the command to include the from/to commit hashes, which may be useful for debugging issues. For example:
```
Checking 'Alien Night | A Dark Discourse Theme' for 'default'... already up to date
Checking 'Star Wars' for 'default'... updating from d8a170dd to 66b9756f
Checking 'Media Overlay' for 'default'... already up to date
```
`account_created` email contains a URL to `/u/password-reset/TOKEN`
which means that the correct scope for the email token is
`password_reset`, not `signup`.
Our previous implementation used a simple `blocked_domain_array.include?(hostname)`
so some values were not matching. Additionally, in some configurations like ours, we'd used
"cat.*.dog.com" with the assumption we'd support globbing.
This change implicitly allows globbing by blocking "http://a.b.com" if "b.com" is a blocked
domain but does not actively do anything for "*".
An upcoming change might include frontend validation for values that can be inserted.
* FIX: Mark invites flash messages as HTML safe.
This change should be safe as all user inputs included in the errors are sanitized before sending it back to the client.
Context: https://meta.discourse.org/t/html-tags-are-explicit-after-latest-update/214220
* If somebody adds a new error message that includes user input and doesn't sanitize it, using html-safe suddenly becomes unsafe again. As an extra layer of protection, we make the client sanitize the error message received from the backend.
* Escape user input instead of sanitizing
For now this is still gated behind a `QUNIT_EMBER_CLI=1` environment variable, but will eventually become the default so that we can remove `run-qunit.js`.
If the SiteSetting `allowed_onebox_iframes` contains a value of `*`, it will use the values of `all_iframe_origins` during the Oneboxing process. If `all_iframe_origins` itself contains a value of `*`, `origins_to_regexes` will try to return a "catch-all" regex.
Other code assumes `origins_to_regexes`will return an array, so this change ensures the `*` case will return an array containing only the catch-all regex.
1. `html_doc.css('.Box.md')` always returns a truthy value (e.g. `[]`) so the second branch of the if-elsif never ran
2. `node&.css('text()')` was invalid code that would raise an error
3. Matching on h3 elements is no longer correct with the current html structure returned by GitHub
This reverts commit 2c7906999a.
The changes break some things in local development (putting JS files
into minified files, not allowing debugger, and others)
This reverts commit ea84a82f77.
This is causing problems with `/theme-qunit` on legacy, non-ember-cli production sites. Reverting while we work on a fix
This is quite complex as it means that in production we have to build
Ember CLI test files and allow them to be used by our Rails application.
There is a fair bit of glue we can remove in the future once we move to
Ember CLI completely.
The `plugin:pull_compatible_all` task is intended to take incompatible plugins and downgrade them to an earlier version. Problem is, when running the rake task in development/production environments, the plugins have already been activated. If an incompatible plugin raises an error in `plugin.rb` then the rake task will be unable to start.
This commit centralises our LOAD_PLUGINS detection, adds support for LOAD_PLUGINS=0 in dev/prod, and adds a warning to `plugin:pull_compatible_all` if it's run with plugins enabled.
This commit extends the options which can be passed to
`PrettyText.markdown` so that which Markdown-it rules and Discourse
Markdown plugins to be used when rendering a text can be customizable.
Currently, this extension is mainly used by plugins.
Having to load `ip_addr` is confusing especially when that file exists
to monkey patch Ruby's `IpAddr` class. Moving it to our freedom patches
folder which is automatically loaded on initialization.
Also:
* Remove an unused method (#fill_email)
* Replace a method that was used just once (#generate_username) with `SecureRandom.alphanumeric`
* Remove an obsolete dev puma `tmp/restart` file logic
* File.exists? is deprecated and removed in Ruby 3.2 in favor of
File.exist?
* Dir.exists? is deprecated and removed in Ruby 3.2 in favor of
Dir.exist?
The rake task deleted here was added back in Feb 2020
when bookmarks were first converted from PostAction
records, it is no longer needed. The ignored columns
were removed in ed83d7573e.
This commit adds a check that runs regularly as per
2d68e5d942 which tests the
credentials of groups with SMTP or IMAP enabled. If any issues
are found with those credentials a high priority problem is added to the
admin dashboard.
This commit also formats the admin dashboard differently if
there are high priority problems, bringing them to the top of
the list and highlighting them.
The problem will be cleared if the issue is fixed before the next
problem check, or if the group's settings are updated with a valid
credential.
Bianca Nenciu
Martin Brennan
David Taylor
Krzysztof Kotlarek
Osama Sayegh
Alan Guo Xiang Tan
Jarek Radosz
Dan Ungureanu
Blake Erickson
jbrw
Andrei Prigorshnev
janzenisaac
Rafael dos Santos Silva
Noah Gibbs
Natalie Tay
Martin Brennan
Neil Lalonde
Gerhard Schlager
Roman Rizzi
jbrw
Joffrey JAFFEUX
Robin Ward
Vinoth Kannan
Peter Zhu
Bianca Nenciu
Penar Musaraj