* DEV: Fix flaky admin badges.json api docs spec
This commit is to fix this incredibly vague error message:
```
Failure/Error: expect(valid).to eq(true)
expected: true
got: false
```
From this test:
> Assertion: badges /admin/badges.json get success response behaves like
> a JSON endpoint response body matches the documented response schema
I was finally able to repro locally using parallel tests:
```
RAILS_ENV=test bundle exec ./bin/turbo_rspec
```
I *think* the parallel tests might be swallowing the `puts` output, but
when I also specified the individual spec file
```
RAILS_ENV=test bundle exec ./bin/turbo_rspec spec/requests/api/badges_spec.rb
```
It revealed the issue:
```
VALIDATION DETAILS: {"missing_keys"=>["i18n_name"]}
```
``` ruby
...
def include_i18n_name?
object.system?
end
```
Looks like if the "system" user isn't being used the `i18n_name` won't
be returned in the json response so we shouldn't mark it as a required
attribute.
* Switch to using fab!
When using `let(:badge)` to fabricate a test badge it wouldn't be
returned from the controller, but switching to using `fab!` allows it to
be returned in the json data giving us a non-system badge to test
against.
When calling the API to delete a user:
```
curl -X DELETE "https://discourse.example.com/admin/users/159.json" \
-H "Content-Type: multipart/form-data;" \
-H "Api-Key: ***" \
-H "Api-Username: ***" \
-F "delete_posts=true" \
-F "block_email=false" \
-F "block_urls=false" \
-F "block_ip=false"
```
Setting the parameters `block_email`, `block_urls` and `block_ip`explicitly to `false` did not work because the values weren't being parsed to boolean.
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
In certain situations, a logged in user can redeem an invite with an email that
either doesn't match the invite's email or does not adhere to the email domain
restriction of an invite link. The impact of this flaw is aggrevated
when the invite has been configured to add the user that accepts the
invite into restricted groups.
The category description fields as part of the rswag specs for the
site.json endpoint were flakey. Removing the `required` attribute allows
us to still document that these fields exists, but that depending on
certain site settings they may not be present in the response.
Certain rogue bots such as Yandex may send across invalid CSP reports
when CSP report collection is enabled.
This ensures that invalid reports will not cause log floods and simply
returns a 422 error.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
This reverts commit 94c3bbc2d1.
At this current point in time, we do not have enough data on whether
this centralisation is the trade-offs of coupling features into a single
channel.
Presence endpoints are often called asynchronously at the same time as other request, and never need to modify the session. Skipping ensures that an unneeded cookie rotation doesn't race against another request and cause issues.
This change brings presence in line with message-bus's behaviour.
As part of this commit, a bug where updating a tag's notification level on the server side does not update the state of the user's tag notification levels on the client side is fixed too.
The `WebhookController` inherits directly from `ActionController::Base`. Since Rails 5.2, forgery protection has been enabled by default. When we applied those new defaults in 0403a8633b, it took effect on this controller and broke integrations.
This commit explicitly disables CSRF protection on these webhook routes, and updates the specs so they'll catch this kind of regression in future.
This commit resolves a bug where users are not auto approved based on
`SiteSetting.auto_approve_email_domains` when
`SiteSetting.must_approve_users` has been enabled.
This security fix affects sites which have `SiteSetting.must_approve_users`
enabled. There are intentional and unintentional cases where invited
users can be auto approved and are deemed to have skipped the staff approval process.
Instead of trying to reason about when auto-approval should happen, we have decided that
enabling the `must_approve_users` setting going forward will just mean that all new users
must be explicitly approved by a staff user in the review queue. The only case where users are auto
approved is when the `auto_approve_email_domains` site setting is used.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
The server-side implementation had unintentionally changed to include `-{id}` at the end of the body class name. This change meant that the JS client was unaware of the class, and didn't remove it when navigating away from the category page.
This commit fixes the server-side implementation to match the client
Previously we limited Discourse Connect provider to 1 secret per domain.
This made it pretty awkward to cycle secrets in environments where config
takes time to propagate
This change allows for the same domain to have multiple secrets
Also fixes internal implementation on DiscourseConnectProvider which was
not thread safe as it leaned on class variables to ferry data around
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Co-authored-by: David Taylor <david@taylorhq.com>
Previously we hardcoded the DOWNLOAD_URL_EXPIRES_AFTER_SECONDS const
inside S3Helper to be 5 minutes (300 seconds). For various reasons,
some hosted sites may need this to be longer for other integrations.
The maximum expiry time for presigned URLs is 1 week (which is
604800 seconds), so that has been added as a validation on the
setting as well. The setting is hidden because 99% of the time
it should not be changed.
When searching for PMs or PMs in a group inbox, results in the header search were not being limited to 5 with a "More" link to the full page search. This PR fixes that.
It also simplifies the logic and updates the search API docs to include recently added `in:messages` and `group_messages:groupname` options.
This commit migrates all bookmarks to be polymorphic (using the
bookmarkable_id and bookmarkable_type) columns. It also deletes
all the old code guarded behind the use_polymorphic_bookmarks setting
and changes that setting to true for all sites and by default for
the sake of plugins.
No data is deleted in the migrations, the old post_id and for_topic
columns for bookmarks will be dropped later on.
The title had to be added both on the 404 page generated by the server
side, displayed when the user reaches a bad page directly and the 404
page rendered by Ember when a user reaches a missing topic while
navigating the forum.
The logic in 06893380 only works for `.js` files. It breaks down for `.br.js` and `.gz.js` files. This commit makes things more robust by extracting only the base_url from the service-worker JS, and taking the map filename from the original `sourceMappingURL` comment.
`script_asset_path('.../blah.js.map')` was appending `.js`, which would result in a filename like `.js.map.js`. It would also lose the `/assets` prefix, since the map files are not included in the sprockets manifest.
This commit updates the sourceMappingURL rewriting logic to calculate the service-worker's own JS url, and then append `.map`.
We have a .ics endpoint for user bookmarks, this
commit makes it so polymorphic bookmarks work on
that endpoint, using the serializer associated with
the RegisteredBookmarkable.
Currently the only way to allow tagging on pms is to use the `allow_staff_to_tag_pms` site setting. We are removing that site setting and replacing it with `pm_tags_allowed_for_groups` which will allow for non staff tagging. It will be group based permissions instead of requiring the user to be staff.
If the existing value of `allow_staff_to_tag_pms` is `true` then we include the `staff` groups as a default for `pm_tags_allowed_for_groups`.
This was causing issues on some sites, having the const, because this really is heavily
dependent on upload speed. We request 5-10 URLs at a time with this endpoint; for
a 1.5GB upload with 5mb parts this could mean 60 requests to the server to get all
the part URLs. If the user's upload speed is super fast they may request all 60
batches in a minute, if it is slow they may request 5 batches in a minute.
The other external upload endpoints are not hit as often, so they can stay as constant
values for now. This commit also increases the default to 20 requests/minute.
We have not used anything related to bookmarks for PostAction
or UserAction records since 2020, bookmarks are their own thing
now. Deleting all this is just cleaning up old cruft.
A bit of a mixed bag, this addresses several edge areas of bookmarks and makes them compatible with polymorphic bookmarks (hidden behind the `use_polymorphic_bookmarks` site setting). The main ones are:
* ExportUserArchive compatibility
* SyncTopicUserBookmarked job compatibility
* Sending different notifications for the bookmark reminders based on the bookmarkable type
* Import scripts compatibility
* BookmarkReminderNotificationHandler compatibility
This PR also refactors the `register_bookmarkable` API so it accepts a class descended from a `BaseBookmarkable` class instead. This was done because we kept having to add more and more lambdas/properties inline and it was very messy, so a factory pattern is cleaner. The classes can be tested independently as well.
Some later PRs will address some other areas like the discourse narrative bot, advanced search, reports, and the .ics endpoint for bookmarks.
`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`
This was due to a server side bug when unicode usernames have been
enabled. We were double encoding the unicode username in the URL
resulting in a invalid URL.
This will make future changes to the 'pull hotlinked images' system easier. This commit should not introduce any functional change.
For now, the old post_custom_field data is kept in the database. This will be dropped in a future commit.
This commit introduces a new site setting: `use_name_for_username_suggestions` (default true)
Admins can disable it if they want to stop using Name values when generating usernames for users. This can be useful if you want to keep real names private-by-default or, when used in conjunction with the `use_email_for_username_and_name_suggestions` setting, you would prefer to use email-based username suggestions.
* hidden siteSetting to enable experimental sidebar
* user preference to enable experimental sidebar
* `experimental_sidebar_enabled` attribute for current user
* Empty glimmer component for Sidebar
This pull request follows on from https://github.com/discourse/discourse/pull/16308. This one does the following:
* Changes `BookmarkQuery` to allow for querying more than just Post and Topic bookmarkables
* Introduces a `Bookmark.register_bookmarkable` method which requires a model, serializer, fields and preload includes for searching. These registered `Bookmarkable` types are then used when validating new bookmarks, and also when determining which serializer to use for the bookmark list. The `Post` and `Topic` bookmarkables are registered by default.
* Adds new specific types for Post and Topic bookmark serializers along with preloading of associations in `UserBookmarkList`
* Changes to the user bookmark list template to allow for more generic bookmarkable types alongside the Post and Topic ones which need to display in a particular way
All of these changes are gated behind the `use_polymorphic_bookmarks` site setting, apart from the .hbs changes where I have updated the original `UserBookmarkSerializer` with some stub methods.
Following this PR will be several plugin PRs (for assign, chat, encrypt) that will register their own bookmarkable types or otherwise alter the bookmark serializers in their own way, also gated behind `use_polymorphic_bookmarks`.
This commit also removes `BookmarkQuery.preloaded_custom_fields` and the functionality surrounding it. It was added in 0cd502a558 but only used by one plugin (discourse-assign) where it has since been removed, and is now used by no plugins. We don't need it anymore.
Previously, accessing the Rails app directly in development mode would give you assets from our 'legacy' Ember asset pipeline. The only way to run with Ember CLI assets was to run ember-cli as a proxy. This was quite limiting when working on things which are bypassed when using the ember-cli proxy (e.g. changes to `application.html.erb`). Also, since `ember-auto-import` introduced chunking, visiting `/theme-qunit` under Ember CLI was failing to include all necessary chunks.
This commit teaches Sprockets about our Ember CLI assets so that they can be used in development mode, and are automatically collected up under `/public/assets` during `assets:precompile`. As a bonus, this allows us to remove all the custom manifest modification from `assets:precompile`.
The key changes are:
- Introduce a shared `EmberCli.enabled?` helper
- When ember-cli is enabled, add ember-cli `/dist/assets` as the top-priority Rails asset directory
- Have ember-cli output a `chunks.json` manifest, and teach `preload_script` to read it and append the correct chunks to their associated `afterFile`
- Remove most custom ember-cli logic from the `assets:precompile` step. Instead, rely on Rails to take care of pulling the 'precompiled' assets into the `public/assets` directory. Move the 'renaming' logic to runtime, so it can be used in development mode as well.
- Remove fingerprinting from `ember-cli-build`, and allow Rails to take care of things
Long-term, we may want to replace Sprockets with the lighter-weight Propshaft. The changes made in this commit have been made with that long-term goal in mind.
tldr: when you visit the rails app directly, you'll now be served the current ember-cli assets. To keep these up-to-date make sure either `ember serve`, or `ember build --watch` is running. If you really want to load the old non-ember-cli assets, then you should start the server with `EMBER_CLI_PROD_ASSETS=0`. (the legacy asset pipeline will be removed very soon)
It used to show the warning that said only members of certain groups
could view the topic even if the group "everyone" was listed in
category's permission list.
Discourse has the Discourse Connect Provider protocol that makes it possible to
use a Discourse instance as an identity provider for external sites. As a
natural extension to this protocol, this PR adds a new feature that makes it
possible to use Discourse as a 2FA provider as well as an identity provider.
The rationale for this change is that it's very difficult to implement 2FA
support in a website and if you have multiple websites that need to have 2FA,
it's unrealistic to build and maintain a separate 2FA implementation for each
one. But with this change, you can piggyback on Discourse to take care of all
the 2FA details for you for as many sites as you wish.
To use Discourse as a 2FA provider, you'll need to follow this guide:
https://meta.discourse.org/t/-/32974. It walks you through what you need to
implement on your end/site and how to configure your Discourse instance. Once
you're done, there is only one additional thing you need to do which is to
include `require_2fa=true` in the payload that you send to Discourse.
When Discourse sees `require_2fa=true`, it'll prompt the user to confirm their
2FA using whatever methods they've enabled (TOTP or security keys), and once
they confirm they'll be redirected back to the return URL you've configured and
the payload will contain `confirmed_2fa=true`. If the user has no 2FA methods
enabled however, the payload will not contain `confirmed_2fa`, but it will
contain `no_2fa_methods=true`.
You'll need to be careful to re-run all the security checks and ensure the user
can still access the resource on your site after they return from Discourse.
This is very important because there's nothing that guarantees the user that
will come back from Discourse after they confirm 2FA is the same user that
you've redirected to Discourse.
Internal ticket: t62183.
* FEATURE: Let sites add a sitemap.xml file.
This PR adds the same features discourse-sitemap provides to core. Sitemaps are only added to the robots.txt file if the `enable_sitemap` setting is enabled and `login_required` disabled.
After merging discourse/discourse-sitemap#34, this change will take priority over the sitemap plugin because it will disable itself. We're also using the same sitemaps table, so our migration won't try to create it
again using `if_not_exists: true`.
After this commit, category group permissions can only be seen by users
that are allowed to manage a category. In the past, we inadvertently
included a category's group permissions settings in `CategoriesController#show`
and `CategoriesController#find_by_slug` endpoints for normal users when
those settings are only a concern to users that can manage a category.
There are still some, but those are in actual code that's used outside core, so the change there would need to go through the deprecation cycle. That's a task for another day.
Previously we only supported a single 'required tag group' for a category. This commit allows admins to specify multiple required tag groups, each with their own minimum tag count.
A new category_required_tag_groups database table replaces the existing columns on the categories table. Data is automatically migrated.
This did not work properly everytime because the destination URL was
saved in a cookie and that can be lost for various reasons. This commit
redirects the user to invited topic if it exists.
Previous to this change if any of the assets were not allowed extensions
they would simply be silently ignored, this could lead to broken themes
that are very hard to debug
This commit introduces a new use_polymorphic_bookmarks site setting
that is default false and hidden, that will be used to help continuous
development of polymorphic bookmarks. This setting **should not** be
enabled anywhere in production yet, it is purely for local development.
This commit uses the setting to enable create/update/delete actions
for polymorphic bookmarks on the server and client side. The bookmark
interactions on topics/posts are all usable. Listing, searching,
sending bookmark reminders, and other edge cases will be handled
in subsequent PRs.
Comprehensive UI tests will be added in the final PR -- we already
have them for regular bookmarks, so it will just be a matter of
changing them to be for polymorphic bookmarks.
Since we give a 200 response for login errors, we should be checking
whether the error key exists in each case or not.
Some tests were broken, because they weren't checking.
* FIX: Redirect if Discourse-Xhr-Redirect is present
`handleRedirect` was passed an wrong argument type (a string) instead of
a jqXHR object and missed the fields checked in condition, thus always
evaluating to `false`.
* FIX: Add `errors` field if group update confirmation
An explicit confirmation about the effect of the group update is
required if the default notification level changes. Previously, if the
confirmation was missing the API endpoint failed silently returning
a 200 response code and a `user_count` field. This change ensures that
a proper error code is returned (422), a descriptive error message and
the additional information in the `user_count` field.
This commit also refactors the API endpoint to use the
`Discourse-Xhr-Redirect` header to redirect the user if the group is
no longer visible.
Previously we would issue a 403 for all invalid routes under `/tags/c/...`, which is not semantically correct. In some cases, these 403'd routes would then be handled successfully in the Ember app, leading to some very confusing behavior.
* DEV: Allow params to be passed on topic redirects
There are several places where we redirect a url to a standard topic url
like `/t/:slug/:topic_id` but we weren't always passing query parameters
to the new url.
This change allows a few more query params to be included on the
redirect. The new params that are permitted are page, print, and
filter_top_level_replies. Any new params will need to be specified.
This also prevents the odd trailing empty page param that would
sometimes appear on a redirect. `/t/:slug/:id.json?page=`
* rubocop: fix missing space after comma
* fix another page= reference
* FEATURE: use canonical links in posts.rss feed
Previously we used non canonical links in posts.rss
These links get crawled frequently by crawlers when discovering new
content forcing crawlers to hop to non canonical pages just to end up
visiting canonical pages
This uses up expensive crawl time and adds load on Discourse sites
Old links were of the form:
`https://DOMAIN/t/SLUG/43/21`
New links are of the form
`https://DOMAIN/t/SLUG/43?page=2#post_21`
This also adds a post_id identified element to crawler view that was
missing.
Note, to avoid very expensive N+1 queries required to figure out the
page a post is on during rss generation, we cache that information.
There is a smart "cache breaker" which ensures worst case scenario is
a "page drift" - meaning we would publicize a post is on page 11 when
it is actually on page 10 due to post deletions. Cache holds for up to
12 hours.
Change only impacts public post RSS feeds (`/posts.rss`)
This update topic route has never worked. Better late than never. I am
in favor of using non-slug urls when using the api so I do think we
should fix this route.
Just thought I would update the `:id` param to `:topic_id` here in the
routes file instead of updating the controller to handle both params.
Added a spec to test this route.
Also added the same constraint we have on other topic routes to ensure
we only pass in an ID that is a digit.
Previously, if an admin user tried to add/remove
users to another user's ignored list, it would
be added to their own ignore list because the
controller used current_user. Now for admins only
a source_user_id parameter can be passed through,
which will be used to ignore the target user for
that source user.
When creating files with create-multipart, if the file
size was somehow zero we were showing a very unhelpful
error message to the user. Now we show a nicer message,
and proactively don't call the API if we know the file
size is 0 bytes in JS, along with extra console logging
to help with debugging.
This change adds support for the categories endpoint to have an api
scope. Only adds GET scope for listing categories and for fetching a
single category.
See: https://meta.discourse.org/t/218080/4
This PR adds an extra description to the 2FA page when granting a user admin access. It also introduces a general system for adding customized descriptions that can be used by future actions.
(Follow-up to dd6ec65061)
Discourse users and associated accounts are created or updated when a
user logins or connects the account using their account preferences.
This new API can be used to create associated accounts and users too,
if necessary.
Our @mention user search prioritized users based on prefix matches.
So if searching for `sa` we will display `sam`, `asam` in that order
Previously, we did not prioritize group matches based on prefix. This change ensures better parity.
Implementation notes:
1. User search only prioritizes based on username prefix, not name prefix. TBD if we want to change that.
2. @mention on client side will show 0 group matches if we fill up all the spots with user matches. TBD if we want to unconditionally show the first / second group match.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
This feature was rarely used, could be used for spamming users and was
impossible to add a context to why the user was notified of a topic. A
simple private messages that includes the link and personalized message
can be used instead.
* DEV: Show only top level replies
Adds a new query param to the topic view so that we can filter out posts
that aren't top level replies. If a post is a reply to another post
instead of the original topic post we should not include it in the
response if the `filter_top_level_replies` query param is present.
* add rspec test
It's very easy to forget to add `require 'rails_helper'` at the top of every core/plugin spec file, and omissions can cause some very confusing/sporadic errors.
By setting this flag in `.rspec`, we can remove the need for `require 'rails_helper'` entirely.
This allows text editors to use correct syntax coloring for the heredoc sections.
Heredoc tag names we use:
languages: SQL, JS, RUBY, LUA, HTML, CSS, SCSS, SH, HBS, XML, YAML/YML, MF, ICS
other: MD, TEXT/TXT, RAW, EMAIL
* FEATURE: upload an avatar option for uploading avatars with selectable avatars
Allow staff or users at or above a trust level to upload avatars even when the site
has selectable avatars enabled.
Everyone can still pick from the list of avatars. The option to upload is shown
below the selectable avatar list.
refactored boolean site setting into an enum with the following values:
disabled: No selectable avatars enabled (default)
everyone: Show selectable avatars, and allow everyone to upload custom avatars
tl1: Show selectable avatars, but require tl1+ and staff to upload custom avatars
tl2: Show selectable avatars, but require tl2+ and staff to upload custom avatars
tl3: Show selectable avatars, but require tl3+ and staff to upload custom avatars
tl4: Show selectable avatars, but require tl4 and staff to upload custom avatars
staff: Show selectable avatars, but only allow staff to upload custom avatars
no_one: Show selectable avatars. No users can upload custom avatars
Co-authored-by: Régis Hanol <regis@hanol.fr>
Currently, providing things like `filter[%24acunetix]=1` to
`UserActionsController#index` will throw an exception because instead of
getting a string as expected, we get a hash instead.
This patch simply uses `#permit` from strong parameters properly: first
we apply it on the whole parameters, this way it filters the keys we’re
interested in. By doing this, if the value is a hash for example, the
whole key/value pair will be ignored completely.
aa1442fdc3 split theme stylesheets so that every component gets its own stylesheet. Therefore, there is now no need for parent themes to collate the settings/variables of its children during scss compilation.
Technically this is a breaking change for any themes which depend on the settings/variables of their child components. That was never a supported/recommended arrangement, so we don't expect this to cause issues.
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.
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.
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.
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.
* DEV: Document external topic id endpoints
This commit documents the existing Create Topic endpoint with the
`external_id` param and documents the new get topic by external id
endpoint.
It also refactors the existing topic show endpoint to use the new format
where we load the expected json schema response from a file.
See: 71f7f7ed49
* clean up unused test variables
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.
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.
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.
* 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>
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`.
- 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
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
* 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
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.
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.
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
* 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
This reverts commit 2c7906999a.
The changes break some things in local development (putting JS files
into minified files, not allowing debugger, and others)
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.
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.
Also:
* Remove an unused method (#fill_email)
* Replace a method that was used just once (#generate_username) with `SecureRandom.alphanumeric`
* Remove an obsolete dev puma `tmp/restart` file logic
Adding a spec for documenting the delete post API endpoint for our api
docs. As part of this added detailed info for the `force_destroy`
parameter for permanently deleting a post.
Tests fail in Ruby 3.0 and later due to separation of positional and
keyword arguments. RSpec treats the hash at the end of include_examples
as keyword arguments when it should be passed as a positional argument.
* File.exists? is deprecated and removed in Ruby 3.2 in favor of
File.exist?
* Dir.exists? is deprecated and removed in Ruby 3.2 in favor of
Dir.exist?
This allows authenticators to instruct the Auth::Result to override attributes without using the general site settings. This provides an easy migration path for auth plugins which offer their own "overrides email", "overrides username" or "overrides name" settings. With this new api, they can set `overrides_*` on the result object, and the attribute will be overriden regardless of the general site setting.
ManagedAuthenticator is updated to use this new API. Plugins which consume ManagedAuthenticator will instantly take advantage of this change.
This commit adds API documentation for the new upload
endpoints related to direct + multipart external uploads.
Also included is a rake task which watches the files in
the spec/requests/api directory and calls a script file
(spec/regenerate_swagger_docs) whenever one changes. This
script runs rake rswag:specs:swaggerize and then copies
the openapi.yml file over to the discourse_api_docs repo
directory, and hits a script there to convert the YML to
JSON so the API docs are refreshed while the server is
still running. This makes the loop of making a doc change
and seeing it in the local server much faster.
The rake task is rake autospec:swagger
* FEATURE: hide_email_address_taken forces use of email in forgot password form
This strengthens this site setting which is meant to be used to harden sites
that are experiencing abuse on forgot password routes.
Previously we would only deny letting people know if forgot password worked on not
New change also bans usage of username for forgot password when enabled
Discourse sent only translation overrides for the current language to the client instead of sending overrides from fallback locales as well. This especially impacted en_GB -> en since most overrides would be done in English instead of English (UK).
This also adds lots of tests for previously untested code.
There's a small caveat: The client currently doesn't handle fallback locales for MessageFormat strings. That is why overrides for those strings always have a higher priority than regular translations. So, as an example, the lookup order for MessageFormat strings in German is:
1. override for de
2. override for en
3. value from de
4. value from en
Some time ago, we made this fix to external authentication – https://github.com/discourse/discourse/pull/13706. We didn't address Discourse Connect (https://meta.discourse.org/t/discourseconnect-official-single-sign-on-for-discourse-sso/13045) at that moment, so I wanted to fix it for Discourse Connect as well.
Turned out though that Discourse Connect doesn't contain this problem and already handles staged users correctly. This PR adds tests that confirm it. Also, I've extracted two functions in Discourse Connect implementation along the way and decided to merge this refactoring too (the refactoring is supported with tests).
A plugin API that allows customizing existing topic-backed static pages, like:
faq, tos, privacy (see: StaticController) The block passed to this
method has to return a SiteSetting name that contains a topic id.
```
add_topic_static_page("faq") do |controller|
current_user&.locale == "pl" ? "polish_faq_topic_id" : "faq_topic_id"
end
```
You can also add new pages in a plugin, but remember to add a route,
for example:
```
get "contact" => "static#show", id: "contact"
```
We can fake redis transactions so that `fab!` works for redis and PG
data, but it's too slow to be used indiscriminately. Instead, you can
opt into it with the `use_redis_snapshotting` helper.
Insofar as snapshotting allows us to `fab!` more things, it provides a
speedup.
This commit introduces a new site setting "google_oauth2_hd_groups". If enabled, group information will be fetched from Google during authentication, and stored in the Discourse database. These 'associated groups' can be connected to a Discourse group via the "Membership" tab of the group preferences UI.
The majority of the implementation is generic, so we will be able to add support to more authentication methods in the near future.
https://meta.discourse.org/t/managing-group-membership-via-authentication/175950
We don't need it anymore. Actually, I removed using of it on the client side a long time ago, when I was working on improving blank page syndrome on user activity pages (see https://github.com/discourse/discourse/pull/14311).
This PR also removes some old resource strings that we don't use anymore. We have new strings for blank pages.
Documenting the `/u/:username:/emails.json` endpoint.
Also removing some email fields from user api responses because they
aren't actually included in the response unless you are querying
yourself.
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.
Related to: 20f736aa11.
`auto_update` is true by default at the database level, but it doesn't make sense for `auto_update` to be true on themes that are not imported from a Git repository.
Under some conditions, these varied responses could lead to cache poisoning, hence the 'security' label.
Previously the Rails application would serve JSON data in place of HTML whenever Ember CLI requested an `application.html.erb`-rendered page. This commit removes that logic, and instead parses the HTML out of the standard response. This means that Rails doesn't need to customize its response for Ember CLI.
Recently, the wrong new behavior appeared – we started to suggest to invited users usernames like "user1".
To reproduce:
1. Create an invitation with default settings, do not restrict it to email
2. Copy an invitation link and follow it in incognito mode
See username already filled, with eg “user1”. See screenshot. Should be empty.
This bug was very likely introduced by my recent changes to UserNameSuggester.
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.
Currently when a user creates posts that are moderated (for whatever
reason), a popup is displayed saying the post needs approval and the
total number of the user’s pending posts. But then this piece of
information is kind of lost and there is nowhere for the user to know
what are their pending posts or how many there are.
This patch solves this issue by adding a new “Pending” section to the
user’s activity page when there are some pending posts to display. When
there are none, then the “Pending” section isn’t displayed at all.
* 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>
This commit adds token_hash and scopes columns to email_tokens table.
token_hash is a replacement for the token column to avoid storing email
tokens in plaintext as it can pose a security risk. The new scope column
ensures that email tokens cannot be used to perform a different action
than the one intended.
To sum up, this commit:
* Adds token_hash and scope to email_tokens
* Reuses code that schedules critical_user_email
* Refactors EmailToken.confirm and EmailToken.atomic_confirm methods
* Periodically cleans old, unconfirmed or expired email tokens
This commit refactors the direct external upload routes (get presigned
put, complete external, create/abort/complete multipart) into a
helper which is then included in both BackupController and the
UploadController. This is done so UploadController doesn't need
strange backup logic added to it, and so each controller implementing
this helper can do their own validation/error handling nicely.
This is a follow up to e4350bb966
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.
Uppy adds the file name as the "name" parameter in the
payload by default, which means that for things like the
emoji uploader which have a name param used by the controller,
that param will be passed as the file name. We already use
the existing file name if the name param is null, so this
commit just does further cleanup of the name param, removing
the extension if it is a filename so we don't end up with
emoji names like blah_png.
Uppy and Resumable slice up their chunks differently, which causes a difference
in this algorithm. Let's take a 131.6MB file (137951695 bytes) with a 5MB (5242880 bytes)
chunk size. For resumable, there are 26 chunks, and uppy there are 27. This is
controlled by forceChunkSize in resumable which is false by default. The final
chunk size is 6879695 (chunk size + remainder) whereas in uppy it is 1636815 (just remainder).
This means that the current condition of uploaded_file_size + current_chunk_size >= total_size
is hit twice by uppy, because it uses a more correct number of chunks. This
can be solved for both uppy and resumable by checking the _previous_ chunk
number * chunk_size as the uploaded_file_size.
An example of what is happening before that change, using the current
chunk number to calculate uploaded_file_size.
chunk 26: resumable: uploaded_file_size (26 * 5242880) + current_chunk_size (6879695) = 143194575 >= total_size (137951695) ? YES
chunk 26: uppy: uploaded_file_size (26 * 5242880) + current_chunk_size (5242880) = 141557760 >= total_size (137951695) ? YES
chunk 27: uppy: uploaded_file_size (27 * 5242880) + current_chunk_size (1636815) = 143194575 >= total_size (137951695) ? YES
An example of what this looks like after the change, using the previous
chunk number to calculate uploaded_file_size:
chunk 26: resumable: uploaded_file_size (25 * 5242880) + current_chunk_size (6879695) = 137951695 >= total_size (137951695) ? YES
chunk 26: uppy: uploaded_file_size (25 * 5242880) + current_chunk_size (5242880) = 136314880 >= total_size (137951695) ? NO
chunk 27: uppy: uploaded_file_size (26 * 5242880) + current_chunk_size (1636815) = 137951695 >= total_size (137951695) ? YES
This PR introduces a new `enable_experimental_backup_uploads` site setting (default false and hidden), which when enabled alongside `enable_direct_s3_uploads` will allow for direct S3 multipart uploads of backup .tar.gz files.
To make multipart external uploads work with both the S3BackupStore and the S3Store, I've had to move several methods out of S3Store and into S3Helper, including:
* presigned_url
* create_multipart
* abort_multipart
* complete_multipart
* presign_multipart_part
* list_multipart_parts
Then, S3Store and S3BackupStore either delegate directly to S3Helper or have their own special methods to call S3Helper for these methods. FileStore.temporary_upload_path has also removed its dependence on upload_path, and can now be used interchangeably between the stores. A similar change was made in the frontend as well, moving the multipart related JS code out of ComposerUppyUpload and into a mixin of its own, so it can also be used by UppyUploadMixin.
Some changes to ExternalUploadManager had to be made here as well. The backup direct uploads do not need an Upload record made for them in the database, so they can be moved to their final S3 resting place when completing the multipart upload.
This changeset is not perfect; it introduces some special cases in UploadController to handle backups that was previously in BackupController, because UploadController is where the multipart routes are located. A subsequent pull request will pull these routes into a module or some other sharing pattern, along with hooks, so the backup controller and the upload controller (and any future controllers that may need them) can include these routes in a nicer way.
We are only using list_multipart_parts right now in the
uploads controller for multipart uploads to check if the
upload exists; thus we don't need up to 1000 parts.
Also adding a note for future explorers that list_multipart_parts
only gets 1000 parts max, and adding params for max parts
and starting parts.
Support for invites alongside DiscourseConnect was added in 355d51af. This commit fixes the guardian method so that the bulk invite button functionality also works.
* FIX: allowed_theme_ids should not be persisted in GlobalSettings
It was observed that the memoized value of `GlobalSetting.allowed_theme_ids` would be persisted across requests, which could lead to unpredictable/undesired behaviours in a multisite environment.
This change moves that logic out of GlobalSettings so that the returned theme IDs are correct for the current site.
Uses get_set_cache, which ultimately uses DistributedCache, which will take care of multisite issues for us.
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)
`/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.
- 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
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
This commit resolves refactors can_invite_to? to use
can_invite_to_forum? for checking the site-wide permissions and then
perform topic specific checkups.
Similarly, can_invite_to? is always used with a topic object and this is
now enforced.
There was another problem before when `must_approve_users` site setting
was not checked when inviting users to forum, but was checked when
inviting to a topic.
Another minor security issue was that group owners could invite to
group topics even if they did not have the minimum trust level to do
it.
It was possible to see notifications of other users using routes:
- notifications/responses
- notifications/likes-received
- notifications/mentions
- notifications/edits
We weren't showing anything private (like notifications about private messages), only things that're publicly available in other places. But anyway, it feels strange that it's possible to look at notifications of someone else. Additionally, there is a risk that we can unintentionally leak something on these pages in the future.
This commit restricts these routes.
The file size error messages for max_image_size_kb and
max_attachment_size_kb are shown to the user in the KB
format, regardless of how large the limit is. Since we
are going to support uploading much larger files soon,
this KB-based limit soon becomes unfriendly to the end
user.
For example, if the max attachment size is set to 512000
KB, this is what the user sees:
> Sorry, the file you are trying to upload is too big (maximum
size is 512000KB)
This makes the user do math. In almost all file explorers that
a regular user would be familiar width, the file size is shown
in a format based on the maximum increment (e.g. KB, MB, GB).
This commit changes the behaviour to output a humanized file size
instead of the raw KB. For the above example, it would now say:
> Sorry, the file you are trying to upload is too big (maximum
size is 512 MB)
This humanization also handles decimals, e.g. 1536KB = 1.5 MB
Allows creating a bookmark with the `for_topic` flag introduced in d1d2298a4c set to true. This happens when clicking on the Bookmark button in the topic footer when no other posts are bookmarked. In a later PR, when clicking on these topic-level bookmarks the user will be taken to the last unread post in the topic, not the OP. Only the OP can have a topic level bookmark, and users can also make a post-level bookmark on the OP of the topic.
I had to do some pretty heavy refactors because most of the bookmark code in the JS topics controller was centred around instances of Post JS models, but the topic level bookmark is not centred around a post. Some refactors were just for readability as well.
Also removes some missed reminderType code from the purge in 41e19adb0d
Due to the way that rswag expands shared components we were getting this
warning when linting our api docs:
```
Component: "user_response" is never used.
```
This change refactors the `api/users_spec.rb` file so that it uses the
new way of doing things with a separate `user_get_response.json` schema
file rather then the old way of loading a shared response inside of the
swagger_helper.rb file.
The color_scheme_id needs to be an integer not a string.
This is one of the failing tests that showed this error:
https://github.com/discourse/discourse/runs/3598414971
It showed this error
`POSSIBLE ISSUE W/: /user_themes/0/color_scheme_id`
And this is part of the site.json response:
```
...
"user_themes"=>[{"theme_id"=>149, "name"=>"Cool theme 111", "default"=>false, "color_scheme_id"=>37}]
...
```
We don't actually use the reminder_type for bookmarks anywhere;
we are just storing it. It has no bearing on the UI. It used
to be relevant with the at_desktop bookmark reminders (see
fa572d3a7a)
This commit marks the column as readonly, ignores it, and removes
the index, and it will be dropped in a later PR. Some plugins
are relying on reminder_type partially so some stubs have been
left in place to avoid errors.
We don't need no stinkin' denormalization! This commit ignores
the topic_id column on bookmarks, to be deleted at a later date.
We don't really need this column and it's better to rely on the
post.topic_id as the canonical topic_id for bookmarks, then we
don't need to remember to update both columns if the bookmarked
post moves to another topic.
Administrators can use second factor to confirm granting admin access
without using email. The old method of confirmation via email is still
used as a fallback when second factor is unavailable.
The previous excerpt was a simple truncated raw message. Starting with
this commit, the raw content of the draft is cooked and an excerpt is
extracted from it. The logic for extracting the excerpt mimics the the
`ExcerptParser` class, but does not implement all functionality, being
a much simpler implementation.
The two draft controllers have been merged into one and the /draft.json
route has been changed to /drafts.json to be consistent with the other
route names.
When a user archives a personal message, they are redirected back to the
inbox and will refresh the list of the topics for the given filter.
Publishing an event to the user results in an incorrect incoming message
because the list of topics has already been refreshed.
This does mean that if a user has two tabs opened, the non-active tab
will not receive the incoming message but at this point we do not think
the technical trade-offs are worth it to support this feature. We
basically have to somehow exclude a client from an incoming message
which is not easy to do.
Follow-up to fc1fd1b416
At this point in time, we do not think supporting unread and new when an
admin is looking at another user's messages is worth supporting.
Follow-up to fc1fd1b416
From the openapi spec:
https://spec.openapis.org/oas/latest.html#fixed-fields-7
each endpoint needs to have an `operationId`:
> Unique string used to identify the operation. The id MUST be unique
> among all operations described in the API. The operationId value is
> case-sensitive. Tools and libraries MAY use the operationId to uniquely
> identify an operation, therefore, it is RECOMMENDED to follow common
> programming naming conventions.
Running the linter on our openapi.json file with this command:
`npx @redocly/openapi-cli lint openapi.json`
produced the following warning on all of our endpoints:
> Operation object should contain `operationId` field
This commit resolves these warnings by adding an operationId field to
each endpoint.
Previously, a group's `default_notification_level` change will only affect the users added after it.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
PresenceChannel aims to be a generic system for allow the server, and end-users, to track the number and identity of users performing a specific task on the site. For example, it might be used to track who is currently 'replying' to a specific topic, editing a specific wiki post, etc.
A few key pieces of information about the system:
- PresenceChannels are identified by a name of the format `/prefix/blah`, where `prefix` has been configured by some core/plugin implementation, and `blah` can be any string the implementation wants to use.
- Presence is a boolean thing - each user is either present, or not present. If a user has multiple clients 'present' in a channel, they will be deduplicated so that the user is only counted once
- Developers can configure the existence and configuration of channels 'just in time' using a callback. The result of this is cached for 2 minutes.
- Configuration of a channel can specify permissions in a similar way to MessageBus (public boolean, a list of allowed_user_ids, and a list of allowed_group_ids). A channel can also be placed in 'count_only' mode, where the identity of present users is not revealed to end-users.
- The backend implementation uses redis lua scripts, and is designed to scale well. In the future, hard limits may be introduced on the maximum number of users that can be present in a channel.
- Clients can enter/leave at will. If a client has not marked itself 'present' in the last 60 seconds, they will automatically 'leave' the channel. The JS implementation takes care of this regular check-in.
- On the client-side, PresenceChannel instances can be fetched from the `presence` ember service. Each PresenceChannel can be used entered/left/subscribed/unsubscribed, and the service will automatically deduplicate information before interacting with the server.
- When a client joins a PresenceChannel, the JS implementation will automatically make a GET request for the current channel state. To avoid this, the channel state can be serialized into one of your existing endpoints, and then passed to the `subscribe` method on the channel.
- The PresenceChannel JS object is an ember object. The `users` and `count` property can be used directly in ember templates, and in computed properties.
- It is important to make sure that you `unsubscribe()` and `leave()` any PresenceChannel objects after use
An example implementation may look something like this. On the server:
```ruby
register_presence_channel_prefix("site") do |channel|
next nil unless channel == "/site/online"
PresenceChannel::Config.new(public: true)
end
```
And on the client, a component could be implemented like this:
```javascript
import Component from "@ember/component";
import { inject as service } from "@ember/service";
export default Component.extend({
presence: service(),
init() {
this._super(...arguments);
this.set("presenceChannel", this.presence.getChannel("/site/online"));
},
didInsertElement() {
this.presenceChannel.enter();
this.presenceChannel.subscribe();
},
willDestroyElement() {
this.presenceChannel.leave();
this.presenceChannel.unsubscribe();
},
});
```
With this template:
```handlebars
Online: {{presenceChannel.count}}
<ul>
{{#each presenceChannel.users as |user|}}
<li>{{avatar user imageSize="tiny"}} {{user.username}}</li>
{{/each}}
</ul>
```
The generate_presigned_put endpoint for direct external uploads
(such as the one for the uppy-image-uploader) records allowed
S3 metadata values on the uploaded object. We use this to store
the sha1-checksum generated by the UppyChecksum plugin, for later
comparison in ExternalUploadManager.
However, we were not doing this for the create_multipart endpoint,
so the checksum was never captured and compared correctly.
Also includes a fix to make sure UppyChecksum is the last preprocessor to run.
It is important that the UppyChecksum preprocessor is the last one to
be added; the preprocessors are run in order and since other preprocessors
may modify the file (e.g. the UppyMediaOptimization one), we need to
checksum once we are sure the file data has "settled".
Users can invite people to topic and they will be automatically
redirected to the topic when logging in after signing up. This commit
ensures a "invited_to_topic" notification is created when the invite is
redeemed.
The same notification is used for the "Notify" sharing method that is
found in share topic modal.
Since ad3ec5809f when a user chooses
the Dismiss New... option in the New topic list, we send a request
to topics/reset-new.json with ?tracked=false as the only parameter.
This then uses Topic as the scope for topics to dismiss, with no
other limitations. When we do topic_scope.pluck(:id), it gets the
ID of every single topic in the database (that is not deleted) to
pass to TopicsBulkAction, causing a huge query with severe performance
issues.
This commit changes the default scope to use
`TopicQuery.new(current_user).new_results(limit: false)`
which should only use the topics in the user's New list, which
will be a much smaller list, depending on the user's "new_topic_duration_minutes"
setting.
There are certain design decisions that were made in this commit.
Private messages implements its own version of topic tracking state because there are significant differences between regular and private_message topics. Regular topics have to track categories and tags while private messages do not. It is much easier to design the new topic tracking state if we maintain two different classes, instead of trying to mash this two worlds together.
One MessageBus channel per user and one MessageBus channel per group. This allows each user and each group to have their own channel backlog instead of having one global channel which requires the client to filter away unrelated messages.
This pull request introduces the endpoints required, and the JavaScript functionality in the `ComposerUppyUpload` mixin, for direct S3 multipart uploads. There are four new endpoints in the uploads controller:
* `create-multipart.json` - Creates the multipart upload in S3 along with an `ExternalUploadStub` record, storing information about the file in the same way as `generate-presigned-put.json` does for regular direct S3 uploads
* `batch-presign-multipart-parts.json` - Takes a list of part numbers and the unique identifier for an `ExternalUploadStub` record, and generates the presigned URLs for those parts if the multipart upload still exists and if the user has permission to access that upload
* `complete-multipart.json` - Completes the multipart upload in S3. Needs the full list of part numbers and their associated ETags which are returned when the part is uploaded to the presigned URL above. Only works if the user has permission to access the associated `ExternalUploadStub` record and the multipart upload still exists.
After we confirm the upload is complete in S3, we go through the regular `UploadCreator` flow, the same as `complete-external-upload.json`, and promote the temporary upload S3 into a full `Upload` record, moving it to its final destination.
* `abort-multipart.json` - Aborts the multipart upload on S3 and destroys the `ExternalUploadStub` record if the user has permission to access that upload.
Also added are a few new columns to `ExternalUploadStub`:
* multipart - Whether or not this is a multipart upload
* external_upload_identifier - The "upload ID" for an S3 multipart upload
* filesize - The size of the file when the `create-multipart.json` or `generate-presigned-put.json` is called. This is used for validation.
When the user completes a direct S3 upload, either regular or multipart, we take the `filesize` that was captured when the `ExternalUploadStub` was first created and compare it with the final `Content-Length` size of the file where it is stored in S3. Then, if the two do not match, we throw an error, delete the file on S3, and ban the user from uploading files for N (default 5) minutes. This would only happen if the user uploads a different file than what they first specified, or in the case of multipart uploads uploaded larger chunks than needed. This is done to prevent abuse of S3 storage by bad actors.
Also included in this PR is an update to vendor/uppy.js. This has been built locally from the latest uppy source at d613b849a6. This must be done so that I can get my multipart upload changes into Discourse. When the Uppy team cuts a proper release, we can bump the package.json versions instead.
* FIX: Revoking admin or moderator status doesn't require refresh to delete/anonymize/merge user
On the /admin/users/<id>/<username> page, there are action buttons that are either visible or hidden depending on a few fields from the AdminDetailsSerializer: `can_be_deleted`, `can_be_anonymized`, `can_be_merged`, `can_delete_all_posts`.
These fields are updated when granting/revoking admin or moderator status. However, those updates were not being reflected on the page. E.g. if a user is granted moderation privileges, the 'anonymize user' and 'merge' buttons still appear on the page, which is inconsistent with the backend state of the user. It requires refreshing the page to update the state.
This commit fixes that issue, by syncing the client model state with the server state when handling a successful response from the server. Now, when revoking privileges, the buttons automatically appear without refreshing the page. Similarly, when granting moderator privileges, the buttons automatically disappear without refreshing the page.
* Add detailed user response to spec for changed routes.
Add tests to verify that the revoke_moderation, grant_moderation, and revoke_admin routes return a response formatted according to the AdminDetailedUserSerializer.
This is useful in the DiscourseHub mobile app, currently the app queries
the `about.json` endpoint, which can raise a CORS issue in some cases,
for example when the site only accepts logins from an external provider.
`nullable` is no longer a valid type, and types also can't be an empty
string, so just bringing a number of issues with types in compliance
with the openapi spec.
When a user signs up via an external auth method, a new link is added to the signup modal which allows them to connect an existing Discourse account. This will only happen if:
- There is at least 1 other auth method available
and
- The current auth method permits users to disconnect/reconnect their accounts themselves
This handles a few edge cases which are extremely rare (due to the UI layout), but still technically possible:
- Ensure users are authenticated before attempting association.
- Add a message and logic for when a user already has an association for a given auth provider.
This reverts a part of changes introduced by https://github.com/discourse/discourse/pull/13947
In that PR I:
1. Disallowed topic feature links for TL-0 users
2. Additionally, disallowed just putting any URL in topic titles for TL-0 users
Actually, we don't need the second part. It introduced unnecessary complexity for no good reason. In fact, it tries to do the job that anti-spam plugins (like Akismet plugin) should be doing.
This PR reverts this second change.
This disallows putting URLs in topic titles for TL0 users, which means that:
If a TL-0 user puts a link into the title, a topic featured link won't be generated (as if it was disabled in the site settings)
Server methods for creating and updating topics will be refusing featured links when they are called by TL-0 users
TL-0 users won't be able to put any link into the topic title. For example, the title "Hey, take a look at https://my-site.com" will be rejected.
Also, it improves a bit server behavior when creating or updating feature links on topics in the categories with disabled featured links. Before the server just silently ignored a featured link field that was passed to him, now it will be returning 422 response.
This adds a few different things to allow for direct S3 uploads using uppy. **These changes are still not the default.** There are hidden `enable_experimental_image_uploader` and `enable_direct_s3_uploads` settings that must be turned on for any of this code to be used, and even if they are turned on only the User Card Background for the user profile actually uses uppy-image-uploader.
A new `ExternalUploadStub` model and database table is introduced in this pull request. This is used to keep track of uploads that are uploaded to a temporary location in S3 with the direct to S3 code, and they are eventually deleted a) when the direct upload is completed and b) after a certain time period of not being used.
### Starting a direct S3 upload
When an S3 direct upload is initiated with uppy, we first request a presigned PUT URL from the new `generate-presigned-put` endpoint in `UploadsController`. This generates an S3 key in the `temp` folder inside the correct bucket path, along with any metadata from the clientside (e.g. the SHA1 checksum described below). This will also create an `ExternalUploadStub` and store the details of the temp object key and the file being uploaded.
Once the clientside has this URL, uppy will upload the file direct to S3 using the presigned URL. Once the upload is complete we go to the next stage.
### Completing a direct S3 upload
Once the upload to S3 is done we call the new `complete-external-upload` route with the unique identifier of the `ExternalUploadStub` created earlier. Only the user who made the stub can complete the external upload. One of two paths is followed via the `ExternalUploadManager`.
1. If the object in S3 is too large (currently 100mb defined by `ExternalUploadManager::DOWNLOAD_LIMIT`) we do not download and generate the SHA1 for that file. Instead we create the `Upload` record via `UploadCreator` and simply copy it to its final destination on S3 then delete the initial temp file. Several modifications to `UploadCreator` have been made to accommodate this.
2. If the object in S3 is small enough, we download it. When the temporary S3 file is downloaded, we compare the SHA1 checksum generated by the browser with the actual SHA1 checksum of the file generated by ruby. The browser SHA1 checksum is stored on the object in S3 with metadata, and is generated via the `UppyChecksum` plugin. Keep in mind that some browsers will not generate this due to compatibility or other issues.
We then follow the normal `UploadCreator` path with one exception. To cut down on having to re-upload the file again, if there are no changes (such as resizing etc) to the file in `UploadCreator` we follow the same copy + delete temp path that we do for files that are too large.
3. Finally we return the serialized upload record back to the client
There are several errors that could happen that are handled by `UploadsController` as well.
Also in this PR is some refactoring of `displayErrorForUpload` to handle both uppy and jquery file uploader errors.
* DEV: Return 400 instead of 500 for invalid top period
This change will prevent a fatal 500 error when passing in an invalid
period param value to the `/top` route.
* Check if the method exists first
I couldn't get `ListController.respond_to?` to work, but was still able
to check if the method exists with
`ListController.action_methods.include?`. This way we can avoid relying
on the `NoMethodError` exception which may be raised during the course
of executing the method.
* Just check if the period param value is valid
* Use the new TopTopic.validate_period method
* Copy remove_member to new `leave` method
* Remove unneeded code from the leave method
* Rearrange the leave method
* Remove unneeded code from the remove_member method
* Add tests
* Implement on the client side
* Copy the add_members method to the new join method
* Remove unneeded code from the join method
* Rearrange the join method
* Remove unneeded stuff from the add_members method
* Extract add_user_to_group method
* Implement of the client side
* Tests
* Doesn't inline users.uniq
* Return promise from join.then()
* Remove unnecessary begin and end
* Revert "Return promise from join.then()"
This reverts commit bda84d8d
* Remove variable already_in_group
Will show the last 6 seen users as filtering suggestions when typing @ in quick search. (Previously the user suggestion required a character after the @.)
This also adds a default limit of 6 to the user search query, previously the backend was returning 20 results but a maximum of 6 results was being shown anyway.
When configured, all topics in the category inherits the slow mode
duration from the category's default.
Note that currently there is no way to remove the slow mode from the
topics once it has been set.
When the Forever option is selected for suspending a user, the user is suspended for 1000 years. Without customizing the site’s text, this time period is displayed to the user in the suspension email that is sent to the user, and if the user attempts to log back into the site. Telling someone that they have been suspended for 1000 years seems likely to come across as a bad attempt at humour.
This PR special case messages when a user suspended or silenced forever.
Flips content_security_policy_frame_ancestors default to enabled, and
removes HTTP_REFERER checks on embed requests, as the new referer
privacy options made the check fragile.
* No need to return anything except a status code from the server
* Switch a badge state before sending a request and then switch it back in case of an error
Currently when bulk-awarding a badge that can be granted multiple times, users in the CSV file are granted the badge once no matter how many times they're listed in the file and only if they don't have the badge already.
This PR adds a new option to the Badge Bulk Award feature so that it's possible to grant users a badge even if they already have the badge and as many times as they appear in the CSV file.
This PR adds the first use of Uppy in our codebase, hidden behind a enable_experimental_image_uploader site setting. When the setting is enabled only the user card background uploader will use the new uppy-image-uploader component added in this PR.
I've introduced an UppyUpload mixin that has feature parity with the existing Upload mixin, and improves it slightly to deal with multiple/single file distinctions and validations better. For now, this just supports the XHRUpload plugin for uppy, which keeps our existing POST to /uploads.json.
If user had a staged account and logged in using a third party service
a different username was suggested. This change will try to use the
username given by the authentication provider first, then the current
staged username and last suggest a new one.
When a staged user tried to redeem an invite, a different username was
suggested and manually typing the staged username failed because the
username was not available.
User flair was given by user's primary group. This PR separates the
two, adds a new field to the user model for flair group ID and users
can select their flair from user preferences now.
Mixing multisite and standard specs can lead to issues (e.g. when using `fab!`)
Disabled the (upcoming https://github.com/discourse/rubocop-discourse/pull/11) rubocop rule for two files that have thoroughly tangled both types of specs.
* FEATURE: Redirect logged in user to invite topic
Users who were already logged in and were given an invite link to a
topic used to see an error message saying that they already have an
account and cannot redeem the invite. This commit amends that behavior
and redirects the user directly to the topic, if they can see it.
* FEATURE: Add logged in user to invite groups
Users who were already logged in and were given an invite link to a
group used to see an error message saying that they already have an
account and cannot redeem the invite. This commit amends that behavior
and adds the user to the group.
And also move all the "top topics by period" routes to query string param.
/top/monthly => /top?period=monthly
/c/:slug/:id/l/top/monthly => /c/:slug/:id/l/top?period=monthly
/tag/:slug/l/top/daily => /tag/:slug/l/top?period=daily (new)
Badges that are awarded multiple times can be favorite and not favorite
at the same time. This caused few problems when users tried to favorite
them as they were counted multiple times or their state was incorrectly
displayed.
Before this change, calling `StyleSheet::Manager.stylesheet_details`
for the first time resulted in multiple queries to the database. This is
because the code was modelled in a way where each `Theme` was loaded
from the database one at a time.
This PR restructures the code such that it allows us to load all the
theme records in a single query. It also allows us to eager load the
required associations upfront. In order to achieve this, I removed the
support of loading multiple themes per request. It was initially added
to support user selectable theme components but the feature was never
completed and abandoned because it wasn't a feature that we thought was
worth building.
The first thing we needed here was an enum rather than a boolean to determine how a directory_column was created. Now we have `automatic`, `user_field` and `plugin` directory columns.
This plugin API is assuming that the plugin has added a migration to a column to the `directory_items` table.
This was created to be initially used by discourse-solved. PR with API usage - https://github.com/discourse/discourse-solved/pull/137/
When dismissing new topics for the Tracked filter, the dismiss was
limited to 30 topics which is the default per page count for TopicQuery.
This happened even if you specified which topic IDs you were
selectively dismissing. This PR fixes that bug, and also moves
the per_page_count into a DEFAULT_PER_PAGE_COUNT for the TopicQuery
so it can be stubbed in tests.
Also moves the unused stub_const method into the spec helpers
for cases like this; it is much better to handle this in one place
with an ensure. In a follow up PR I will clean up other specs that
do the same thing and make them use stub_const.
Subclasses must call #delete_user_actions inside build_actions to support user deletion. The method adds a delete user bundle, which has a delete and a delete + block option. Every subclass is responsible for implementing these actions.
The `ember_jquery` bundle contains production builds of Ember and jQuery
which doesn't work with tests. This commits introduces a new
`theme_qunit_vendor` bundle which is copy of the `vendor` bundle but
doesn't contain `ember_jquery`.
This commit is a partial revert of
409c8585e4
Over the years we have found that a few communities never discovered tags.
Instead of having them default off we now have them default on, ensuring
that everyone finds out about them.
Co-authored-by: Dan Ungureanu <dan@ungureanu.me>
In Ember CLI addons get put into the vendor bundle, as opposed to their
own bundle like we're doing in the Rails app. We never use pretty-text
without our vendor bundle so this should have no difference on
performance.
We need to keep the pretty-text bundle for server side cooking.
It used to require SiteSetting.min_trust_level_to_allow_invite to
invite a user to a group, even if the user existed and the inviter was
a group owner.
In Ember CLI, the vendor bundler includes Ember/jQuery, so this brings
our app closer to that configuration.
We have a couple pages (Reset Password / Confirm New Email) where we need
`ember_jquery` without vendor so the file still exists for those cases.
