Commit Graph

115 Commits

Author SHA1 Message Date
Loïc Guitaut
f58b844f45
Revert "DEV: Upgrade Rails to version 7.1" (#27625)
This reverts commit ce00f83173.
2024-06-26 18:55:05 +02:00
Ted Johansson
d63f1826fe
FEATURE: User fields required for existing users - Part 2 (#27172)
We want to allow admins to make new required fields apply to existing users. In order for this to work we need to have a way to make those users fill up the fields on their next page load. This is very similar to how adding a 2FA requirement post-fact works. Users will be redirected to a page where they can fill up the remaining required fields, and until they do that they won't be able to do anything else.
2024-06-25 19:32:18 +08:00
Loïc Guitaut
ce00f83173 DEV: Upgrade Rails to version 7.1
---------

Co-authored-by: Jarek Radosz <jradosz@gmail.com>
2024-06-24 11:16:14 +02:00
Loïc Guitaut
160011793a Revert "DEV: Upgrade Rails to version 7.1 (#27539)"
This reverts commit ca4af53be8.
2024-06-21 11:20:40 +02:00
Loïc Guitaut
ca4af53be8 DEV: Upgrade Rails to version 7.1 (#27539)
* DEV: Upgrade Rails to 7.1

* FIX: Remove references to `Rails.logger.chained`

`Rails.logger.chained` was provided by Logster before Rails 7.1
introduced their broadcast logger. Now all the loggers are added to
`Rails.logger.broadcasts`.

Some code in our initializers was still using `chained` instead of
`broadcasts`.

* DEV: Make parameters optional to all FakeLogger methods

* FIX: Set `override_level` on Logster loggers (#27519)

A followup to f595d599dd

* FIX: Don’t duplicate Rack response

---------

Co-authored-by: Jarek Radosz <jradosz@gmail.com>
2024-06-21 09:44:06 +02:00
Loïc Guitaut
982c005979 Revert "DEV: Upgrade Rails to version 7.1 (#27539)"
This reverts commit 2301dddcff.
2024-06-20 11:43:35 +02:00
Loïc Guitaut
2301dddcff
DEV: Upgrade Rails to version 7.1 (#27539)
* DEV: Upgrade Rails to 7.1

* FIX: Remove references to `Rails.logger.chained`

`Rails.logger.chained` was provided by Logster before Rails 7.1
introduced their broadcast logger. Now all the loggers are added to
`Rails.logger.broadcasts`.

Some code in our initializers was still using `chained` instead of
`broadcasts`.

* DEV: Make parameters optional to all FakeLogger methods

* FIX: Set `override_level` on Logster loggers (#27519)

A followup to f595d599dd

* FIX: Don’t duplicate Rack response

---------

Co-authored-by: Jarek Radosz <jradosz@gmail.com>
2024-06-20 10:33:01 +02:00
Krzysztof Kotlarek
0e1102b332
FIX: do not force configure 2FA when OAuth and not enforced (#27547)
In this PR we introduced `enforce_second_factor_on_external_auth` setting https://github.com/discourse/discourse/pull/27506

When it is set to false and the user is authenticated via OAuth, then we should not enforce the 2fa configuration.
2024-06-20 11:49:46 +10:00
Ted Johansson
9468e0c0f2
DEV: More robust referrer host parsing (#27534) 2024-06-19 16:30:40 +08:00
Ted Johansson
3ff7ce78e7
FEATURE: Add hidden site setting to list 'unsafe-none' COOP referrers (#27510)
Some tooling may rely on an unsafe-none cross origin opener policy to work. This change adds a hidden site setting that can be used to list referrers where we add this header instead of the default one configured in cross_origin_opener_policy_header.
2024-06-19 11:11:35 +08:00
Jarek Radosz
5cb84f8dcf
DEV: Revert rails 7.1 upgrade (#27522)
* Revert "FIX: Set `override_level` on Logster loggers (#27519)"

This reverts commit c1b0488c54.

* Revert "DEV: Make parameters optional to all FakeLogger methods"

This reverts commit 3318dad7b4.

* Revert "FIX: Remove references to `Rails.logger.chained`"

This reverts commit f595d599dd.

* Revert "DEV: Upgrade Rails to 7.1"

This reverts commit 081b00391e.
2024-06-18 23:48:30 +02:00
Loïc Guitaut
081b00391e DEV: Upgrade Rails to 7.1 2024-06-18 15:58:05 +02:00
Osama Sayegh
361992bb74
FIX: Apply crawler rate limits to cached requests (#27174)
This commit moves the logic for crawler rate limits out of the application controller and into the request tracker middleware. The reason for this move is to apply rate limits to all crawler requests instead of just the requests that make it to the application controller. Some requests are served early from the middleware stack without reaching the Rails app for performance reasons (e.g. `AnonymousCache`) which results in crawlers getting 200 responses even though they've reached their limits and should be getting 429 responses.

Internal topic: t/128810.
2024-05-27 16:26:35 +03:00
David Taylor
f230767722
FIX: Serialization of staff_writes_only (#26866) 2024-05-03 14:36:13 -04:00
David Taylor
1cc8c72a98
DEV: Consolidate experimental 'Link' header implementations (#26377)
This commit removes the 'experimental_preconnect_link_header' site setting, and the 'preload_link_header' site setting, and introduces two new global settings: early_hint_header_mode and early_hint_header_name.

We don't actually send 103 Early Hint responses from Discourse. However, upstream proxies can be configured to cache a response header from the app and use that to send an Early Hint response to future clients.

- `early_hint_header_mode` specifies the mode for the early hint header. Can be nil (disabled), "preconnect" (lists just CDN domains) or "preload" (lists all assets).
- `early_hint_header_name` specifies which header name to use for the early hint. Defaults to "Link", but can be changed to support different proxy mechanisms.
2024-03-27 09:06:50 +00:00
Alan Guo Xiang Tan
27b0ebff4c
DEV: Fix syntax for Link entity header for experimental_preconnect_link_header (#26218)
Per https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link, the
syntax for multiple links is something like

```
Link: <https://one.example.com>; rel="preconnect", <https://two.example.com>; rel="preconnect", <https://three.example.com>; rel="preconnect"
```

There should be no trailing `;` before the `,`.
2024-03-18 19:49:16 +08:00
Alan Guo Xiang Tan
36cdb1444c
EXPERIMENTAL: preconnect and dns-prefetch resource hints for CDN domains (#26215)
Why this change?

In https://web.dev/articles/preconnect-and-dns-prefetch, it describes
how hinting to the browser to preconnect to domains which we will
eventually use the connection for can help improve the time it takes to
load a page.

We are putting this behind an experimental flag so that we can test and
profile this in a production environment.

What does this change introduce?

Introduce a hidden experimental `experimental_preconnect_link_header`
site setting which when enabled will add the `preconnect` and
`dns-prefetch` resource hints to the response headers for full page load
requests.
2024-03-18 13:45:41 +08:00
David Taylor
2546817d07
FIX: correctly strip unneeded csp directives under strict-dynamic (#26180) 2024-03-14 18:50:09 +00:00
Krzysztof Kotlarek
9afb0b29f8
FEATURE: filter additional keywords for the sidebar (#26148)
With the new admin sidebar restructure, we have a link to "Installed plugins". We would like to ensure that when the admin is searching for a plugin name like "akismet" or "automation" this link will be visible. Also when entering the plugins page, related plugins should be highlighted.
2024-03-14 12:28:08 +11:00
David Taylor
92d357f91a
FEATURE: Enable strict-dynamic Content-Security-Policy by default (#26051)
Ref https://meta.discourse.org/t/298172 and https://meta.discourse.org/t/295603
2024-03-07 15:20:31 +00:00
David Taylor
b1f74ab59e
FEATURE: Add experimental option for strict-dynamic CSP (#25664)
The strict-dynamic CSP directive is supported in all our target browsers, and makes for a much simpler configuration. Instead of allowlisting paths, we use a per-request nonce to authorize `<script>` tags, and then those scripts are allowed to load additional scripts (or add additional inline scripts) without restriction.

This becomes especially useful when admins want to add external scripts like Google Tag Manager, or advertising scripts, which then go on to load a ton of other scripts.

All script tags introduced via themes will automatically have the nonce attribute applied, so it should be zero-effort for theme developers. Plugins *may* need some changes if they are inserting their own script tags.

This commit introduces a strict-dynamic-based CSP behind an experimental `content_security_policy_strict_dynamic` site setting.
2024-02-16 11:16:54 +00:00
Martin Brennan
3cc73cfd1e
FIX: Always preload admin plugin list for admin in sidebar (#25606)
When we show the links to installed plugins in the admin
sidebar (for plugins that have custom admin routes) we were
previously only doing this if you opened /admin, not if you
navigated there from the main forum. We should just always
preload this data if the user is admin.

This commit also changes `admin_sidebar_enabled_groups` to
not be sent to the client as part of ongoing efforts to
not check groups on the client, since not all a user's groups
may be serialized.
2024-02-09 12:52:22 +10:00
Ted Johansson
57ea56ee05
DEV: Remove full group refreshes from tests (#25414)
We have all these calls to Group.refresh_automatic_groups! littered throughout the tests. Including tests that are seemingly unrelated to groups. This is because automatic group memberships aren't fabricated when making a vanilla user. There are two places where you'd want to use this:

You have fabricated a user that needs a certain trust level (which is now based on group membership.)
You need the system user to have a certain trust level.
In the first case, we can pass refresh_auto_groups: true to the fabricator instead. This is a more lightweight operation that only considers a single user, instead of all users in all groups.

The second case is no longer a thing after #25400.
2024-01-25 14:28:26 +08:00
Alan Guo Xiang Tan
38abc0d922
DEV: Change default of cross_origin_opener_policy_header (#24940)
Why this change?

This is part of our efforts to harden the security of the Discourse
application. Setting the `CROSS_ORIGIN_OPENER_POLICY` header to `same-origin-allow-popups`
by default makes the application safer. We have opted to make this a
hidden site setting because most admins will never have to care about
this setting so we're are opting not to show it. If they do have to
change it, they can still do so by setting the
`DISCOURSE_CROSS_ORIGIN_OPENER_POLICY` env.
2023-12-19 11:46:44 +08:00
David Taylor
ac896755bb
DEV: Simplify ember-cli proxy strategy (#24242)
Previously, the app HTML served by the Ember-CLI proxy was generated based on a 'bootstrap json' payload generated by Rails. This inevitably leads to differences between the Rails HTML and the Ember-CLI HTML.

This commit overhauls our proxying strategy. Now, we totally ignore the ember-cli `index.html` file. Instead, we take the full HTML from Rails and surgically replace script URLs based on a `data-discourse-entrypoint` attribute. This should be faster (only one request to Rails), more robust, and less confusing for developers.
2023-11-10 11:16:06 +00:00
Daniel Waterworth
6e161d3e75
DEV: Allow fab! without block (#24314)
The most common thing that we do with fab! is:

    fab!(:thing) { Fabricate(:thing) }

This commit adds a shorthand for this which is just simply:

    fab!(:thing)

i.e. If you omit the block, then, by default, you'll get a `Fabricate`d object using the fabricator of the same name.
2023-11-09 16:47:59 -06:00
Martin Brennan
9db4eaa870
DEV: Change anonymous_posting_min_trust_level to a group-based setting (#24072)
No plugins or themes rely on anonymous_posting_min_trust_level so we
can just switch straight over to anonymous_posting_allowed_groups

This also adds an AUTO_GROUPS const which can be imported in JS
tests which is analogous to the one defined in group.rb. This can be used
to set the current user's groups where JS tests call for checking these groups
against site settings.

Finally a AtLeastOneGroupValidator validator is added for group_list site
settings which ensures that at least one group is always selected, since if
you want to allow all users to use a feature in this way you can just use
the everyone group.
2023-10-25 11:45:10 +10:00
Alan Guo Xiang Tan
5724b7bccd
DEV: Add hidden cross_origin_opener_policy_header site setting (#23346)
Why this change?

As part of our ongoing efforts to security harden the Discourse
application, we are adding the `cross_origin_opener_policy_header` site setting
which allows the `Cross-Origin-Opener-Policy` response header to be set on requests
that preloads the Discourse application. In more technical terms, only
GET requests that are not json or xhr will have the response header set.

The `cross_origin_opener_policy_header` site setting is hidden for now
for testing purposes and will either be released as a public site
setting or be remove if we decide to be opinionated and ship a default
for the `Cross-Origin-Opener-Policy` response header.
2023-08-31 08:50:06 -04:00
OsamaSayegh
0976c8fad6
SECURITY: Don't reuse CSP nonce between anonymous requests 2023-07-28 12:53:44 +01:00
Blake Erickson
eed7d86601
SECURITY: Don't reuse CSP nonce between requests (#22544)
Co-authored-by: OsamaSayegh <asooomaasoooma90@gmail.com>
2023-07-11 15:24:36 -06:00
Martin Brennan
db80a8ce79
FIX: Preload fonts before rerendering wizard style canvas (#22361)
]When changing fonts in the `/wizard/steps/styling` step of
the wizard, users would not see the font loaded straight away,
having to switch to another one then back to the original to
see the result. This is because we are using canvas to render
the style preview and this fails with a Chrome-based intervention
when font loading is taking too long:

> [Intervention] Slow network is detected. See
https://www.chromestatus.com/feature/5636954674692096 for more details.
Fallback font will be used while loading:
https://sea2.discourse-cdn.com/business7/fonts/Roboto-Bold.ttf?v=0.0.9

We can get around this by manually loading the fonts selected using
the FontFace JS API when the user selects them and before rerendering
the canvas. This just requires preloading more information about the
fonts if the user is admin so the wizard can query this data.
2023-07-03 11:30:26 +10:00
Martin Brennan
9174716737
DEV: Remove Discourse.redis.delete_prefixed (#22103)
This method is a huge footgun in production, since it calls
the Redis KEYS command. From the Redis documentation at
https://redis.io/commands/keys/:

> Warning: consider KEYS as a command that should only be used in
production environments with extreme care. It may ruin performance when
it is executed against large databases. This command is intended for
debugging and special operations, such as changing your keyspace layout.
Don't use KEYS in your regular application code.

Since we were only using `delete_prefixed` in specs (now that we
removed the usage in production in 24ec06ff85)
we can remove this and instead rely on `use_redis_snapshotting` on the
particular tests that need this kind of clearing functionality.
2023-06-16 12:44:35 +10:00
David Taylor
cb932d6ee1
DEV: Apply syntax_tree formatting to spec/* 2023-01-09 11:49:28 +00:00
Rafael dos Santos Silva
6888eb5c2d
Revert "Revert "FEATURE: Preload resources via link header (#18475)" (#18511)" (#18531)
* Revert "Revert "FEATURE: Preload resources via link header (#18475)" (#18511)"

This reverts commit 95a57f7e0c.

* put behind feature flag

* env -> global setting

* declare global setting

* forgot one spot
2022-10-11 20:11:44 -03:00
Rafael dos Santos Silva
95a57f7e0c
Revert "FEATURE: Preload resources via link header (#18475)" (#18511)
This reverts commit 2d1dbc6f96.

We need to increase nginx proxy buffer to land this.
2022-10-07 15:08:40 -03:00
Rafael dos Santos Silva
2d1dbc6f96
FEATURE: Preload resources via link header (#18475)
Experiment moving from preload tags in the document head to preload information the the response headers.

While this is a minor improvement in most browsers (headers are parsed before the response body), this allows smart proxies like Cloudflare to "learn" from those headers and build HTTP 103 Early Hints for subsequent requests to the same URI, which will allow the user agent to download and parse our JS/CSS while we are waiting for the server to generate and stream the HTML response.

Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
2022-10-07 13:19:50 -03:00
Vinoth Kannan
076abe46fa
FEATURE: new site setting to set locale from cookie for anonymous users. (#18377)
This new hidden default-disabled site setting `set_locale_from_cookie` will set locale from anonymous user's cookie value.
2022-09-27 14:26:06 +05:30
David Taylor
0f8e4d7acc
DEV: Compile splash screen JS with ember-cli before inlining (#18150)
This lets us use all our normal JS tooling like prettier, esline and babel on the splash screen JS. At runtime the JS file is read and inlined into the HTML. This commit also switches us to use a CSP hash rather than a nonce for the splash screen.
2022-09-01 09:58:48 +01:00
Loïc Guitaut
3eaac56797 DEV: Use proper wording for contexts in specs 2022-08-04 11:05:02 +02:00
Joe
e3c71221e5
UX: Makes splash screen setting enabled by default (#17327)
We now want the splash screen to be enabled by default.
2022-07-05 02:06:28 +08:00
Joe
49905a4e6c
DEV: Use performance timings to as a baseline for the splash take 2 (#17284)
We previously relied on CSS animation-delay for the splash. This means that we can get inconsistent results based on device/network conditions.

This PR moves us to a more consistent timing based on {request time + 2 seconds}

Internal topic: /t/65378/65
2022-06-30 21:22:00 +08:00
Joe
e82a2ce9ae
UX: Introduces a splash screen behind a hidden site setting (#17094)
This PR introduces a new hidden site setting that allows admins to display a splash screen while site assets load.

The splash screen can be enabled via the `splash_screen` hidden site setting.

This is what the splash screen currently looks like

5ceb72f085.mp4

Once site assets load, the splash screen is automatically removed.

To control the loading text that shows in the splash screen, you can change the preloader_text translation string in admin > customize > text
2022-06-22 04:35:46 +08:00
Blake Erickson
ae6a907943
SECURITY: banner-info (#17071) 2022-06-13 11:10:21 -06:00
Jarek Radosz
3f0e767106
DEV: Use FakeLogger in RequestTracker specs (#16640)
`TestLogger` was responsible for some flaky specs runs:

```
Error during failsafe response: undefined method `debug' for #<TestLogger:0x0000556c4b942cf0 @warnings=1>
Did you mean?  debugger
```

This commit also cleans up other uses of `FakeLogger`
2022-05-05 09:53:54 +08:00
David Taylor
c9dab6fd08
DEV: Automatically require 'rails_helper' in all specs (#16077)
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.
2022-03-01 17:50:50 +00:00
David Taylor
cfb6199a95
FIX: Don't redirect XHR/JSON requests when login is required (#15093)
When redirecting to login, we store a destination_url cookie, which the user is then redirected to after login. We never want the user to be redirected to a JSON URL. Instead, we should return a 403 in these situations.

This should also be much less confusing for API consumers - a 403 is a better representation than a 302.
2021-12-02 15:12:25 +00:00
Osama Sayegh
7bd3986b21
FEATURE: Replace Crawl-delay directive with proper rate limiting (#15131)
We have a couple of site setting, `slow_down_crawler_user_agents` and `slow_down_crawler_rate`, that are meant to allow site owners to signal to specific crawlers that they're crawling the site too aggressively and that they should slow down.

When a crawler is added to the `slow_down_crawler_user_agents` setting, Discourse currently adds a `Crawl-delay` directive for that crawler in `/robots.txt`. Unfortunately, many crawlers don't support the `Crawl-delay` directive in `/robots.txt` which leaves the site owners no options if a crawler is crawling the site too aggressively.

This PR replaces the `Crawl-delay` directive with proper rate limiting for crawlers added to the `slow_down_crawler_user_agents` list. On every request made by a non-logged in user, Discourse will check the User Agent string and if it contains one of the values of the `slow_down_crawler_user_agents` list, Discourse will only allow 1 request every N seconds for that User Agent (N is the value of the `slow_down_crawler_rate` setting) and the rest of requests made within the same interval will get a 429 response. 

The `slow_down_crawler_user_agents` setting becomes quite dangerous with this PR since it could rate limit lots if not all of anonymous traffic if the setting is not used appropriately. So to protect against this scenario, we've added a couple of new validations to the setting when it's changed:

1) each value added to setting must 3 characters or longer
2) each value cannot be a substring of tokens found in popular browser User Agent. The current list of prohibited values is: apple, windows, linux, ubuntu, gecko, firefox, chrome, safari, applewebkit, webkit, mozilla, macintosh, khtml, intel, osx, os x, iphone, ipad and mac.
2021-11-30 12:55:25 +03:00
Rafael dos Santos Silva
5647819de4
FEATURE: Send a 'noindex' header in non-canonical responses (#15026)
* FEATURE: Optionally send a 'noindex' header in non-canonical responses

This will be used in a SEO experiment.

Co-authored-by: David Taylor <david@taylorhq.com>
2021-11-25 16:58:39 -03:00
Osama Sayegh
b86127ad12
FEATURE: Apply rate limits per user instead of IP for trusted users (#14706)
Currently, Discourse rate limits all incoming requests by the IP address they
originate from regardless of the user making the request. This can be
frustrating if there are multiple users using Discourse simultaneously while
sharing the same IP address (e.g. employees in an office).

This commit implements a new feature to make Discourse apply rate limits by
user id rather than IP address for users at or higher than the configured trust
level (1 is the default).

For example, let's say a Discourse instance is configured to allow 200 requests
per minute per IP address, and we have 10 users at trust level 4 using
Discourse simultaneously from the same IP address. Before this feature, the 10
users could only make a total of 200 requests per minute before they got rate
limited. But with the new feature, each user is allowed to make 200 requests
per minute because the rate limits are applied on user id rather than the IP
address.

The minimum trust level for applying user-id-based rate limits can be
configured by the `skip_per_ip_rate_limit_trust_level` global setting. The
default is 1, but it can be changed by either adding the
`DISCOURSE_SKIP_PER_IP_RATE_LIMIT_TRUST_LEVEL` environment variable with the
desired value to your `app.yml`, or changing the setting's value in the
`discourse.conf` file.

Requests made with API keys are still rate limited by IP address and the
relevant global settings that control API keys rate limits.

Before this commit, Discourse's auth cookie (`_t`) was simply a 32 characters
string that Discourse used to lookup the current user from the database and the
cookie contained no additional information about the user. However, we had to
change the cookie content in this commit so we could identify the user from the
cookie without making a database query before the rate limits logic and avoid
introducing a bottleneck on busy sites.

Besides the 32 characters auth token, the cookie now includes the user id,
trust level and the cookie's generation date, and we encrypt/sign the cookie to
prevent tampering.

Internal ticket number: t54739.
2021-11-17 23:27:30 +03:00
David Taylor
9ac6f1d3bb
FIX: Include the Vary:Accept header on all Accept-based responses (#14647)
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)
2021-10-25 12:53:50 +01:00