* DEV: Sanitize HTML admin inputs
This PR adds on-save HTML sanitization for:
Client site settings
translation overrides
badges descriptions
user fields descriptions
I used Rails's SafeListSanitizer, which [accepts the following HTML tags and attributes](018cf54073/lib/rails/html/sanitizer.rb (L108))
* Make sure that the sanitization logic doesn't corrupt settings with special characters
This PR doesn't change any behavior, but just removes code that wasn't in use. This is a pretty dangerous place to change, since it gets called during user's registration. At the same time the refactoring is very straightforward, it's clear that this code wasn't doing any work (it still needs to be double-checked during review though). Also, the test coverage of UserNameSuggester is good.
By default, Rails only includes the Vary:Accept header in responses when the Accept: header is included in the request. This means that proxies/browsers may cache a response to a request with a missing Accept header, and then later serve that cached version for a request which **does** supply the Accept header. This can lead to some very unexpected behavior in browsers.
This commit adds the Vary:Accept header for all requests, even if the Accept header is not present in the request. If a format parameter (e.g. `.json` suffix) is included in the path, then the Accept header is still omitted. (The format parameter takes precedence over any Accept: header, so the response is no longer varies based on the Accept header)
Not reseting the registry could lead to assets still being registered for example.
This flakky spec was reprdocible with this call: `bundle exec rspec --seed 9472 spec/components/discourse_plugin_registry_spec.rb spec/components/svg_sprite/svg_sprite_spec.rb`
Which would trigger the following error:
```
Failures:
1) DiscoursePluginRegistry#register_asset registers vendored_core_pretty_text properly
Failure/Error: expect(registry.javascripts.count).to eq(0)
expected: 0
got: 1
(compared using ==)
# ./spec/components/discourse_plugin_registry_spec.rb:248:in `block (3 levels) in <top (required)>'
# ./spec/rails_helper.rb:280:in `block (2 levels) in <top (required)>'
# /Users/joffreyjaffeux/.gem/ruby/2.7.3/gems/webmock-3.14.0/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
```
When inviting a group to a topic, there may be members of
the group already in the topic as topic allowed users. These
can be safely removed from the topic, because they are implicitly
allowed in the topic based on their group membership.
Also, this prevents issues with group SMTP emails, which rely
on the topic_allowed_users of the topic to send to and cc's
for emails, and if there are members of the group as topic_allowed_users
then that complicates things and causes odd behaviour.
We also ensure that the OP of the topic is not removed from
the topic_allowed_users when a group they belong to is added,
as it will make it harder to add them back later.
User API keys (not the same thing as admin API keys) are currently
leaked to redis when rate limits are applied to them since redis is the
backend for rate limits in Discourse and the API keys are included in
the redis keys that are used to track usage of user API keys in the last
24 hours.
This commit stops the leak by using a SHA-256 representation of the user
API key instead of the key itself to form the redis key.
We don't need to manually delete the existing redis keys that contain
unhashed user API keys because they're not long-lived and will be
automatically deleted within 48 hours after this commit is deployed to
your Discourse instance.
This is a follow-up to https://github.com/discourse/discourse/pull/14541. This adds a hidden setting for restoring the old behavior for those users who rely on it. We'll likely deprecate this setting at some point in the future.
This commit removes the recipient's username from the
respond to / participants list that is shown at the bottom
of user notification emails. For example if the recipient's
username was jsmith, and there were participants ljones and
bmiller, we currently show this:
> "reply to this email to respond to jsmith, ljones, bmiller"
or
> "Participants: jsmith, ljones, bmiller"
However this is a bit redundant, as you are not replying to
yourself here if you are the recipient user. So we omit the
recipient user's username from this list, which is only used
in the text of the email and not elsewhere.
The method was only used for mega topics but it was redundant as the
first post can be determined from using the condition where
`Post#post_number` equal to one.
* FEATURE: Cache CORS preflight requests for 2h
Browsers will cache this for 5 seconds by default. If using MessageBus
in a different domain, Discourse will issue a new long polling, by
default, every 30s or so. This means we would be issuing a new preflight
request **every time**. This can be incredibly wasteful, so let's cache
the authorization in the client for 2h, which is the maximum Chromium
allows us as of today.
* fix tests
`/u/username/invited.json?filter=expired` and `/u/username/invited.json?filter=pending` APIs are already returning data to admins. However, the `can_see_invite_details?` boolean was false, which prevented the Ember frontend from showing the tabs correctly. This commit updates the guardian method to match reality.
* FIX: do not display add to calendar for past dates
There is no value in saving past dates into calendar
* FIX: remove postId and move ICS to frontend
PostId is not necessary and will make the solution more generic for dates which doesn't belong to a specific post.
Also, ICS file can be generated in JavaScript to avoid calling backend.
Sometimes administrators want to permanently delete posts and topics
from the database. To make sure that this is done for a good reasons,
administrators can do this only after one minute has passed since the
post was deleted or immediately if another administrator does it.
We don't want to be using emails as source for username and name suggestions in cases when it's possible that a user have no chance to intervene and correct a suggested username. It risks exposing email addresses.
Previosuly, quotes from original topics are rendered incorrectly since the moved posts are not rebaked.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
* FIX: Remove List-Post email header
This header is used for mailing lists and can confuse some email clients
such as Thunderbird to display wrong replying options.
* FIX: Replace reply_key in email custom headers
Admins can add custom email headers from site settings. Email sender
will try to replace the reply key if %{reply_key} exists or remove the
header if a reply key does not exist.
Calling create_notification_alert could still send a notification to a
suspended user. This just moves the check if user is suspended right
before sending the notification.
Invite is used in two contexts, when inviting a new user to the forum
and when inviting an existent user to a topic. The first case is more
complex and it involves permission checks to ensure that new users can
be created. In the second case, it is enough to ensure that the topic
is visible for both users and that all preconditions are met.
One edge case is the invite to topic via email functionality which
checks for both conditions because first the user must be invited to
create an account first and then to the topic.
A side effect of these changes is that all site settings related to
invites refer to inviting new users only now.
We aren't translating these settings, so it makes more sense to move them into the code. I added an instance method so plugins can add mappings for custom reasons.
- Allow the `/presence/get` endpoint to return multiple channels in a single request (limited to 50)
- When multiple presence channels are initialized in a single Ember runloop, batch them into a single GET request
- Introduce the `presence-pretender` to allow easy testing of PresenceChannel-related features
- Introduce a `use_cache` boolean (default true) on the the server-side PresenceChannel initializer. Useful during testing.
Change the type for default_calendar to a string.
The type specified for the default calendar in the api docs wasn't a
valid type. The linting in the api docs repo reports:
```
`type` can be one of the following only: "object", "array", "string", "number", "integer", "boolean", "null".
```
This linting currently is only in the `discourse_api_docs` repo.
* DEV: Add include_subcategories param to api docs
Adding the `include_subcategories=true` query param to the
`/categories.json` api docs.
Follow up to: fe676f334a
* fix spec
This commit fixes the `eval` and `evalsha` commands/methods and any other methods that don't have a wrapper in `DiscourseRedis` and expect keyword arguments. I noticed this problem in Logster when I was trying to fetch some log messages in JSON format using the rails console and saw the messages were missing the `env` field. Logster uses the `eval` command to fetch messages `env`s:
dc351fd00f/lib/logster/redis_store.rb (L250-L253)
and that code was not fetching anything because `DiscourseRedis` didn't pass the `keys` keyword arg to the redis gem.
It allows saving local date to calendar.
Modal is giving option to pick between ics and google. User choice can be remembered as a default for the next actions.
* FEATURE: Return subcategories on categories endpoint
When using the API subcategories will now be returned nested inside of
each category response under the `subcategory_list` param. We already
return all the subcategory ids under the `subcategory_ids` param, but
you then would have to make multiple separate API calls to fetch each of
those subcategories. This way you can get **ALL** of the categories
along with their subcategories in a single API response.
The UI will not be affected by this change because you need to pass in
the `include_subcategories=true` param in order for subcategories to be
returned.
In a follow up PR I'll add the API scoping for fetching categories so
that a readonly API key can be used for the `/categories.json` endpoint. This
endpoint should be used instead of the `/site.json` endpoint for
fetching a sites categories and subcategories.
* Update PR based on feedback
- Have spec check for specific subcategory
- Move comparison check out of loop
- Only populate subcategory list if option present
- Remove empty array initialization
- Update api spec to allow null response
* More PR updates based on feedback
- Use a category serializer for the subcategory_list
- Don't include the subcategory_list param if empty
- For the spec check for the subcategory by id
- Fix spec to account for param not present when empty
Usually, when an email is received a user lookup is performed using the
email address found in the `From` header. When an email has an
`X-Original-From` header, if it is equal to `Reply-To` then it uses that
one instead. The comparison was sensitive to whitespaces and other
insignificant characters such as quotes because it reconstructed the
`From` header.
For the fixture added in this commit, it compared the reconstructed
`From` header `John Doe <johndoe@example.com>` with the `Reply-To`
header `"John Doe" <johndoe@example.com>`.
We relied on backticks to identify and replace site setting names with links. Unfortunately, some translations don't follow this convention, breaking this feature.
Additionally, this lets us linkify `category settings` and `watched words` without using HTML in the translations.
You may notice that I split the texts we want to linkify into two groups. I did this on purpose to emphasize those that should be translated (regular_links) from those who don't (site_settings_link). If you can think of a better solution, I'm open to suggestions.
* DEV: Remove HTML setting type and sanitization logic.
We concluded that we don't want settings to contain HTML, so I'm removing the setting type and sanitization logic. Additionally, we no longer allow the global-notice text to contain HTML.
I searched for usages of this setting type in the `all-the-plugins` repo and found none, so I haven't added a migration for existing settings.
* Mark Global notices containing links as HTML Safe.
FinalDestination now supports the `follow_canonical` option, which will perform an initial GET request, parse the canonical link if present, and perform a HEAD request to it.
We use this mode during embeds to avoid treating URLs with different query parameters as different topics.
Previously, while retrieving each upload urls in a post S3 CDN urls with different path in prefix (external urls technically) are considered as uploaded url. It created issue while checking missing uploads.
Roman Rizzi
Andrei Prigorshnev
Dan Ungureanu
Bianca Nenciu
David Taylor
Joffrey JAFFEUX
Martin Brennan
Osama Sayegh
Arpit Jalan
Jean
Alan Guo Xiang Tan
Rafael dos Santos Silva
Krzysztof Kotlarek
Yasuo Honda
Vinoth Kannan
Robin Ward
Blake Erickson
Penar Musaraj