When changing upload security using `Upload#update_secure_status`,
we may not have the context of how an upload is being created, because
this code path can be run through scheduled jobs. When calling
update_secure_status, the normal ActiveRecord validations are run,
and ours include validating extensions. In some cases the upload
is created in an automated way, such as user export zips, and the
security is applied later, with the extension prohibited from
use when normally uploading.
This caused the upload to fail validation on `update_secure_status`,
causing the security change to silently fail. This fixes the issue
by skipping the file extension validation when the upload security
is being changed.
This commit fixes two issues at play. The first was introduced
in f6c852b (or maybe not introduced
but rather revealed). When a user posted a new message in a topic,
they received the unread topic tracking state MessageBus message,
and the Unread (X) indicator was incremented by one, because with the
aforementioned perf commit we "guess" the correct last read post
for the user, because we no longer calculate individual users' read
status there. This meant that every time a user posted in a topic
they tracked, the unread indicator was incremented. To get around
this, we can just exclude the user who created the post from the
target users of the unread state message.
The second issue was related to the private message topic tracking
state, and was somewhat similar. Whenever a user created a new private
message, the New (X) indicator was incremented, and could not be
cleared until the page was refreshed. To solve this, we just don't
update the topic state for the user when the new_topic tracking state
message comes through if the user who created the topic is the
same as the current user.
cf. https://meta.discourse.org/t/bottom-of-topic-shows-there-is-1-unread-remaining-when-there-are-actually-0-unread-topics-remaining/220817
Discourse has the Discourse Connect Provider protocol that makes it possible to
use a Discourse instance as an identity provider for external sites. As a
natural extension to this protocol, this PR adds a new feature that makes it
possible to use Discourse as a 2FA provider as well as an identity provider.
The rationale for this change is that it's very difficult to implement 2FA
support in a website and if you have multiple websites that need to have 2FA,
it's unrealistic to build and maintain a separate 2FA implementation for each
one. But with this change, you can piggyback on Discourse to take care of all
the 2FA details for you for as many sites as you wish.
To use Discourse as a 2FA provider, you'll need to follow this guide:
https://meta.discourse.org/t/-/32974. It walks you through what you need to
implement on your end/site and how to configure your Discourse instance. Once
you're done, there is only one additional thing you need to do which is to
include `require_2fa=true` in the payload that you send to Discourse.
When Discourse sees `require_2fa=true`, it'll prompt the user to confirm their
2FA using whatever methods they've enabled (TOTP or security keys), and once
they confirm they'll be redirected back to the return URL you've configured and
the payload will contain `confirmed_2fa=true`. If the user has no 2FA methods
enabled however, the payload will not contain `confirmed_2fa`, but it will
contain `no_2fa_methods=true`.
You'll need to be careful to re-run all the security checks and ensure the user
can still access the resource on your site after they return from Discourse.
This is very important because there's nothing that guarantees the user that
will come back from Discourse after they confirm 2FA is the same user that
you've redirected to Discourse.
Internal ticket: t62183.
Due to default CSP web workers instantiated from CDN based assets are still
treated as "same-origin" meaning that we had no way of safely instansiating
a web worker from a theme.
This limits the theme system and adds the arbitrary restriction that WASM
based components can not be safely used.
To resolve this limitation all js assets in about.json are also cached on
local domain.
{
"name": "Header Icons",
"assets" : {
"worker" : "assets/worker.js"
}
}
This can then be referenced in JS via:
settings.theme_uploads_local.worker
local_js_assets are unconditionally served from the site directly and
bypass the entire CDN, using the pre-existing JavascriptCache
Previous to this change this code was completely dormant on sites which
used s3 based uploads, this reuses the very well tested and cached asset
system on s3 based sites.
Note, when creating local_js_assets it is highly recommended to keep the
assets lean and keep all the heavy working in CDN based assets. For example
wasm files can still live on the CDN but the lean worker that loads it can
live on local.
This change unlocks wasm in theme components, so wasm is now also allowed
in `theme_authorized_extensions`
* more usages of upload.content
* add a specific test for upload.content
* Adjust logic to ensure that after upgrades we still get a cached local js
on save
There are still some, but those are in actual code that's used outside core, so the change there would need to go through the deprecation cycle. That's a task for another day.
Previously we only supported a single 'required tag group' for a category. This commit allows admins to specify multiple required tag groups, each with their own minimum tag count.
A new category_required_tag_groups database table replaces the existing columns on the categories table. Data is automatically migrated.
Previous to this change an optimisation stripped crawler content from
all mobile browsers.
This had a side effect that meant that when we dropped support for an old
mobile platform we would stop rendering topic and topic list pages.
The new implementation ensures we only perform the optimisation on modern
mobile browsers.
This patch removes some of our freedom patches that have been deprecated
for some time now.
Some of them have been updated so we’re not shipping code based on an
old version of Rails.
This commit introduces a new use_polymorphic_bookmarks site setting
that is default false and hidden, that will be used to help continuous
development of polymorphic bookmarks. This setting **should not** be
enabled anywhere in production yet, it is purely for local development.
This commit uses the setting to enable create/update/delete actions
for polymorphic bookmarks on the server and client side. The bookmark
interactions on topics/posts are all usable. Listing, searching,
sending bookmark reminders, and other edge cases will be handled
in subsequent PRs.
Comprehensive UI tests will be added in the final PR -- we already
have them for regular bookmarks, so it will just be a matter of
changing them to be for polymorphic bookmarks.
Under some conditions, replacing an `<img` with `![]()` can break rendering, and make the image disappear.
Context at https://meta.discourse.org/t/152801
* DEV: add testing for multi del on keys
Following #15905 we were missing some tests, this covers cases where
del is used in the form of .del(key1,key2)
Tags (and tag groups) can be configured so that they can only be used in specific categories and (optionally) restrict topics in these categories to be able to add/use only these tags. These restrictions work as expected when a topic is created without going through the review queue; however, if the topic has to be reviewed by a moderator then these restrictions currently aren't checked before the topic is sent to the review queue, but they're checked later when a moderator tries to approve the topic. This is because if a user manages to submit a topic that doesn't meet the restrictions, moderators won't be able to approve and it'll be stuck in the review queue.
This PR prevents topics that don't meet the tags requirements from being sent to the review queue and shows the poster an error message that indicates which tags that cannot be used.
Internal ticket: t60562.
Previously, our `upload://` protocol urls were only supported in markdown image tags. This meant that our PullHotlinkedImages job was forced to convert `<img` tags to markdown. Depending on the exact syntax, this can actually cause the image to break.
This commit adds support for `upload://` inside regular HTML `<img` tags. In a future commit, we'll be able to use this to make our PullHotlinkedImages job much more robust.
Context at https://meta.discourse.org/t/152801
PostAnalyzer and CookedPostProcessor both replace URLs with oneboxes.
PostAnalyzer did not use the max_oneboxes_per_post site and setting and
CookedPostProcessor replaced at most max_oneboxes_per_post URLs ignoring
the oneboxes that were replaced already by PostAnalyzer.
If the crawled page returned an error, `FinalDestination#safe_get`
yielded `nil` for `uri` and `chunk` arguments. Another problem is that
`get` did not handle the case when `safe_get` failed and did not return
the `location` and `set_cookie` headers.
If a group's messageable_level is set to nobody then staff can't should not be able to send PMs to it.
Co-authored-by: Martin Brennan <martin@discourse.org>
The `blocked onebox domains` setting lets site owners change what sites
are allowed to be oneboxed. When a link is entered into a post,
Discourse checks the domain of the link against that setting and blocks
the onebox if the domain is blocked. But if there's a chain of
redirects, then only the final destination website is checked against
the site setting.
This commit amends that behavior so that every website in the redirect
chain is checked against the site setting, and if anything is blocked
the original link doesn't onebox at all in the post. The
`Discourse-No-Onebox` header is also checked in every response and the
onebox is blocked if the header is set to "1".
Additionally, Discourse will now include the `Discourse-No-Onebox`
header with every response if the site requires login to access content.
This is done to signal to a Discourse instance that it shouldn't attempt
to onebox other Discourse instances if they're login-only. Non-Discourse
websites can also use include that header if they don't wish to have
Discourse onebox their content.
Internal ticket: t59305.
The user can select what happens with a bookamrk after it expires. New
option allow bookmark's reminder to be kept even after it has expired.
After a bookmark's reminder notification is created, the reminder date
will be highlighted in red until the user resets the reminder date.
User can do that using the new Clear Reminder button from the dropdown.
The search_ignore_accents site setting can be used to make the search
indexer remove the accents before indexing the content. The unaccent
function from PostgreSQL is better than Ruby's unicode_normalize(:nfkd).
Some product pages on Amazon are using a new HTML structure, meaning the previous Onebox engine was unable to gather the price and/or description. This change should allow these pages to be Oneboxed.
This PR adds an extra description to the 2FA page when granting a user admin access. It also introduces a general system for adding customized descriptions that can be used by future actions.
(Follow-up to dd6ec65061)
Discourse users and associated accounts are created or updated when a
user logins or connects the account using their account preferences.
This new API can be used to create associated accounts and users too,
if necessary.
It's very easy to forget to add `require 'rails_helper'` at the top of every core/plugin spec file, and omissions can cause some very confusing/sporadic errors.
By setting this flag in `.rspec`, we can remove the need for `require 'rails_helper'` entirely.
This allows text editors to use correct syntax coloring for the heredoc sections.
Heredoc tag names we use:
languages: SQL, JS, RUBY, LUA, HTML, CSS, SCSS, SH, HBS, XML, YAML/YML, MF, ICS
other: MD, TEXT/TXT, RAW, EMAIL
Previously email validations could fire when deleting posts if for
certain reasons any user validations fail on the user objects
This kind of condition could happen in core due to a corruption of a
user record, or via a plugin that introduces a new validation on User
Lib specs moved in 45cc16098d
Move the new selectable_avatars_mode_validator_spec to the new location
Remove the old selectable_avatars_enabled_validator_spec
follow-up of d1bdb6c65d
This commit handles the edge case where a draft is lost with no warnings if the user edits the title (or category/tags) of a topic while they're replying.to the same topic. Repro steps are as follows:
1. Start replying to a topic and type enough to get a draft saved.
2. Scroll up to the topic title and click the pencil icon next to the topic title, change the title, category and/or tags, and then save the changes.
3. Reload the page and you'll see that the draft is gone.
This happens because we only allow 1 draft per topic per user and when you edit the title of a topic that you're replying to, from the server perspective it'll look like as if you've submitted your reply so it will advance the draft sequence for the topic and delete the draft.
The fix in this commit makes `PostRevisor` skip advancing the draft sequence when a topic's title is edited using the pencil button next to the title.
Internal ticket: t60854.
Co-authored-by: Robin Ward <robin.ward@gmail.com>
This option will make it so the [quote] bbcode will always
include the HTML link to the quoted post, even if a topic_id
is not provided in the PrettyText#cook options. This is so
[quote] bbcode can be used in other places, like chat messages,
that always need the link and do not have an "off-topic" ID
to use.
Previously cached counting made redis calls in main thread and performed
the flush in main thread.
This could lead to pathological states in extreme heavy load.
This refactor reduces load and cleans up the interface
Previously we were publishing one messagebus message per user which was 'tracking' a topic. On large sites, this can easily be 1000+ messages. The important information in the message is common between all users, so we can manage with a single message on a shared channel, which will be much more efficient.
For user-specific values (notification_level and last_read_post_number), the JS app can infer values which are 'good enough'. Correct values will be loaded as soon as a topic-list containing the topic is visited.
Themes often cache `nil` values in a DistributedCache. This bug meant that we were re-calculating some values on every request, AND triggering message-bus publishing on every request.
This fix should provide a significant performance improvement for busy sites.
2FA support in Discourse was added and grown gradually over the years: we first
added support for TOTP for logins, then we implemented backup codes, and last
but not least, security keys. 2FA usage was initially limited to logging in,
but it has been expanded and we now require 2FA for risky actions such as
adding a new admin to the site.
As a result of this gradual growth of the 2FA system, technical debt has
accumulated to the point where it has become difficult to require 2FA for more
actions. We now have 5 different 2FA UI implementations and each one has to
support all 3 2FA methods (TOTP, backup codes, and security keys) which makes
it difficult to maintain a consistent UX for these different implementations.
Moreover, there is a lot of repeated logic in the server-side code behind these
5 UI implementations which hinders maintainability even more.
This commit is the first step towards repaying the technical debt: it builds a
system that centralizes as much as possible of the 2FA server-side logic and
UI. The 2 main components of this system are:
1. A dedicated page for 2FA with support for all 3 methods.
2. A reusable server-side class that centralizes the 2FA logic (the
`SecondFactor::AuthManager` class).
From a top-level view, the 2FA flow in this new system looks like this:
1. User initiates an action that requires 2FA;
2. Server is aware that 2FA is required for this action, so it redirects the
user to the 2FA page if the user has a 2FA method, otherwise the action is
performed.
3. User submits the 2FA form on the page;
4. Server validates the 2FA and if it's successful, the action is performed and
the user is redirected to the previous page.
A more technically-detailed explanation/documentation of the new system is
available as a comment at the top of the `lib/second_factor/auth_manager.rb`
file. Please note that the details are not set in stone and will likely change
in the future, so please don't use the system in your plugins yet.
Since this is a new system that needs to be tested, we've decided to migrate
only the 2FA for adding a new admin to the new system at this time (in this
commit). Our plan is to gradually migrate the remaining 2FA implementations to
the new system.
For screenshots of the 2FA page, see PR #15377 on GitHub.
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
Follow up to 6f7364e48b to add a spec
that tests the full authentication of a Windows Hello algorithm (-257)
webauthn verification. The test added in that commit only tested that
we know about that algorithm, not whether it was actually usable.
