Currently, `window.I18n` is defined in an old school hand written
script, inlined into locale/*.js by the Rails asset pipeline, and
then the global variable is shimmed into a pseudo AMD module later
in `module-shims.js`.
This approach has some problems – for one thing, when we add a new
V2 addon (e.g. in #23859), Embroider/Webpack is stricter about its
dependencies and won't let you `import from "I18n";` when `"I18n"`
isn't listed as one of its `dependencies` or `peerDependencies`.
This moves `I18n` into a real package – `discourse-i18n`. (I was
originally planning to keep the `I18n` name since it's a private
package anyway, but NPM packages are supposed to have lower case
names and that may cause problems with other tools.)
This package defines and exports a regular class, but also defines
the default global instance for backwards compatibility. We should
use the exported class in tests to make one-off instances without
mutating the global instance and having to clean it up after the
test run. However, I did not attempt that refactor in this PR.
Since `discourse-i18n` is now included by the app, the locale
scripts needs to be loaded after the app chunks. Since no "real"
work happens until later on when we kick things off in the boot
script, the order in which the script tags appear shouldn't be a
problem. Alternatively, we can rework the locale bundles to be more
lazy like everything else, and require/import them into the app.
I avoided renaming the imports in this commit since that would be
quite noisy and drowns out the actual changes here. Instead, I used
a Webpack alias to redirect the current `"I18n"` import to the new
package for the time being. In a separate commit later on, I'll
rename all the imports in oneshot and remove the alias. As always,
plugins and the legacy bundles (admin/wizard) still relies on the
runtime AMD shims regardless.
For the most part, I avoided refactoring the actual I18n code too
much other than making it a class, and some light stuff like `var`
into `let`.
However, now that it is in a reasonable format to work with (no
longer inside the global script context!) it may also be a good
opportunity to refactor and make clear what is intended to be
public API vs internal implementation details.
Speaking of, I took the librety to make `PLACEHOLDER`, `SEPARATOR`
and `I18nMissingInterpolationArgument` actual constants since it
seemed pretty clear to me those were just previously stashed on to
the `I18n` global to avoid polluting the global namespace, rather
than something we expect the consumers to set/replace.
This is part 2 (of 3) for passkeys support.
This adds a hidden site setting plus routes and controller actions.
1. registering passkeys
Passkeys are registered in a two-step process. First, `create_passkey`
returns details for the browser to create a passkey. This includes
- a challenge
- the relying party ID and Origin
- the user's secure identifier
- the supported algorithms
- the user's existing passkeys (if any)
Then the browser creates a key with this information, and submits it to
the server via `register_passkey`.
2. authenticating passkeys
A similar process happens here as well. First, a challenge is created
and sent to the browser. Then the browser makes a public key credential
and submits it to the server via `passkey_auth_perform`.
3. renaming/deleting passkeys
These routes allow changing the name of a key and deleting it.
4. checking if session is trusted for sensitive actions
Since a passkey is a password replacement, we want to make sure to confirm the user's identity before allowing adding/deleting passkeys. The u/trusted-session GET route returns success if user has confirmed their session (and failed if user hasn't). In the frontend (in the next PR), we're using these routes to show the password confirmation screen.
The `/u/confirm-session` route allows the user to confirm their session with a password. The latter route's functionality already existed in core, under the 2FA flow, but it has been abstracted into its own here so it can be used independently.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
For the admin plugin list we want to be able to link to
a meta topic for plugins, but we have no standard way to
do this at the moment. This adds support for meta_topic_id
alongside other plugin metadata like authors, URL etc,
that gets built into a Meta topic URL in the serializer.
To match discourse_theme CLI behavior, we should skip hidden files/directories (e.g. `.git`), and two regular directories: `node_modules/` and `src/`.
Without these excludes, it's very easy for a theme to hit the file count limit. e.g. when trying this with discourse-kanban-board, I got:
> The number of files (20366) in the theme has exceeded the maximum allowed number of files (1024)
We have a custom implementation of #symbolize_keys in our Onebox helpers. This is likely a legacy from when Onebox was a standalone gem. This change replaces all usages with either #deep_symbolize_keys from ActiveSupport, or appropriate option to the JSON parser gem used.
We have a custom implementation of #blank? in our Onebox helpers. This is likely a legacy from when Onebox was a standalone gem. This change replaces all usages with respective incarnations of #blank?, #present?, and #presence from ActiveSupport. It changes a bunch of "unless blank" to "if present" as well.
This is part 1 of 3, split up of PR #23529. This PR refactors the
webauthn code to support passkey authentication/registration.
Passkeys aren't used yet, that is coming in PRs 2 and 3.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
This commit adds support for an optional `prompt` parameter in the
payload of the /session/sso_provider endpoint. If an SSO Consumer
adds a `prompt=none` parameter to the encoded/signed `sso` payload,
then Discourse will avoid trying to login a not-logged-in user:
* If the user is already logged in, Discourse will immediately
redirect back to the Consumer with the user's credentials in a
signed payload, as usual.
* If the user is not logged in, Discourse will immediately redirect
back to the Consumer with a signed payload bearing the parameter
`failed=true`.
This allows the SSO Consumer to simply test whether or not a user is
logged in, without forcing the user to try to log in. This is useful
when the SSO Consumer allows both anonymous and authenticated access.
(E.g., users that are already logged-in to Discourse can be seamlessly
logged-in to the Consumer site, and anonymous users can remain
anonymous until they explicitly ask to log in.)
This feature is similar to the `prompt=none` functionality in an
OpenID Connect Authentication Request; see
https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
If a user somehow is looking at an old version of the page and attempts
to like a post they already like. Display a more reasonable error message.
Previously we would display:
> You are not permitted to view the requested resource.
New error message is:
> Oops! You already performed this action. Can you try refreshing the page?
Triggering this error condition is very tricky, you need to stop the
message bus. A possible reason for it could be bad network connectivity.
We will soon be dropping support for `/theme-qunit` in production, so this will start failing if we don't remove it. Plus, we now have system specs which verify the end-to-end functionality of the Theme QUnit system.
This was the last thing which was using the legacy `run-qunit` script, so that can also be dropped.
* FIX: min_personal_message_post_length not applying to first post
Due to the way PostCreator is wired, we were not applying min_personal_message_post_length
to the first post.
This meant that admins could not configure it so PMs have different
limits.
The code was already pretending that this works, but had no reliable way
of figuring out if we were dealing with a private message
This commit adds limits to themes and theme components on the:
- file size of about.json and .discourse-compatibility
- file size of theme assets
- number of files in a theme
This extends search so it can have consumers that:
1. Can split off "term" from various advanced filters and orders
2. Can build a relation of either order or filter
It also moves a lot of stuff around in the search class for clarity.
Two new APIs are exposed:
`.apply_filter` to apply all the special filters to a posts/topics relation
`.apply_order` to force a particular order (eg: order:latest)
This can then be used by semantic search in Discourse AI
Why this change?
Currently, we do not have an easy way to test themes and theme components
using Rails system tests. While we support QUnit acceptance tests for
themes and theme components, QUnit acceptance tests stubs out the server
and setting up the fixtures for server responses is difficult and can lead to a
frustrating experience. System tests on the other hand allow authors to
set up the test fixtures using our fabricator system which is much
easier to use.
What does this change do?
In order for us to allow authors to run system tests with their themes
installed, we are adding a `upload_theme` helper that is made available
when writing system tests. The `upload_theme` helper requires a single
`directory` parameter where `directory` is the directory of the theme
locally and returns a `Theme` record.
This comment isn't necessarily on a line by itself, so we need to remove the `^` from the regex. This will fix `EMBER_ENV=development bin/rake assets:precompile`
Until now, we have allowed testing themes in production environments via `/theme-qunit`. This was made possible by hacking the ember-cli build so that it would create the `tests.js` bundle in production. However, this is fundamentally problematic because a number of test-specific things are still optimized out of the Ember build in production mode. It also makes asset compilation significantly slower, and makes it more difficult for us to update our build pipeline (e.g. to introduce Embroider).
This commit removes the ability to run qunit tests in production builds of the JS app when the Embdroider flag is enabled. If a production instance of Discourse exists exclusively for the development of themes (e.g. discourse.theme-creator.io) then they can add `EMBER_ENV: development` to their `app.yml` file. This will build the entire app in development mode, and has a significant performance impact. This must not be used for real production sites.
This commit also refactors many of the request specs into system specs. This means that the tests are guaranteed to have Ember assets built, and is also a better end-to-end test than simply checking for the presence of certain `<script>` tags in the HTML.
This method is used by assets:precompile to decide whether to apply `terser` to a file. Embroider chunks do not necessarily start with `chunk.`, and so they were incorrectly being re-terser'd by our assets:precompile task. This is inefficient, and also led to broken sourcemaps on some assets.
This is a follow up to 9caba30d5c
In that commit, we were migrating the database but we didn't actually
ensure that the database was created and that plugins were updated
before the databases were migrated.
## What is the context here?
The `docker.rake` Rakefile contains Rake tasks that are meant to be run
in the `discourse/discourse_test:release` Docker image. For example, we
have the `docker:test` Rake task that makes it easier to run the test
suite for a particular Discourse commit.
Why are we introducing a `docker:test:setup` Rake task?
While we have the `docker:test` Rake task, it is very limited in the
test commands that can be executed. It is very useful for automated
testing but not very useful for running tests in the development
environment. Therefore, we are introducing a `docker:test:setup` rake
task that can be used to set up the test environment for running tests.
The envisioned example usage is something like this:
```
docker run -d --name=discourse_test --entrypoint=/sbin/boot discourse/discourse_test:release
docker exec -u discourse:discourse discourse_test ruby script/docker_test.rb --no-tests
docker exec -u discourse:discourse discourse_test bundle exec rake docker:test:setup
docker exec -u discourse:discourse discourse_test bundle exec rspec <path to file>
```
Currently, if the review queue has both a flagged post and a flagged chat message, one of the two will have some of the labels of their actions replaced by those of the other. In other words, the labels are getting mixed up. For example, a flagged chat message might show up with an action labelled "Delete post".
This is happening because when using bundles, we are sending along the actions in a separate part of the response, so they can be shared by many reviewables. The bundles then index into this bag of actions by their ID, which is something generic describing the server action, e.g. "agree_and_delete".
The problem here is the same action can have different labels depending on the type of reviewable. Now that the bag of actions contains multiple actions with the same ID, which one is chosen is arbitrary. I.e. it doesn't distinguish based on the type of the reviewable.
This change adds an additional field to the actions, server_action, which now contains what used to be the ID. Meanwhile, the ID has been turned into a concatenation of the reviewable type and the server action, e.g. post-agree_and_delete.
This still provides the upside of denormalizing the actions while allowing for different reviewable types to have different labels and descriptions.
At first I thought I would prepend the reviewable type to the ID, but this doesn't work well because the ID is used on the server-side to determine which actions are possible, and these need to be shared between different reviewables. Hence the introduction of server_action, which now serves that purpose.
I also thought about changing the way that the bundle indexes into the bag of actions, but this is happening through some EmberJS mechanism, so we don't own that code.
