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.
* FIX: Don't accept accents in slug if generation_method == 'ascii'
Fixes bug reported in:
- https://meta.discourse.org/t/404-when-trying-to-edit-category-with-accent-in-slug/214762
- https://meta.discourse.org/t/formatting-and-accents-in-urls/215734/5
Assuming `SiteSetting.slug_generation_method == 'ascii'.
If the user provides a slug containing non-ascii characters while
creating the category, the user will receive a 404 error just
after saving the category since the slug will be escaped anyway but
Category.find_by_slug_path won't escape the category slug
causing the Edit Page of the category to be inaccessible.
This commit checks the provided slug and raises an error if the
provided slugcontains non-ascii characters ensuring that the
provided value is consistent with the site settings.
It also changes Category.find_by_slug_path to always escape the slug,
since if present, it is escaped anyway in Category.ensure_slug to
prevent the 404 in the Edit Category Page in case the user already
have some category with a non-ascii slug.
* Removed trailing whitespace
When parent category or grandparent category is muted, then category should be muted as well.
Still, it can be overridden by setting individual subcategory notification level.
CategoryUser record is not created, mute for subcategories is purely virtual.
This can happen if the topic to which a user is invited is in a private
category and the user was not invited to one of the groups that can see
that specific category.
This used to be a warning and this commit makes it an error.
This was deprecated in Discourse 2.4, but no end version was put on the deprecation. Many plugins/themes are still using it. This commit restores it under ember-cli so that it does not block the Ember CLI rollout, and can be removed in a future commit.
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.
Previously calls such as `Emoji["smile"]` would force a full dehydration of
objects from Redis.
This introduces a version safe site and global emoji cache so lookups are
cheap. It eliminates iterating through the list of emojis and pulling from
redis.
Distributed cache uses a normalized name as the key and stores an Array tuple
with version and Emoji. Successful hits always confirm version matches.
Interface to Emoji object remains unchanged.
We opted for 2 caches to improve reuse on multisites. misses though will be
stored in both caches. If there is a hit on the global cache we can avoid
looking up in site local cache and storing a miss there.
Previously we were calling `EXPIRE` every time we incremented a given key. Instead, we can call EXPIRE once when the key is first populated. A LUA script is used to make this as efficient as possible.
Consumers of this Concern use daily keys. Since we're now calling EXPIRE only at the beginning of the day, rather than throughout the day, the expire time has been increased from 3 to 4 days.
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.
There is a couple of layers of caching for theme JavaScript in Discourse:
The first layer is the `javascript_caches` table in the database. When a theme
with JavaScript files is installed, Discourse stores each one of the JavaScript
files in the `theme_fields` table, and then concatenates the files, compiles
them, computes a SHA1 digest of the compiled JavaScript and store the results
along with the SHA1 digest in the `javascript_caches` table.
Now when a request comes in, we need to render `<script>` tags for the
activated theme(s) of the site. To do this, we retrieve the `javascript_caches`
records of the activated themes and generate a `<script>` tag for each record.
The `src` attribute of these tags is a path to the `/theme-javascripts/:digest`
route which simply responds with the compiled JavaScript that has the requested
digest.
The second layer is a distributed cache whose purpose is to make rendering
`<script>` a lot more efficient. Without this cache, we'd have to query the
`javascript_caches` table to retrieve the SHA1 digests for every single
request. So we use this cache to store the `<script>` tags themselves so that
we only have to retrieve the `javascript_caches` records of the activated
themes for the first request and future requests simply get the cached
`<script>` tags.
What this commit does it ensures that the SHA1 digest in the
`javascript_caches` table stay the same across compilations by adding an order
by id clause to the query that loads the `theme_fields` records. Currently, we
specify no order when retrieving the `theme_fields` records so the order in
which they're retrieved can change across compilations and therefore cause the
SHA1 to change even though the individual records have not changed at all.
An inconsistent SHA1 digest across compilations can cause the database cache
and the distributed cache to have different digests and that causes the
JavaScript to fail to load (and if the theme heavily customizes the site, it
gives the impression that the site is broken) until the cache is cleared.
This can happen in busy sites when 2 concurrent requests recompile the
JavaScript files of a theme at the same time (this can happen when deploying a
new Discourse version) and request A updates the database cache after request B
did, and request B updates the distributed cache after request A did.
Internal ticket: t60783.
Co-authored-by: David Taylor <david@taylorhq.com>
We serve `service-worker.js` in an unusual way, which means that the sourcemap is not available on an adjacent path. This means that the browser fails to fetch the map, and shows an error in the console.
This commit re-writes the source map reference in the static_controller to be an absolute link to the asset (including the appropriate CDN, if enabled), and adds a spec for the behavior.
It's important to do this at runtime, rather than JS precompile time, so that changes to CDN configuration do not require re-compilation to take effect.
The old choose-topic component did not have the same style as the rest
of the create invite modal and was not very suitable to use in the modal
because it introduced the search results in modal's body.
The new topic-chooser is built using select-kit and provides a more
polished user experience.
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.
- Update UI to improve contrast
- Make it clear that the message is only shown to administrators
- Add theme name and id to the console output
- Parse the error backtrace to identify the theme-id for post-decoration errors
- Improve console output to include the theme name / URL
- Add `?safe_mode=no_custom` to the admin panel link, so that it will work even if the theme is causing the site to break
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
In 8e5b945b0f, we reverted the commit but
at the same time resulted in Theme::BASE_COMPILER_VERSION going
backwards which caused problems with themes caching.
This commit bumps the version to clear all the caches.
Follow-up to 8e5b945b0f
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.
Another use case for focusComposer() is if the user is
already inside a topic but another component (such as the
floating chat window) needs to open the composer. This
commit also fixes the appendText option to only prepend
2 new lines if there is text before the text to be appended.
Follow up 7850ee318f
- Update UI to improve contrast
- Make it clear that the message is only shown to administrators
- Add theme name and id to the console output
- Parse the error backtrace to identify the theme-id for post-decoration errors
- Improve console output to include the theme name / URL
- Add `?safe_mode=no_custom` to the admin panel link, so that it will work even if the theme is causing the site to break
accept HTML attribute is not fully supported on iOS yet and can contain
only MIME types. This changes the input to allow all files and the
extension check is performed later in JavaScript.
Instead of relaying on /timings request, we should cache last read post number. That should protect from having incorrect unread counter when going back to topic list.
This additional cache is very temporary as once /timings request is finished, serializer will have a correct result.
Simplified flow is:
1. Store in cache information about last seen post number before /timings request is sent
2. When getting back to topic list compare value of last seen post number returned by /latest request and information in cache. If cache number is higher, than use it instead of information returned by /latest. In addition delete cache item as there is high chance that `/timings` request already finished.
3. Optionally, delete cache when timings request is done and topic list was not yet visited.
Keeping cache reasonably small should not affect performance.
TopicTrackingState should correctly set filterCategory and filterTag for all different configurations.
When filterTag exists and new_topic message arrives, it ensures that filterTag is included in payload tags
If filterTag is part of payload tags, message that new topics are available is displayed and after click, new topics are included in the list.
This adds logic to increase an `InvitedUser` record, increase
`redemption_count` and create a `:invitee_accepted` to let the inviter
know that the invitee used the invite.
Initial support for this was implemented in commit 9969631.
In ab5361d69a, we rescue from the PG error
but the transaction is already aborted causing any DB query after to
fail. As such, we avoid triggering the error in the first place by
checking that we would not be insertin a negative number into the
counter cache.
Follow-up to ab5361d69a
Allows to write custom code blocks:
```
```mermaid height=200,foo=bar
test
```
```
Which will then get converted to:
```
<pre data-code-wrap="mermaid" data-code-height="200" data-code-foo="bar">
<code class="lang-nohighlight">
test
</code>
</pre>
```
Sorting group members worked always kept the group owners at the top of
the list. This commit keeps the group owners at the top of the list only
when no order exists.
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.
We need this in other places, this commit moves clipboardCopy
to the utilities.js lib. Had to remove use of Promise as well because
lib/utilities cannot import it, otherwise it will cause a mini racer error.
This commit adds a new helpful function to the composer controller
which can be used to focus the composer and insert text, regardless
of whether the consumer knows whether the composer is open or has
a draft. This is good for cases where an action needs to copy text
to the composer or open it with text after navigating to a URL.
The inspiration for this addition is the discourse-chat plugin,
which needs to be able to copy quoted markdown from the chat
and insert it into the composer, and unlike in the topic controller
we have no idea of the state of the composer from chat.
There are still spots in the code base which results in us trying to turn the post and topic count negative. However,
we have a job that runs on a daily basis which will correct the count. Therefore, avoid raising an error for now
and log the exception instead.
* 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>
Under ember-cli, we rely on the `ember-export-application-global` addon to make `window.Discourse` available. This happens in an initializer. Previously this inititalizer would run after `auto-load-modules`, and so any widget/helper modules would not be able to access it. This commit sets some `after` parameters on the `auto-load-modules` and `inject-objects` initializers to ensure that `export-application-global` is run first.
This reverts commit f4c6a61855 and a8325c9016
This update of ember-auto-import and webpack causes significantly higher memory use during rebuilds. This made ember-cli totally unusable on 1GB RAM / 2GB swap environments. We don't have a specific need for this upgrade right now, so reverting for now.
Random strings can result into much longer tsvectors. For example
parsing a Base64 string of ~600kb can result in a tsvector of over 1MB,
which is the maximum size of a tsvector.
Follow-up-to: 823c3f09d4
This commit introduces our own handling and warning for Sidekiq's new 'non-json-serializable' warning. This decouples us from Sidekiq's own deprecation cycle, and allows us to use our own deprecation system. It also means that the dump/parse happens in test mode, which will help us to catch occurrences before they reach production.
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
In the commit d8bf2810ff we hoisted
the userOptionFields array to a module-level variable, but kept
the code inside save() the same. This causes an issue where if
save() is called twice on the same user with some array of user
option fields, the userOptionFields array is mutated, which means
the second save is likely not saving the fields intended.
This commit fixes the issue by not mutating the array. We cannot
change them into consts though, because we have an API to add more
items to the array.
This commit allows group SMTP emails to be sent with a
different from email address that has been set up as an
alias in the email provider. Emails from the alias will
be grouped correctly using Message-IDs in the mail client,
and replies to the alias go into the correct group inbox.
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`.
Ember tests follows a convention where test files have a postfix of
`-test.js`. This ensures that any files in the tests folder which
follows this pattern is included.
Job arguments go via JSON, and so DateTime objects will appear as strings in the Job's `#execute` method. The latest version of Sidekiq has started warning about this to reduce developer confusion.
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.
The latest version of Sidekiq introduced a warning when jobs are queued with arguments which 'do not stringify to JSON safely'. In the vast majority of cases, this is because a hash is passed with symbols as keys. When those args are passed to the job, the keys will be stringified.
Our job wrapper already takes care of this issue by calling '.with_indifferent_access' on the args before passing them to `#execute`, so we don't need to change anything about our use. All we need to do is satisfy Sidekiq's warning system by 'stringifying' all the keys before enqueuing the job.
When uploading an image, we change the uploading placeholder several times. Every time, we correct the position of the cursor after replacing. But we schedule repositioning of cursor to the afterRender queue in Ember Run Loop. As a result, sometimes we replace the placeholder several times but correct the cursor position only once at the end.
It just cannot work correctly with scheduling, we'll always be dealing with cumulative error. Removing scheduling fixes the problem.
Sadly, I cannot make the test work, I skipped it for now, going to give it another try later.
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 commit adds a requestCustomMarkdownCookFunction function
to the `helper` that is provided to custom markdown rules
via their `setup` function.
The way this works is that once the default markdown engine that
we use for cooking posts has been set up, we loop through all
of the callbacks registered by `requestCustomMarkdownCookFunction`
and call `_buildCustomMarkdownCookFunction`. This creates
a new markdown engine using many of the same settings as the
default one, but will allow for the following options to be
changed by the markdown rule requesting the custom function:
* featuresOverride - The markdown-it features to allow for the engine
* markdownItRules - The markdown-it rules to allow for the engine
After this engine is set up a render function which renders + sanitizes
the output is returned for use by the markdown rule.
The use case for this API is mainly for block BBCode markdown rules
which want to render their content with a limited subset of the
markdown features/rules. Our initial use case for this is chat message
quoting.
This commit also does some minor refactoring of discourse-markdown-it
to accommodate this new engine building.
When changing to uppy for file uploads we forgot to add
these conditions to the paste event from 9c96511ec4
Basically, if you are pasting more than just a file (e.g. text,
html, rtf), then we should not handle the file and upload it, and
instead just paste in the text. This causes issues with spreadsheet
tools, that will copy the text representation and also an image
representation of cells to the user's clipboard.
This also moves the paste event for composer-upload-uppy to the
element found by the `editorClass` property, so it shares the paste
event with d-editor (via TextareaTextManipulation), which makes testing
this possible as the ember paste bindings are not picked up unless both
paste events are on the same element.
Adds up and down buttons next to the inputs of value lists when there is more than 1 item present. This helps to re-order the items in the value lists if necessary.
- 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
In our legacy environment, Ember RFC176 shims are included in `discourse-loader.js` which is part of the `vendor.js` bundle. This meant that the module shims were available as soon as the vendor.js asset was loaded.
Under Ember CLI, we were defining these shims in `discourse-boot.js`. This is loaded by the browser much later, and meant that the shims were not available to themes/plugins that call `require()` before Discourse has booted. This was causing errors under some circumstances.
This commit refactors the Ember CLI implementation so that the shims are included in the vendor.js bundle. This is done via an addon which leans on the ember-rfc176-data NPM package. This will ensure we have all the definitions, without the need for manual copy/paste.
In our legacy environment, Ember RFC176 shims are included in `discourse-loader.js` which is part of the `vendor.js` bundle. This meant that the module shims were available as soon as the vendor.js asset was loaded.
Under Ember CLI, we were defining these shims in `discourse-boot.js`. This is loaded by the browser much later, and meant that the shims were not available to themes/plugins that call `require()` before Discourse has booted. This was causing errors under some circumstances.
This commit refactors the Ember CLI implementation so that the shims are included in the vendor.js bundle. This is done via an addon which leans on the ember-rfc176-data NPM package. This will ensure we have all the definitions, without the need for manual copy/paste.
Previously, `resetSite()` would immediately generate a new `Site` instance, and run all the initialization logic within the model. This included initializing Category objects.
This was problematic because `resetSite()` is called before any initializers have been run. That means that any modifications to the Site or Category classes would not have any effect on the already-initialized Site/Category instances.
This commit makes two main changes so so that the test environment is more production-like:
1. Update `resetSite` so that it simply stores the new data in the PreloadStore, and destroys the old Site instance. Initialization of a new site instance happens 'just in time' (normally during the `inject-discourse-objects` initializer)
2. Update the `helperContext` in tests to use getters. This avoids the need to look up `Site.current()` before initializers have run
It also makes a minor adjustment to one test which was relying on a side-effect of the previous behavior.
This should resolve the failing tests for discourse-category-expert under Ember-CLI: https://github.com/discourse/discourse-category-experts/pull/69
Non-staff users are not allowed to see whisper so this change prevents
non-staff user from seeing a like count that does not make sense to
them. In the future, we might consider adding another like count column
for staff user.
Follow-up to 4492718864
This also switches to using the NPM package for better build stability. And adds a clearer label in the alert that is displayed to show your current timezone (when changing timezones).
* Some are no longer flaky or easily fixed
* Some are out of date or test things we can't do accurately (scroll
position) and are removed.
* Unwinds some uppy tests and makes sure all promises and runloops are
resolved.
Everything has been run in legacy/ember cli multiple times to ensure no
obvious suite regressions.
When the record is not saved, we should display a proper message.
One potential reason can be plugins for example discourse-calendar is specifying that only first post can contain event
This fixes rare cases of layout shift caused by images appearing slightly smaller after being loaded.
For example, a 371x1031 image is uploaded. It gets lightboxed, with the generated thumbnail of size 179x500. `height: auto` changes that thumbnail's size (only after being loaded) to 179x497, causing a 3px shift.
I did not observe any regressions with this change.
* FIX: Remove svg icons from webmanifest shortcuts
While SVGs are valid in the webmanifest, Chromium has not implemented
support for it in this specific manifest member.
Revert when https://bugs.chromium.org/p/chromium/issues/detail?id=1091612
lands.
* fix test
We don't need raw to decide if we can fast edit or not, we will fetch the raw later when we do the replacement, but this step can be done directly from innerHTML.
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.
This reverts commit f43bba8d59.
Adding randomness has introduced a lot of flakiness in our ember-cli tests. We should fix those issues at the source. However, given the upcoming stable release, this randomness has been reverted so that the stable release includes a stable test suite. Having a stable test suite on stable will make backporting future commits much easier.
When staff visits the user profile of another user, the `email` field
in the model is empty. In this case, staff cannot send the reset email
password because nothing is passed in the `login` field.
This commit changes the behavior for staff users to allow resetting
password by username instead.
We initialize models as part of the warmup process in production, so this was being logged on every boot. We only want to log if a plugin is actually using the model, so after_save is a safer bet.
The topic ID portion of the topic URL is optional in Discourse as long as the topic slug is unique across the site. If you navigate to a topic without the ID in the URL, Discourse will redirect you to the canonical version of the URL that includes the ID.
However, we have a now regression where the client app doesn't correctly handle ID-less topic URLs displays an error message when the user clicks on such URL. The regression was introduced b537d591b3 when we switched from `DiscourseURL.routeTo` to using Ember's router to perform the redirecting to the canonical version of the URL, but the problem is that the canonical version comes from the server and it contains the hostname which the Ember router doesn't understand because it expects a relative URL.
This PR fixes the problem by constructing a relative URL that contains the topic slug and ID and passing that to the Ember route.
This commit fixes a bug where we our `HTMLScrubber` was only searching
for emoji img tags which contains only the "emoji" class. However, our emoji image tags
may contain more than just the "emoji" class like "only-emoji" when an
emoji exists by itself on a single line.
Follow up to: 12f041de5d
Probably best to lookup the "everyone" group_id instead of hard-coding
it to `0`. Also now its more clear what this `0` means.
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
```
Removes one layer of indirection in the tests. `emoji-uploader`'s
`uploadDone` can call the test handler directly without going through
an additional action method.
If a model class calls preload_custom_fields twice then
we have to clear this otherwise the fields are cached inside the
already existing proxy and no new ones are added, so when we check
for custom_fields[KEY] an error is likely to occur
This commit makes a more specific N1NotPreLoadedError from
StandardError to raise when a custom field is loaded before
being preloaded, so it is easier to test that this does
not happen from plugins. Also adds the name of the class
trying to load the custom field to the error message.
It does this by creating a new initializer that runs every time the app
is booted to track the current test. Then after each test, we see if the
app needs to be torn down.
This creates a helper function with all the cleanup tasks we need to do
after tests, then makes sure to call it after tests that previously
weren't.
This fixes a lot of flakey tests.
Testing this is kinda complicated ATM (especially mobile template with hbr) , this is a component we should definitely aim to test very extensively when we move away from hbr templates.
In the unlikely, but possible, scenario where a user has no email_tokens, and has an invite record for their email address, login would fail. This commit fixes the `Invite` `user_doesnt_already_exist` validation so that it only applies to new invites, or when changing the email address.
This regressed in d8fe0f4199 (based on `git bisect`)
The UI used to request a password reset by username when the user was
logged in. This did not work when hide_email_already_taken site setting
was enabled, which disables the lookup-by-username functionality.
This commit also introduces a check to ensure that the parameter is an
email when hide_email_already_taken is enabled as the single allowed
type is email (no usernames are allowed).
* FIX: Tag watching for everyone tag groups
Tags in tag groups that have permissions set to everyone were not able
to be saved correctly. A user on their preferences page would mark the
tags that they wanted to save, but the watched_tags in the response
would be empty. This did not apply to admins, just regular users. Even
though the watched tags were being saved in the db, the user serializer
response was filtering them out. When a user refreshed their preferences
pages it would show zero watched tags.
This appears to be a regression introduced by:
0f598ca51e
The issue that needed to be fixed is that we don't track the "everyone"
group (which has an id of 0) in the group_users table. This is because
everyone has access to it, so why fill a row for every single user, that
would be a lot. The fix was to update the query to include tag groups
that had permissions set to the "everyone" group (group_id 0).
I also added another check to the existing spec for updating
watched tags for tags that aren't in a tag group so that it checks the
response body. I then added a new spec which updates watched tags for
tags in a tag group which has permissions set to everyone.
* Resolve failing tests
Improve SQL query syntax for including the "everyone" group with the id
of 0.
This commit also fixes a few failing tests that were introduced. It
turns out that the Fabrication of the Tag Group Permissions was faulty.
What happens when creating the tag groups without any permissions is
that it sets the permission to "everyone". If we then follow up with
fabricating a tag group permission on the tag group instead of having a
single permission it will have 2 (everyone + the group specified)! We
don't want this. To fix it I removed the fabrication of tag group
permissions and just set the permissions directly when creating the tag
group.
* Use response.parsed_body instead of JSON.parse
* 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
If themes/plugins introduce a sidebar on the left of the screen, the quote button would sometimes be positioned underneath. This commit ensures that the positioning logic keeps the floating buttons within the width of `.topic-area`
Some safari-specific logic was inadvertently removed during the refactoring in b2d45c59. This commit restores it. The logic requires some state, so the getRangeBoundaryRect helper has to be moved back into the Component class. The functional change in this commit is the three lines enclosed by `if (this.capabilities.isSafari) {`.
As part of /t/10298, try to remove the first flaky test in the list.
One finding is that the /t/280 topic has a very long post stream, so that may have caused some delay when rendering the topic. One way is to wait for the first expected element to load, but that doesn't scale well given how many waits we will need to add. So I chose to render a shorter topic instead.
Previously we were adding `/assets/discourse/tests/core_plugin_tests.js` to the test html all the time. This works in development mode, but fails silently when using testem via the `ember test` CLI, because there is no proxy running.
This commit makes a few changes to fix this, and make it more useful:
- Only renders the plugin `<script>` when in development mode, or when `LOAD_PLUGINS=1` (matching core's behavior)
- Only loads plugin translations based on the same logic
- When running via testem, and the above conditions are met, testem is configured to proxy `core_plugin_tests.js` through to a rails server. (port based on the `UNICORN_PORT` env variable)
- Adds a descriptive error if the plugin `<script>` fails to load. This can happen if the rails server hasn't been started
- Updates the logic for testem browsers. Ember CLI always launches testem in "CI" mode, and we don't really want 3 browsers opening by default. Our CI explicitly specifies the 3 browsers at runtime
This expands cbf99f48 to apply to all mobile devices. It removes the old mobile positioning logic entirely, refactors the new system a little for robustness and readability, and removes some JQuery.
On Andoid, we also need to avoid the start selection handle. Therefore the logic for locating selection boundaries is abstracted into a function for easier re-use.
* FEATURE: Export topics to markdown
The route `/raw/TOPIC_ID` will now export whole topics (paginated to 100
posts) in a markdown format.
See https://meta.discourse.org/t/-/152185/12
Previously the picker would attempt to avoid positioning itself hover textarea and could in limited width screen end up being out of screen.
This behavior would be even more probable on full screen mode where the textarea takes a lot of space.
We already set border-radius to 0 on all input elements, but we didn't do that for textarea, which resulted in some of those elements appearing rounded on some browsers (iOS Safari)
* The current evaluation of uppy promises is causing the entire suite to fail
if there's an exception. Instead of using `done` we use the simpler
pattern of returning the promise from the test to force Qunit to wait
until it's completed.
* In some browser conditions `/last.json` will be requested depending on the
particular scroll / performance. This causes the tests not to fail if
that is the case.
* Keyboard shortcuts were not being fully cleared between runs,
resulting in tests failures.
This commit adds a hover effect for drag and drop in
the admin emoji uploader. It also changes the "Add New
Emoji" button to open the file selector; previously it
was useless because it was disabled unless a name was
entered (which is not even a requirement for the emoji)
and also it didn't actually do anything on click even
if it wasn't disabled.
Now we have a way of adding files without having to drag
and drop them, which is nice.
Also in this PR, there was no indication before that the upload was
complete apart from the button becoming enabled again.
This commit adds the highlight class to the emoji list
and removes it once the highlight fade animation is done,
like we do for new posts.
Tapping within ~50px of the selection end handle on iOS doesn't trigger a click event. This commit ensures that our quote buttons are always at least 50px away from the end handle. It will try 4 positions in order of preference:
1. The original position
2. 50px to the left of the handle
3. 50px to the right of the handle
4. 50px below the handle, centered on the handle
Follow up to 48f70dcd5f. The group
_appeared_ to be saved in the UI until a refresh when it became
clear that the group wasn't actually sent to the DB. This is because
of the way the per-file data was being set with a computed property.
This commit fixes the computed property by changing it to a regular
function and also makes sure the name resetting after the first upload
in multiple uploads works too.
When uploading multiple emoji in Admin/Customize/Emojis
with an emoji Group selected, the group was cleared between
each file uploaded, making bulk uploading of emojis a chore
if anything but the default group was needed.
This commit fixes the issue, introduces tests for emoji-uploader,
and also adds `add-files` appEvents for uppy-upload mixin, same
as the composer-upload-uppy mixin, for interop with tests and so
we don't have to rely on a file upload element's change event.
This reverts commit 2c7906999a.
The changes break some things in local development (putting JS files
into minified files, not allowing debugger, and others)
This is a workaround a behavior change in Chromium v97.
The following text was sent to the blink-dev mailing list:
> This change broke a SingleSignOn login on the FOSS software Discourse. We have a flow like:
>
> 1. User visits forum.siteA.com, click login
> 2. Gets redirected to idp.siteB.com
> 3. Fills login details
> 4. Gets redirected to forum.siteA.com/session/sso_login?parameters
> 5. Gets redirected to forum.siteA.com/homepage
>
> On step 4, the response includes a `set-cookie` header, with proper `HttpOnly; SameSite=Lax; Secure `and set. But if there is an active service worker, the login will fail as that cookie will be rejected by Chromium due to SameSite rules now.
>
> t=2971 [st=258] COOKIE_INCLUSION_STATUS
> --> domain = "forum.siteA.com"
> --> name = "_t"
> --> operation = "store"
> --> path = "/"
> --> status = "EXCLUDE_SAMESITE_LAX, DO_NOT_WARN"
>
> The service worker is a vanilla WorkboxJS service worker that intercepts all GETs with the "Network First" strategy.
>
> Disabling the service worker or using Firefox results in a successful login. There is no warning in either DevTools network tab nor the console that the cookie was rejected.
>
> Chrome 96: login works
> Chrome 97: login does not work
> Chrome 98: login does not work
>
> Is this expected behavior? Even if the request `GET forum.siteA.com` was initiated because of a redirect from a different domain, is it expected that Chrome will silently drop same site cookies from forum.siteA.com?
Currently when pressing Shift key and hitting Reply button the user
stays on the post they are on and does not get navigated to newly
created topic/PM/reply. This is fine for replies but creates confusion
when composing a new topic/PM.
This commits makes it such that pressing Shift key and Reply button
(or ctrl-shift-enter / cmd-shift-enter) works only for replies and not
for new topic/PM. The user will always be navigated to new topic/PM.
This means that our DiscourseURL logic will work consistently in tests, where `window.location` doesn't get updated.
To make it work properly, our `replaceState` implementation needed to be updated so that it writes the new URL to Ember's router, rather than bypassing the router and going straight to the `location` API.
A couple of tests needed updating following this fix:
- the composer-test was asserting that the new reply should be missing from the DOM... when really it **should** be in the DOM, and this fix to the test environment makes it so
- the topic-test was making a fake topic fixture based on the data from a topic with a different id. This was causing the topic route to get confused, and 'fix' the currentURL. This commit updates it to use a fixture with consistent data.
This commit also removes the feature detection of `window.history`. It's feature-detected within `discourse-location`. Plus, we don't support any browsers without it.
Previously only `<div>one top element</div>` was allowed because we use `firstChild` instead of `children`.
We also want `<div>one</div><div>two</div>` to work with this method.
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.
- Update the TOPIC_URL_REGEXP in `lib/url` so that `navigatedToPost` doesn't attempt to handle slug-less URLs. Slugs must contain at least one non-numeric character, so we can use that fact to make the regex more specific. We want slug-less URLs to be routed as a normal Ember transition, so that `topic-by-slug-or-id` can catch them and re-write the URL to include the slug.
- Update the `topic-by-slug-or-id` afterModel to ensure that the Ember router is used to handle the redirect, rather than DiscourseURL. This guarantees that it will function as a redirect (DiscourseURL.routeTo sometimes bypasses the router). This solves the history problem which was worked-around in 27211ee7bb.
- Update routes/topic to recover from aborted transitions gracefully. This means that following an aborted transition, the browser URL continues to be updated with post numbers as the user scrolls down the page.
An admin could search for all screened ip addresses in a block by
using wildcards. 192.168.* returned all IPs in range 192.168.0.0/16.
This feature allows admins to search for a single IP address in all
screened IP blocks. 192.168.0.1 returns all IP blocks that match it,
for example 192.168.0.0/16.
* FEATURE: Remove roll up button for screened IPs
* FIX: Match more specific screened IP address first
The new warnings cover more cases and more accurate. Most of the
warnings will be visible only to staff members because otherwise they
would leak information about user's preferences.
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.
…after you re-open the modal or select another emoji.
Reason:
Even the most used emoji would be knocked off the list after a while, if you use any emoji outside the recent. Consider the sequence:
✅, 😃, ✅ (from recent), 😀, ✅ (from recent), 😛, ✅ (from recent), 😎, ✅ (from recent), and so on
With the previous logic, the check mark emoji would leave the list, even though it used constantly and (and the time of removal) would the the second most recent used emoji.
---
It doesn't update the list when you use the recent list so that you can click an emoji repeatedly and it doesn't shift from under your mouse cursor.
Penar Musaraj
Osama Sayegh
megothss
Krzysztof Kotlarek
Dan Ungureanu
Kris
David Taylor
Bianca Nenciu
Martin Brennan
Vinoth Kannan
Alan Guo Xiang Tan
Sam
Gerhard Schlager
Mark VanLandingham
Rafael dos Santos Silva
janzenisaac
Jordan Vidrine
Jarek Radosz
Ayke Halder
Joffrey JAFFEUX
Blake Erickson
Martin Brennan
Andrei Prigorshnev
Keegan George
Natalie Tay
Robin Ward
tshenry
Kyle E. Mitchell
Régis Hanol
Mark VanLandingham
Roman Rizzi
Arpit Jalan