This commit removes the ability to enable/disable the Sidebar on a per
user basis and introduces a site wide setting. For testing purposes, sidebar can be enabled/disabled via the `enable_sidebar=1` or `enable_sidebar=0` query param.
The idea behind this refactor is to centralise all of the user ignoring / muting / disallow PM checks in a single place, so they can be used consistently in core as well as for plugins like chat, while improving the main bulk of the checks to run in a single fast non-AR query.
Also fixed up the invite error when someone is muting/ignoring the user that is trying to invite them to the topic.
* FIX: Posts can belong to hard-deleted topics
This was a problem when serializing deleted posts because they might
belong to a topic that was permanently deleted. This caused to DB
lookup to fail immediately and raise an exception. In this case, the
endpoint returned a 404.
* FIX: Remove N+1 queries
Deleted topics were not loaded because of the default scope that
filters out all deleted topics. It executed a query for each deleted
topic.
Before, whispers were only available for staff members.
Config has been changed to allow to configure privileged groups with access to whispers. Post migration was added to move from the old setting into the new one.
I considered having a boolean column `whisperer` on user model similar to `admin/moderator` for performance reason. Finally, I decided to keep looking for groups as queries are only done for current user and didn't notice any N+1 queries.
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.
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.
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>
At some point in the past we decided to rename the 'regular' notification state of topics/categories to 'normal'. However, some UI copy was missed when the initial renaming was done so this commit changes the spots that were missed to the new name.
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 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>
Previous to this fix, in production we were getting 500 errors from
bootstrap.json cause script_asset_path can not find plugin-tests
Opted for the simplest fix that is omitting the files in production
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.
When saving / creating bookmarks, we have code to save
the user's preference of bookmark_auto_delete_preference
to their user_options.
Unfortunately this can cause weirdness when plugins
have code using BookmarkManager to set the auto delete preference for
only a specific bookmark.
This commit introduces a save_user_preferences option (false
by default) so that this user preference is not saved unless
specified by the consumer of BookmarkManager, so plugins will
not have to worry about it.
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.
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.
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.
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.
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.
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.
* 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`)
* DEV: Deprecate /posts/:id/reply-ids/all
It was added in ed4c0c4a63 and its only use was removed in b58867b6e9
Nothing in all-the* seems to be using this endpoint.
* Update app/controllers/posts_controller.rb
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
The `blocked onebox domains` setting lets site owners change what sites
are allowed to be oneboxed. When a link is entered into a post,
Discourse checks the domain of the link against that setting and blocks
the onebox if the domain is blocked. But if there's a chain of
redirects, then only the final destination website is checked against
the site setting.
This commit amends that behavior so that every website in the redirect
chain is checked against the site setting, and if anything is blocked
the original link doesn't onebox at all in the post. The
`Discourse-No-Onebox` header is also checked in every response and the
onebox is blocked if the header is set to "1".
Additionally, Discourse will now include the `Discourse-No-Onebox`
header with every response if the site requires login to access content.
This is done to signal to a Discourse instance that it shouldn't attempt
to onebox other Discourse instances if they're login-only. Non-Discourse
websites can also use include that header if they don't wish to have
Discourse onebox their content.
Internal ticket: t59305.
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 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
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.
This commit handles the edge case where a draft is lost with no warnings if the user edits the title (or category/tags) of a topic while they're replying.to the same topic. Repro steps are as follows:
1. Start replying to a topic and type enough to get a draft saved.
2. Scroll up to the topic title and click the pencil icon next to the topic title, change the title, category and/or tags, and then save the changes.
3. Reload the page and you'll see that the draft is gone.
This happens because we only allow 1 draft per topic per user and when you edit the title of a topic that you're replying to, from the server perspective it'll look like as if you've submitted your reply so it will advance the draft sequence for the topic and delete the draft.
The fix in this commit makes `PostRevisor` skip advancing the draft sequence when a topic's title is edited using the pencil button next to the title.
Internal ticket: t60854.
Co-authored-by: Robin Ward <robin.ward@gmail.com>
We validate the *format* of email addresses in many places with a match against
a regex, often with very slightly different syntax.
Adding a separate EmailAddressValidator simplifies the code in a few spots and
feels cleaner.
Deprecated the old location in case someone is using it in a plugin.
No functionality change is in this commit.
Note: the regex used at the moment does not support using address literals, e.g.:
* localpart@[192.168.0.1]
* localpart@[2001:db8::1]
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.
When parent category or grandparent category is muted, then category should be muted as well.
Still, it can be overridden by setting individual subcategory notification level.
CategoryUser record is not created, mute for subcategories is purely virtual.
This can happen if the topic to which a user is invited is in a private
category and the user was not invited to one of the groups that can see
that specific category.
This used to be a warning and this commit makes it an error.
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.
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 can be disabled by setting `EMBER_CLI_PROD_ASSETS=0`, but this option will not be available for long. If your theme/plugin/site has issues under Ember CLI, please open a topic on https://meta.discourse.org
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.
Job arguments go via JSON, and so symbols will appear as strings in the Job's `#execute` method. The latest version of Sidekiq has started warning about this to reduce developer confusion.
- 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
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.
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: Mark invites flash messages as HTML safe.
This change should be safe as all user inputs included in the errors are sanitized before sending it back to the client.
Context: https://meta.discourse.org/t/html-tags-are-explicit-after-latest-update/214220
* If somebody adds a new error message that includes user input and doesn't sanitize it, using html-safe suddenly becomes unsafe again. As an extra layer of protection, we make the client sanitize the error message received from the backend.
* Escape user input instead of sanitizing
* 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
* 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?
- switches to a raster image QR code so it can be long-pressed (or right
clicked) and added to iCloud keychain
- adds `autocomplete="one-time-code"` to the 2FA input for better
discoverability
This commit adds a check that runs regularly as per
2d68e5d942 which tests the
credentials of groups with SMTP or IMAP enabled. If any issues
are found with those credentials a high priority problem is added to the
admin dashboard.
This commit also formats the admin dashboard differently if
there are high priority problems, bringing them to the top of
the list and highlighting them.
The problem will be cleared if the issue is fixed before the next
problem check, or if the group's settings are updated with a valid
credential.
* 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
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"
```
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.
Currently the Message-IDs we send out for outbound email
are not unique; for a post they look like:
topic/TOPIC_ID/POST_ID@HOST
And for a topic they look like:
topic/TOPIC_ID@HOST
This commit changes the outbound Message-IDs to also have
a random suffix before the host, so the new format is
like this:
topic/TOPIC_ID/POST_ID.RANDOM_SUFFIX@HOST
Or:
topic/TOPIC_ID.RANDOM_SUFFIX@HOST
This should help with email deliverability. This change
is backwards-compatible, the old Message-ID format will
still be recognized in the mail receiver flow, so people
will still be able to reply using Message-IDs, In-Reply-To,
and References headers that have already been sent.
This commit also refactors Message-ID related logic
to a central location, and adds judicious amounts of
tests and documentation.
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.
* REFACTOR: Improve support for consolidating notifications.
Before this commit, we didn't have a single way of consolidating notifications. For notifications like group summaries, we manually removed old ones before creating a new one. On the other hand, we used an after_create callback for likes and group membership requests, which caused unnecessary work, as we need to delete the record we created to replace it with a consolidated one.
We now have all the consolidation rules centralized in a single place: the consolidation planner class. Other parts of the app looking to create a consolidable notification can do so by calling Notification#consolidate_or_save!, instead of the default Notification#create! method.
Finally, we added two more rules: one for re-using existing group summaries and another for deleting duplicated dashboard problems PMs notifications when the user is tracking the moderator's inbox. Setting the threshold to one forces the planner to apply this rule every time.
I plan to add plugin support for adding custom rules in another PR to keep this one relatively small.
* DEV: Introduces a plugin API for consolidating notifications.
This commit removes the `Notification#filter_by_consolidation_data` scope since plugins could have to define their criteria. The Plan class now receives two blocks, one to query for an already consolidated notification, which we'll try to update, and another to query for existing ones to consolidate.
It also receives a consolidation window, which accepts an ActiveSupport::Duration object, and filter notifications created since that value.
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
Use @here to mention all users that were allowed to topic directly or
through group, who liked topics or read the topic. Only first 10 users
will be notified.
This takes the uppy chunking algorithm and combines it with some
form submission from resumable.js for parity with the current
backup controller to make local backup uploads work with uppy.
We can then use this to replace the resumable-upload component
and the resumable.js library from our codebase, once stable.
This is disabled by default, so people using local backups will not
be affected. The enable_experimental_backup_uploader site setting
must be enabled for this to work.
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.
This commit introduces a new s3:ensure_cors_rules rake task
that is run as a prerequisite to s3:upload_assets. This rake
task calls out to the S3CorsRulesets class to ensure that
the 3 relevant sets of CORS rules are applied, depending on
site settings:
* assets
* direct S3 backups
* direct S3 uploads
This works for both Global S3 settings and Database S3 settings
(the latter set directly via SiteSetting).
As it is, only one rule can be applied, which is generally
the assets rule as it is called first. This commit changes
the ensure_cors! method to be able to apply new rules as
well as the existing ones.
This commit also slightly changes the existing rules to cover
direct S3 uploads via uppy, especially multipart, which requires
some more headers.
This commit changes the emoji uploader to use the UppyUploadMixin,
and makes some minor changes to the emoji uploader (tightening the
copy for drag and drop and adding a percentage for the upload).
Since no other uppy upload mixin components have needed to upload
multiple files so far, this necessitated adding a tracker for the
in progress uploads so we know when to reset the uploader once all
uploads are complete.
At the moment, the emoji uploader cannot be used for direct S3 uploads
because the admin emoji controller creates other records and does other
magic with the emojis. At some point we need to factor this kind of thing
into the ExternalUploadManager.transform! action to complete external
uploads.
* 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)
Instead of using image-uploader, which relies on the old
UploadMixin, we can now use the uppy-image-uploader which
uses the new UppyUploadMixin which is stable enough and
supports both regular XHR uploads and direct S3 uploads,
controlled by a site setting (default to XHR).
At some point it may make sense to rename uppy-image-uploader
back to image-uploader, once we have gone through plugins
etc. and given a bit of deprecation time period.
This commit also fixes `for_private_message`, `for_site_setting`,
and `pasted` flags not being sent via uppy uploads onto the
UploadCreator, both via regular XHR uploads and also through
external/multipart uploads.
The uploaders changed are:
* site setting images
* badge images
* category logo
* category background
* group flair
* profile background
* profile card background
* 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.
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
* DEV: Remove HTML setting type and sanitization logic.
We concluded that we don't want settings to contain HTML, so I'm removing the setting type and sanitization logic. Additionally, we no longer allow the global-notice text to contain HTML.
I searched for usages of this setting type in the `all-the-plugins` repo and found none, so I haven't added a migration for existing settings.
* Mark Global notices containing links as HTML Safe.
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 all inboxes was introduced in
016efeadf6 but we decided to roll it back
for performance reasons. The main performance challenge here is that PG
has to basically loop through all the PMs that a user is allowed to view
before being able to order by `Topic#bumped_at`. The all inboxes was not
planned as part of the new/unread filter so we've decided not to tackle
the performance issue for the upcoming release.
Follow-up to 016efeadf6
This commit allows for measuring the time taken for
individual uploads via the new uppy interfaces, only
if the enable_upload_debug_mode site setting is enabled.
Also in this PR, for upload errors with a specific message
locally, we return the real message to show in the modal
instead of the upload.failed message so the developer
does not have to dig around in logs.
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
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.
This partially reverts commit ddb458343d.
Seeing performance degrade on larger sites so back to drawing board on
this one. Instead of the DISTINCT LEFT JOIN, we switch back to
IN(subquery).
First reported in https://meta.discourse.org/t/-/202482/19
There are two optimizations being applied here:
1. Fetch a user's group ids in a seperate query instead of including it
as a sub-query. When I tried a subquery, the query plan becomes very
inefficient.
1. Join against the `topic_allowed_users` and `topic_allowed_groups`
table instead of doing an IN against a subquery where we UNION the
`topic_id`s from the two tables. From my profiling, this enables PG to
do a backwards index scan on the `index_topics_on_timestamps_private`
index.
This commit fixes a bug where listing all messages was incorrectly
excluding topics if a topic has been archived by a group even if the
user did not belong to the group.
This commit also fixes another bug where dismissing private messages
selectively was subjected to the default limit of 30.
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
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.
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.
Plugins listening on the `before_auth` callback can interact with the request object and access data like the user agent or the remote IP address. We'll later store this data in the user record, but it might not exist at this point if we're authenticating a new account.
We shouldn't be checking if a user is allowed to do an action in the logger. We should be checking it just before we perform the action. In fact, guardians in the logger can make things even worse in case of a security bug. Let's say we forgot to check user's permissions before performing some action, but we still have a call to the guardian in the logger. In this case, a user would perform the action anyway, and this action wouldn't even be logged!
I've checked all cases and I confirm that we're safe to delete this calls from the logger.
I've added two calls to guardians in admin/user_controller. We didn't have security bugs there, because regular users can't access admin/... routes at all. But it's good to have calls to guardian in these methods anyway, neighboring methods have them.
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.
The cache_fullpath for the Stylesheet::Manager was the same for
every test runner in a parallel test environment, so when other
specs or other places e.g. the stylesheets_controller_spec ran
rm -rf Stylesheet::Manager.cache_fullpath this caused errors
for other specs running that went through the
Stylesheet::Manager::Builder#compile path, causing the error
```
Errno::ENOENT:
No such file or directory @ rb_sysopen
```
Also fixed the stylesheet_controller which was interpolating Rails.root + CACHE_PATH
itself instead of just using Stylesheet::Manager.cache_fullpath
* 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
This is confusing because you're running the tests on the older version
of Ember. Use `/tests` for Ember CLI, and `/qunit` when using Rails'
asset pipeline (but only if REALLY necessary!)
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.
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.
* 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)
Users can invite people to topics from secured category, but they will
not be redirected to the topic after signing up unless they have the
permissions to view the topic. This commit shows a warning when invite
is saved if the topic is in a secured category and none of the invite
groups are allowed to see it.
Exposes to Ember CLI environment the feature provided in the production env by `lib/stylesheet/manager.rb:295`.
Fixes development env compatibility with discourse-color-scheme-toggle.
ATM it only implements server side of it, as my need is for automation purposes. However it should probably be added in the UI too as it's unexpected to have pinned_until and no bannered_until.
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.
The exception page is shown before Ember can actually figure out what the final destination URL we're going to is.
This means that the new page is not present in the history stack, so if we attempt to use the history stack to go back, we will actually navigate back by two steps.
By instead forcing a navigation to the current URL, we achieve the goal of going "back" with no history mucking.
Unfortunately, the actual URL that was attempted is not available. Additionally, this only works for the on-screen back button and not the browser back.
Additionally, several modernizations of the exception page code were made.
This was previously broken by 59ef48c0b9 (#11425, #11424).
Centralize the logic into the exception controller, which avoids the problematic bug and makes it easy to add additional detailed 404 pages in the future.
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.
The `bootstrap.json` contains most preloaded information but some routes
provide extra information, such as invites.
This fixes the issue by having the preload request pass on the preloaded
data from the source page, which is then merged with the bootstrap's
preloaded data for the final HTML payload.
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.
I merged this PR in yesterday, finally thinking this was done https://github.com/discourse/discourse/pull/12958 but then a wild performance regression occurred. These are the problem methods:
1aa20bd681/app/serializers/topic_tracking_state_serializer.rb (L13-L21)
Turns out date comparison is super expensive on the backend _as well as_ the frontend.
The fix was to just move the `treat_as_new_topic_start_date` into the SQL query rather than using the slower `UserOption#treat_as_new_topic_start_date` method in ruby. After this change, 1% of the total time is spent with the `created_in_new_period` comparison instead of ~20%.
----
History:
Original PR which had to be reverted **https://github.com/discourse/discourse/pull/12555**. See the description there for what this PR is achieving, plus below.
The issue with the original PR is addressed in 92ef54f402
If you went to the `x unread` link for a tag Chrome would freeze up and possibly crash, or eventually unfreeze after nearly 10 mins. Other routes for unread/new were similarly slow. From profiling the issue was the `sync` function of `topic-tracking-state.js`, which calls down to `isNew` which in turn calls `moment`, a change I had made in the PR above. The time it takes locally with ~1400 topics in the tracking state is 2.3 seconds.
To solve this issue, I have moved these calculations for "created in new period" and "unread not too old" into the tracking state serializer.
When I was looking at the profiler I also noticed this issue which was just compounding the problem. Every time we modify topic tracking state we recalculate the sidebar tracking/everything/tag counts. However this calls `forEachTracked` and `countTags` which can be quite expensive as they go through the whole tracking state (and were also calling the removed moment functions).
I added some logs and this was being called 30 times when navigating to a new /unread route because `sync` is being called from `build-topic-route` (one for each topic loaded due to pagination). So I just added a debounce here and it makes things even faster.
Finally, I changed topic tracking state to use a Map so our counts of the state keys is faster (Maps have .size whereas objects you have to do Object.keys(obj) which is O(n).)
<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
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.
Original PR which had to be reverted **https://github.com/discourse/discourse/pull/12555**. See the description there for what this PR is achieving, plus below.
The issue with the original PR is addressed in 92ef54f402
If you went to the `x unread` link for a tag Chrome would freeze up and possibly crash, or eventually unfreeze after nearly 10 mins. Other routes for unread/new were similarly slow. From profiling the issue was the `sync` function of `topic-tracking-state.js`, which calls down to `isNew` which in turn calls `moment`, a change I had made in the PR above. The time it takes locally with ~1400 topics in the tracking state is 2.3 seconds.
To solve this issue, I have moved these calculations for "created in new period" and "unread not too old" into the tracking state serializer.
When I was looking at the profiler I also noticed this issue which was just compounding the problem. Every time we modify topic tracking state we recalculate the sidebar tracking/everything/tag counts. However this calls `forEachTracked` and `countTags` which can be quite expensive as they go through the whole tracking state (and were also calling the removed moment functions).
I added some logs and this was being called 30 times when navigating to a new /unread route because `sync` is being called from `build-topic-route` (one for each topic loaded due to pagination). So I just added a debounce here and it makes things even faster.
Finally, I changed topic tracking state to use a Map so our counts of the state keys is faster (Maps have .size whereas objects you have to do Object.keys(obj) which is O(n).)
This overhauls the user interface for the group email settings management, aiming to make it a lot easier to test the settings entered and confirm they are correct before proceeding. We do this by forcing the user to test the settings before they can be saved to the database. It also includes some quality of life improvements around setting up IMAP and SMTP for our first supported provider, GMail. This PR does not remove the old group email config, that will come in a subsequent PR. This is related to https://meta.discourse.org/t/imap-support-for-group-inboxes/160588 so read that if you would like more backstory.
### UI
Both site settings of `enable_imap` and `enable_smtp` must be true to test this. You must enable SMTP first to enable IMAP.
You can prefill the SMTP settings with GMail configuration. To proceed with saving these settings you must test them, which is handled by the EmailSettingsValidator.
If there is an issue with the configuration or credentials a meaningful error message should be shown.
IMAP settings must also be validated when IMAP is enabled, before saving.
When saving IMAP, we fetch the mailboxes for that account and populate them. This mailbox must be selected and saved for IMAP to work (the feature acts as though it is disabled until the mailbox is selected and saved):
### Database & Backend
This adds several columns to the Groups table. The purpose of this change is to make it much more explicit that SMTP/IMAP is enabled for a group, rather than relying on settings not being null. Also included is an UPDATE query to backfill these columns. These columns are automatically filled when updating the group.
For GMail, we now filter the mailboxes returned. This is so users cannot use a mailbox like Sent or Trash for syncing, which would generally be disastrous.
There is a new group endpoint for testing email settings. This may be useful in the future for other places in our UI, at which point it can be extracted to a more generic endpoint or module to be included.
There are two methods which the server uses to verify an invite is being redeemed with a matching email:
1) The email token, supplied via a `?t=` parameter
2) The validity of the email, as provided by the auth provider
Only one of these needs to be true for the invite to be redeemed successfully on the server. The frontend logic was previously only checking (2). This commit updates the frontend logic to match the server.
This commit does not affect the invite redemption logic. It only affects the 'show' endpoint, and the UI.
The previous commits removed reviewables leading to a bad user
experience. This commit updates the status, replaces actions with a
message and greys out the reviewable.
This PR improves the UI of bulk select so that its context is applied to the Dismiss Unread and Dismiss New buttons. Regular users (not just staff) are now able to use topic bulk selection on the /new and /unread routes to perform these dismiss actions more selectively.
For Dismiss Unread, there is a new count in the text of the button and in the modal when one or more topic is selected with the bulk select checkboxes.
For Dismiss New, there is a count in the button text, and we have added functionality to the server side to accept an array of topic ids to dismiss new for, instead of always having to dismiss all new, the same as the bulk dismiss unread functionality. To clean things up, the `DismissTopics` service has been rolled into the `TopicsBulkAction` service.
We now also show the top Dismiss/Dismiss New button based on whether the bottom one is in the viewport, not just based on the topic count.
This allows plugins to store/modify things in the session (e.g. the destination_url). This change is backwards compatible with existing plugins. If they do not specify a third argument, they will just be passed the first two.
It used to allow adding email addresses to a group even if invites were
disabled for the site. This does not allow user to input email address
if they cannot invite.
The second thing this commit improves is the message that is displayed
to the user when they hit the invite rate limit.
Over the years we accrued many spelling mistakes in the code base.
This PR attempts to fix spelling mistakes and typos in all areas of the code that are extremely safe to change
- comments
- test descriptions
- other low risk areas
* FIX: Hide tag watched words if tagging is disabled
These 'autotag' words were shown even if tagging was disabled.
* FIX: Make autotag watched words case insensitive
This commit also fixes the bug when no tag was applied if no other tag
was already present.
* FIX: Ensure the same email cannot be invited twice
When creating a new invite with a duplicated email, the old invite will
be updated and returned. When updating an invite with a duplicated email
address, an error will be returned.
* FIX: not Ember helper does not exist
* FIX: Sync can_invite_to_forum? and can_invite_to?
The two methods should perform the same basic set of checks, such as
check must_approve_users site setting.
Ideally, one of the methods would call the other one or be merged and
that will happen in the future.
* FIX: Show invite to group if user is group owner
Some emails coming in via the mail receiver can still end up
with bad encoding when trying to enqueue the job. This catches
the last encoding issue and forces iso-8559-1 and encodes to
UTF-8 to circumvent the issue.
Currently, when the target is not available we're returning the error message "`You are not permitted to view the requested resource`" which is not clear.
* FEATURE: Allow sending a message with invite
It used to be a staff-only feature and this commit makes it available
to everyone who can invite.
* FIX: Inviting to topic uses another email template
This used to be the case, but the extra parameter was lost when we
switched to the new modal.
We have found when receiving and posting inbound emails to the handle_mail route, it is better to POST the payload as a base64 encoded string to avoid strange encoding issues. This introduces a new param of `email_encoded` and maintains the legacy param of email, showing a deprecation warning. Eventually the old param of `email` will be dropped and the new one `email_encoded` will be the only way to handle_mail.
We really want to encourage all developers to use Ember CLI for local
development and testing. This will display an error page if they are not
with instructions on how to start the local server.
To disable it, you can set `NO_EMBER_CLI=1` as an ENV variable
* FEATURE: Small improvements to the topic list embed
- Ability to wrap the list in a custom class so you can styles different
lists using specific CSS
- Adds a topic link to the thumbnail when using the complete template
* FIX: Be more strict about allowed chars in class name
This commit allows site admins to run theme tests in production via a new `/theme-qunit` route. When you visit `/theme-qunit`, you'll see a list of the themes/components installed on your site that have tests, and from there you can select a theme or component that you run its tests.
We also have a new rake task `themes:install_and_test` that can be used to install a list of themes/components on a temporary database and run the tests of the themes/components that are installed. This rake task can be useful when upgrading/deploying a Discourse instance to make sure that the installed themes/components are compatible with the new Discourse version being deployed, and if the tests fail you can abort the build/deploy process so you don't end up with a broken site.
The aim of this PR is to improve the topic tracking state JavaScript code and test coverage so further modifications can be made in plugins and in core. This is focused on making topic tracking state changes easier to respond to with callbacks, and changing it so all state modifications go through a single method instead of modifying `this.state` all over the place. I have also tried to improve documentation, make the code clearer and easier to follow, and make it clear what are public and private methods.
The changes I have made here should not break backwards compatibility, though there is no way to tell for sure if other plugin/theme authors are using tracking state methods that are essentially private methods. Any name changes made in the tracking-state.js code have been reflected in core.
----
We now have a `_trackedTopicLimit` in the tracking state. Previously, if a topic was neither new nor unread it was removed from the tracking state; now it is only removed if we are tracking more than `_trackedTopicLimit` topics (which is set to 4000). This is so plugins/themes adding topics with `TopicTrackingState.register_refine_method` can add topics to track that aren't necessarily new or unread, e.g. for totals counts.
Anywhere where we were doing `tracker.states["t" + data.topic_id] = newObject` has now been changed to flow through central `modifyState` and `modifyStateProp` methods. This is so state objects are not modified until they need to be (e.g. sometimes properties are set based on certain conditions) and also so we can run callback functions when the state is modified.
I added `onStateChange` and `onMessageIncrement` methods to register callbacks that are called when the state is changed and when the message count is incremented, respectively. This was done so we no longer need to do things like `@observes("trackingState.states")` in other Ember classes.
I split up giant functions like `sync` and `establishChannels` into smaller functions for readability and testability, and renamed many small functions to _functionName to designate them as private functions which not be called by consumers of `topicTrackingState`. Public functions are now all documented (well...at least ones that are not immediately obvious).
----
On the backend side, I have changed the MessageBus publish events for TopicTrackingState to send back tags and tag IDs for more channels, and done some extra code cleanup and refactoring. Plugins may override `TopicTrackingState.report` so I have made its footprint as small as possible and externalised the main parts of it into other methods.
If the "use_site_small_logo_as_system_avatar" setting is enabled, the site's small logo is displayed as the selected option by the avatar-selector. Choosing a different avatar disables the setting.
These endpoints only return one `Theme` row, but the one-many relations were not being preloaded efficiently. This commit moves the `includes` statement to a scope, and makes use of it in `#index`, `#show`, and `#update`.
When the admin creates a new custom field they can specify if that field should be searchable or not.
That setting is taken into consideration for quick search results.
This commit allows site admins to run theme tests in production via a new `/theme-qunit` route. When you visit `/theme-qunit`, you'll see a list of the themes/components installed on your site that have tests, and from there you can select a theme or component that you run its tests.
We also have a new rake task `themes:install_and_test` that can be used to install a list of themes/components on a temporary database and run the tests of the themes/components that are installed. This rake task can be useful when upgrading/deploying a Discourse instance to make sure that the installed themes/components are compatible with the new Discourse version being deployed, and if the tests fail you can abort the build/deploy process so you don't end up with a broken site.
The old share modal used to host both share and invite functionality,
under two tabs. The new "Share Topic" modal can be used only for
sharing, but has a link to the invite modal.
Among the sharing methods, there is also "Notify" which points out
that existing users will simply be notified (this was not clear
before). Staff members can notify as many users as they want, but
regular users are restricted to one at a time, no more than
max_topic_invitations_per_day. The user will not receive another
notification if they have been notified of the same topic in past hour.
The "Create Invite" modal also suffered some changes: the two radio
boxes for selecting the type (invite or email) have been replaced by a
single checkbox (is email?) and then the two labels about emails have
been replaced by a single one, some fields were reordered and the
advanced options toggle was moved to the bottom right of the modal.
* DEV: Give a nicer error when `--proxy` argument is missing
* DEV: Improve Ember CLI's bootstrap logic
Instead of having Ember CLI know which URLs to proxy or not, have it try
the URL with a special header `HTTP_X_DISCOURSE_EMBER_CLI`. If present,
and Discourse thinks we should bootstrap the application, it will
instead stop rendering and return a HTTP HEAD with a response header
telling Ember CLI to bootstrap.
In other words, any time Rails would otherwise serve up the HTML for the
Ember app, it stops and says "no, you do it."
* DEV: Support asset filters by path using a new options object
Without this, Ember CLI's bootstrap would not get the assets it wants
because the path it was requesting was different than the browser path.
This adds an optional request header to fix it.
So far this is only used by the styleguide.
Rails 6.1.3.1 deprecates a few API and has some internal changes that break our tests suite, so this commit fixes all the deprecations and errors and now Discourse should be fully compatible with Rails 6.1.3.1. We also have a new release of the rails_failover gem that's compatible with Rails 6.1.3.1.
scopes are incredibly annoying to preload, simply adding :user_emails is not
enough.
Instead of relying on scopes simply iterate through user_emails which is
properly preloaded.
This removes 2 * N+1 when generating user reports.
The server used to respond with a generic 'error, contact admin' message
which did not offer any hint what the error was. This happened even when
the error could be easily corrected by the user (for example, if they
chose a very common password).
We used to generate invite keys that were 32-characters long which were
not very friendly and lead to very long links. This commit changes the
generation method to use almost all alphanumeric characters to produce
a 10-character long invite key.
This commit also introduces a rate limit for redeeming invites because
the probability of guessing an invite key has increased.
When invited by email, users will receive an invite URL which contains
a token. If that token is present when the invite is redeemed, their
account will be automatically activated.
This PR adds a new category setting which is a column in the `categories` table, `allow_unlimited_owner_edits_on_first_post`.
What this does is:
* Inside the `can_edit_post?` method of `PostGuardian`, if the current user editing a post is the owner of the post, it is the first post, and the topic's category has `allow_unlimited_owner_edits_on_first_post`, then we bypass the check for `LimitedEdit#edit_time_limit_expired?` on that post.
* Also, similar to wiki topics, in `PostActionNotifier#after_create_post_revision` we send a notification to all users watching a topic when the OP is edited in a topic with the category setting `allow_unlimited_owner_edits_on_first_post` enabled.
This is useful for forums where there is a Marketplace or similar category, where topics are created and then updated indefinitely by the OP rather than the OP making new topics or additional replies. In a way this acts similar to a wiki that only one person can edit.
This makes behavior consistent with documentation:
API:
> Will send an email with this message when present
Web UI:
> Optionally, provide more information about the suspension and it will be emailed to the user
This commit allows themes and theme components to have QUnit tests. To add tests to your theme/component, create a top-level directory in your theme and name it `test`, and Discourse will save all the files in that directory (and its sub-directories) as "tests files" in the database. While tests files/directories are not required to be organized in a specific way, we recommend that you follow Discourse core's tests [structure](https://github.com/discourse/discourse/tree/master/app/assets/javascripts/discourse/tests).
Writing theme tests should be identical to writing plugins or core tests; all the `import` statements and APIs that you see in core (or plugins) to define/setup tests should just work in themes.
You do need a working Discourse install to run theme tests, and you have 2 ways to run theme tests:
* In the browser at the `/qunit` route. `/qunit` will run tests of all active themes/components as well as core and plugins. The `/qunit` now accepts a `theme_name` or `theme_url` params that you can use to run tests of a specific theme/component like so: `/qunit?theme_name=<your_theme_name>`.
* In the command line using the `themes:qunit` rake task. This take is meant to run tests of a single theme/component so you need to provide it with a theme name or URL like so: `bundle exec rake themes:qunit[name=<theme_name>]` or `bundle exec rake themes:qunit[url=<theme_url>]`.
There are some refactors to how Discourse processes JavaScript that comes with themes/components, and these refactors may break your JS customizations; see https://meta.discourse.org/t/upcoming-core-changes-that-may-break-some-themes-components-april-12/186252?u=osama for details on how you can check if your themes/components are affected and what you need to do to fix them.
This commit also improves theme error handling in Discourse. We will now be able to catch errors that occur when theme initializers are run and prevent them from breaking the site and other themes/components.
To add an extra layer of security, we sanitize settings before shipping them to the client. We don't sanitize those that have the "html" type.
The CookedPostProcessor already uses Loofah for sanitization, so I chose to also use it for this. I added it to our gemfile since we installed it as a transitive dependency.
This commit allows themes and theme components to have QUnit tests. To add tests to your theme/component, create a top-level directory in your theme and name it `test`, and Discourse will save all the files in that directory (and its sub-directories) as "tests files" in the database. While tests files/directories are not required to be organized in a specific way, we recommend that you follow Discourse core's tests [structure](https://github.com/discourse/discourse/tree/master/app/assets/javascripts/discourse/tests).
Writing theme tests should be identical to writing plugins or core tests; all the `import` statements and APIs that you see in core (or plugins) to define/setup tests should just work in themes.
You do need a working Discourse install to run theme tests, and you have 2 ways to run theme tests:
* In the browser at the `/qunit` route. `/qunit` will run tests of all active themes/components as well as core and plugins. The `/qunit` now accepts a `theme_name` or `theme_url` params that you can use to run tests of a specific theme/component like so: `/qunit?theme_name=<your_theme_name>`.
* In the command line using the `themes:qunit` rake task. This take is meant to run tests of a single theme/component so you need to provide it with a theme name or URL like so: `bundle exec rake themes:qunit[name=<theme_name>]` or `bundle exec rake themes:qunit[url=<theme_url>]`.
There are some refactors to internal code that's responsible for processing themes/components in Discourse, most notably:
* `<script type="text/discourse-plugin">` tags are automatically converted to modules.
* The `theme-settings` service is removed in favor of a simple `lib` file responsible for managing theme settings. This was done to allow us to register/lookup theme settings very early in our Ember app lifecycle and because there was no reason for it to be an Ember service.
These refactors should 100% backward compatible and invisible to theme developers.
In Improve invite system, a newly created link only invite cannot
be retrieved via API with the invitee's email once created. A new
route, /invites/retrieve, is introduced to fetch an already
created invite by email address.
For 'local logins', the UX for staged users is designed to be identical to unregistered users. However, staged users logging in via external auth were being automatically unstaged, and skipping the registration/invite flow. In the past this made sense because the registration/invite flows didn't work perfectly with external auth. Now, both registration and invites work well with external auth, so it's best to leave the 'unstage' logic to those endpoints.
This problem was particularly noticeable when using the 'bulk invite' feature to invite users with pre-configured User Fields. In that situation, staged user accounts are used to preserve the user field data.
Admins can use bulk invites to pre-populate user fields. The imported
CSV file must have a header with "email" column (first position) and
names of the user fields (exact match).
Under the hood, the bulk invite will create staged users and populate
the user fields of those.
Fixes `Rack::Lint::LintError: a header value must be a String, but the value of 'Retry-After' is a Integer`. (see: 14a236b4f0/lib/rack/lint.rb (L676))
I found it when I got flooded by those warning a while back in a test-related accident 😉 (ember CLI tests were hitting a local rails server at a fast rate)
Find & Replace and Autotag watched words were not completely exported
and import did not work with these either. This commit changes the
input and output format to CSV, which allows for a secondary column.
This change is backwards compatible because a CSV file with only one
column has one value per line.
browser-update script does not work correctly in some very old browsers
because the contents of <noscript> is not accessible in JavaScript.
For these browsers, the server can display the crawler page and add the
browser update notice.
Simply loading the browser-update script in the crawler view is not a
solution because that means all crawlers will also see it.
Users can now pin bookmarks from their bookmark list. This will anchor the bookmark to the top of the list, and show a pin icon next to it. This also applies in the nav bookmarks panel. If there are multiple pinned bookmarks they sort by last updated order.
This commit ensures that email validation is skipped when the email is
obfuscated, that the email is no longer send when it is not an invite
link and no username is suggested if the email is hidden as it may
reveal the first part of the email.
Follow up to commit 033d6b6437.
This PR allows invitations to be used when the DiscourseConnect SSO is enabled for a site (`enable_discourse_connect`) and local logins are disabled. Previously invites could not be accepted with SSO enabled simply because we did not have the code paths to handle that logic.
The invitation methods that are supported include:
* Inviting people to groups via email address
* Inviting people to topics via email address
* Using invitation links generated by the Invite Users UI in the /my/invited/pending route
The flow works like this:
1. User visits an invite URL
2. The normal invitation validations (redemptions/expiry) happen at that point
3. We store the invite key in a secure session
4. The user clicks "Accept Invitation and Continue" (see below)
5. The user is redirected to /session/sso then to the SSO provider URL then back to /session/sso_login
6. We retrieve the invite based on the invite key in secure session. We revalidate the invitation. We show an error to the user if it is not valid. An additional check here for invites with an email specified is to check the SSO email matches the invite email
7. If the invite is OK we create the user via the normal SSO methods
8. We redeem the invite and activate the user. We clear the invite key in secure session.
9. If the invite had a topic we redirect the user there, otherwise we redirect to /
Note that we decided for SSO-based invites the `must_approve_users` site setting is ignored, because the invite is a form of pre-approval, and because regular non-staff users cannot send out email invites or generally invite to the forum in this case.
Also deletes some group invite checks as per https://github.com/discourse/discourse/pull/12353
Currently the process of adding a custom image to badge is quite clunky; you have to upload your image to a topic, and then copy the image URL and pasting it in a text field. Besides being clucky, if the topic or post that contains the image is deleted, the image will be garbage-collected in a few days and the badge will lose the image because the application is not that the image is referenced by a badge.
This commit improves that by adding a proper image uploader widget for badge images.
The cluster name can be configured by setting the `DISCOURSE_CLUSTER_NAME` environment variable. If set, you can then call /srv/status with a `?cluster=` parameter. If the cluster does not match, an error will be returned. This is useful if you need a load balancer to be able to verify the identity, as well as the presence, of an application container.
Staff can send a post to the review queue by clicking the "Flag Post" button next to "Take Action...". Clicking it flags the post using the "Notify moderators" score type and hides it. A custom message will be sent to the user.
The user and an admin could create multiple email change requests for
the same user. If any of the requests was validated and it became
primary, the other request could not be deleted anymore.
* FEATURE: allow category group moderators to pin/unpin topics
Category group moderators should be able to pin/unpin any topics within a category where they have appropraite category group moderator permissions.
Previously, we blocked search engines in tag pages since they may get marked as a duplicate content.
* DEV: block tag inner pages from search engines crawling.
* DEV: Use Ember CLI middleware to decorate the index template
Previously we'd do this on the client side which did not support our
full plugin API. Now requests for the index template will contact the
dev server for a bootstrap.json and apply it to the current template.
* FIX: Allows logins in development mode for Ember CLI
* FIX: Do not show expired invites under Pending tab
* DEV: Controller action was renamed in previous commit
* FEATURE: Add 'Expired' tab to invites
* FEATURE: Refresh model after removing expired invites
* FEATURE: Do not immediately add invite to the list
Opening the 'create-invite' modal used to automatically generate an
invite to reserve an invite link. If the user did not save it and
closed the modal, the invite would be destroyed. This operations caused
the invite list to change in the background and confuse users.
* FEATURE: Sort redeemed users by creation time
* UX: Improve show / hide advanced options link
* FIX: Show redeemed users even if invites were trashed
* UX: Change modal title when editing invite
* UX: Remove Get Link button
Users can get it from the edit modal
* FEATURE: Add limit for invite links generated by regular users
* FEATURE: Add option to skip email
* UX: Show better error messages
* FIX: Show "Invited by" even if invite was trashed
Follow up to 1fdfa13a099d8e46edd0c481b3aaaafe40455ced.
* FEATURE: Add button to save without sending email
Follow up to c86379a465f28a3cc64a4a8c939cf32cf2931659.
* DEV: Use a buffer to hold all changed data
* FEATURE: Close modal after save
* FEATURE: Rate limit resend invite email
* FEATURE: Make the save buttons smarter
* FEATURE: Do not always send email even for new invites
A missing email when accepting an invite link does not make sense so we
should make it a required param which helps to catch bugs in our test
suite and also prevent potential bugs in our code base when the code
trips on a `nil` email.
The user interface has been reorganized to show email and link invites
in the same screen. Staff has more control over creating and updating
invites. Bulk invite has also been improved with better explanations.
On the server side, many code paths for email and link invites have
been merged to avoid duplicated logic. The API returns better responses
with more appropriate HTTP status codes.
We were sending 2 emails for user silencing if a message was provided in the UI. Also always send email for user silence and user suspend with reason regardless of whether message provided.
The API now accepts an array called "ids" to select specific items. This parameter is not present on the UI.
Example usage: "yoursite.com/review.json?ids[]=1&ids[]=2"
This commit includes other various improvements to watched words.
auto_silence_first_post_regex site setting was removed because it overlapped
with 'require approval' watched words.
Adding a scope from a plugin was broken. This commit fixes it and adds a test.
It also documents the instance method and renames the serialized "id" attribute to "scope_id" to avoid a conflict when the scope also has a parameter with the same name.
The server responds with a redirect to URLs with wrong slugs, even when
the slug was the correct but in the encoded form (if it contained
special characters).
Original PR was reverted because of broken migration https://github.com/discourse/discourse/pull/12058
I fixed it by adding this line
```
AND topics.id IN(SELECT id FROM topics ORDER BY created_at DESC LIMIT :max_new_topics)
```
This time it is left joining a limited amount of topics. I tested it on few databases and it worked quite smooth
Follow up https://github.com/discourse/discourse/pull/11968
Dismiss all new topics using the same DismissTopicService. In addition, MessageBus receives exact topic ids which should be marked as `seen`.
* FEATURE: Ability to dismiss new topics in a specific tag
Follow up of https://github.com/discourse/discourse/pull/11927
Using the same mechanism to disable new topics in a tag.
* FIX: respect when category and tag is selected
The 'Discourse SSO' protocol is being rebranded to DiscourseConnect. This should help to reduce confusion when 'SSO' is used in the generic sense.
This commit aims to:
- Rename `sso_` site settings. DiscourseConnect specific ones are prefixed `discourse_connect_`. Generic settings are prefixed `auth_`
- Add (server-side-only) backwards compatibility for the old setting names, with deprecation notices
- Copy `site_settings` database records to the new names
- Rename relevant translation keys
- Update relevant translations
This commit does **not** aim to:
- Rename any Ruby classes or methods. This might be done in a future commit
- Change any URLs. This would break existing integrations
- Make any changes to the protocol. This would break existing integrations
- Change any functionality. Further normalization across DiscourseConnect and other auth methods will be done separately
The risks are:
- There is no backwards compatibility for site settings on the client-side. Accessing auth-related site settings in Javascript is fairly rare, and an error on the client side would not be security-critical.
- If a plugin is monkey-patching parts of the auth process, changes to locale keys could cause broken error messages. This should also be unlikely. The old site setting names remain functional, so security-related overrides will remain working.
A follow-up commit will be made with a post-deploy migration to delete the old `site_settings` rows.
This PR allows entering a float value for topic timers e.g. 0.5 for 30 minutes when entering hours, 0.5 for 12 hours when entering days. This is achieved by adding a new column to store the duration of a topic timer in minutes instead of the ambiguous both hours and days that it could be before.
This PR has ommitted the post migration to delete the duration column in topic timers; it will be done in a subsequent PR to ensure that no data is lost if the UPDATE query to set duration_mintues fails.
I have to keep the old keyword of duration in set_or_create_topic_timer for backwards compat, will remove at a later date after plugins are updated.
If a list of email addresses is pasted into a group’s Add Members form
that has one or more email addresses of users who already belong to the
group and all other email addresses are for users who do not yet exist
on the forum then no invites were being sent. This commit ensures that
we send invites to new users.
This is a try to simplify logic around dismiss new topics to have one solution to work in all places - dismiss all-new, dismiss new in a specific category or even in a specific tag.
This moves all the rate limiting for user second factor (based on `params[:second_factor_token]` existing) to the one place, which rate limits by IP and also by username if a user is found.
Issue originally reported in https://meta.discourse.org/t/bypass-sso-by-adding-unkown-email-to-group/177339
Inviting people via email address to a group when SSO is enabled (or local logins are disabled) led to a situation where user records were being created bypassing single sign-on.
We already prevent that in most places. This adds required checks to `GroupsController`.
This pull requests contains a series of improvements to groups
settings and member management such as:
- Showing which users have set a group as primary
- Moving similar settings together under Effects
- Adding bulk select and actions to members page
Disabling shared drafts used to leave topics in an inconsistent state
where they were not displayed as shared drafts and thus there was no
way of publishing them. Moreover, they were accessible just to users
who have permissions to create shared drafts.
This commit adds another permission check that is used for most
operations and the old can_create_shared_draft? remains used just when
creating a new shared draft.
Adds a new column/setting to groups, allow_unknown_sender_topic_replies, which is default false. When enabled, this scenario is allowed via IMAP:
* OP sends an email to the support email address which is synced to a group inbox via IMAP, creating a group topic
* Group user replies to the group topic
* An email notification is sent to the OP of the topic via GroupSMTPMailer
* The OP has several email accounts and the reply is sent to all of them, or they forward their reply to another email account
* The OP replies from a different email address than the OP (gloria@gmail.com instead of gloria@hey.com for example)
* The a new staged user is created, the new reply is accepted and added to the topic, and the staged user is added to the topic allowed users
Without allow_unknown_sender_topic_replies enabled the new reply creates an entirely new topic (because the email address it is sent from is not previously part of the topic email chain).
Lots of changes but it's mostly a refactoring.
The interesting part that was fix are the 'load_problem_<model>_ids' methods.
They will now return records with no search data associated so they can be properly indexed for the search.
This "bad" state usually happens after a migration.
"I18n.t(key, locale: locale)" fails to find the correct translation in some cases. We should always wrap it with the "I18n.with_locale(locale)" method.
Also, reverting an override wasn't always possible because the serializer always used "I18n.locale" as the locale.
This adds a new table UserNotificationSchedules which stores monday-friday start and ends times that each user would like to receive notifications (with a Boolean enabled to remove the use of the schedule). There is then a background job that runs every day and creates do_not_disturb_timings for each user with an enabled notification schedule. The job schedules timings 2 days in advance. The job is designed so that it can be run at any point in time, and it will not create duplicate records.
When a users saves their notification schedule, the schedule processing service will run and schedule do_not_disturb_timings. If the user should be in DND due to their schedule, the user will immediately be put in DND (message bus publishes this state).
The UI for a user's notification schedule is in user -> preferences -> notifications. By default every day is 8am - 5pm when first enabled.
This should make it easier to track down how the incoming email was created, which is one of four locations:
The POP3 poller (which picks up reply via email replies)
The admin email controller #handle_mail (which is where hosted mail is sent)
The IMAP sync tool
The group SMTP mailer, which sends emails when replying to IMAP topics, pre-emptively creating IncomingEmail records to avoid double syncing
As per @davidtaylorhq 's comment at 6e2be3e#r46069906, this fixes an oversight where if login_required is enabled and an anon user follows a confirm new email link they are forced to login, which is not what the intent of #10830 was.
Admins can now edit translations in different languages without having to change their locale. We display a warning when there's a fallback language set.
This results in some fun disasters if allowed to happen. For now, just issue an oblique error message; a localized message will be added on the client.
Feature for `Must Approve Users` setup. When a user is rejected, a staff member can optionally set a reason for audit purposes. In addition, feedback email can be sent to the user.
Meta: https://meta.discourse.org/t/account-rejection-email/103112/8
It used to change the category of the topic, instead of the destination
category (topic.category_id instead of topic.shared_draft.category_id).
The shared drafts controls were displayed only if the current category
matched the 'shared drafts category', which was not true for shared
drafts that had their categories changed (affected by the previous bug).
Googlebot handles no-index headers very elegantly. It advises to leave as many routes as possible open and uses headers for high fidelity rules regarding indexes.
Discourse adds special `x-robot-tags` noindex headers to users, badges, groups, search and tag routes.
Following up on b52143feff we now have it so Googlebot gets special handling.
Rest of the crawlers get a far more aggressive disallow list to protect against excessive crawling.
This is an edge-case of 9fb3629. An admin could set the shared draft category to one where both TL2 and TL3 users have access but only give shared draft access to TL3 users. If something like this happens, we need to make sure that TL2 users won't be able to see them, and they won't be listed on latest.
Before this change, `SharedDrafts` were lazily created when a destination category was selected. We now create it alongside the topic and set the destination to the same shared draft category.
* FEATURE: Allow categroy group moderators to list/unlist topics
If enabled via SiteSettings, a user belonging to a group which has been granted category group moderator privileges should be able to list/unlist topics belonging to the appropraite category.
25563357 moved the logout redirect logic from the client-side to the server-side. Unfortunately the login_required check was lost during the refactoring which meant that non-login-required sites would redirect to `/login` after redirect, and immediately restart the login process. Depending on the SSO implementation, that can make it impossible for users to log out cleanly.
This commit restores the login_required check, and prevents the potential redirect loop.
We should always hide user_id in response when `hide_email_address_taken` setting is enabled. Currently, it can be used to determine if the email was used or not.
This ensures that users are only served cached content in their own language. This commit also refactors to make use of the `Discourse.cache` framework rather than direct redis access
User directory items are sorted by some activity metric. If those metrics have the same value, postgres does not guarantee the order in which they will be returned. This can cause issues in pagination - some users may appear twice, and some may be missed. To illustrate
```
pry(main)> query = DirectoryItem.where(period_type: DirectoryItem.period_types[:weekly]).order(:likes_received).limit(50);
pry(main)> page1 = query.offset(0).pluck(:id);
pry(main)> page2 = query.offset(50).pluck(:id);
pry(main)> (page1 & page2).count # users on both pages
=> 29
```
If we use the primary key to tie-break matching metrics, things are much more reliable
```
pry(main)> query = DirectoryItem.where(period_type: DirectoryItem.period_types[:weekly]).order(:likes_received, :id).limit(50);
pry(main)> page1 = query.offset(0).pluck(:id);
pry(main)> page2 = query.offset(50).pluck(:id);
pry(main)> (page1 & page2).count # users on both pages
=> 0
```
This most commonly effects new sites where all the directory metrics are zero.
The fact that the ordering is indeterminate makes it difficult to write a reliable test case for this.
- Display reason for validation error when logging in via an authenticator
- Fix email validation handling for 'Discourse SSO', and add a spec
Previously, validation errors (e.g. blocked or already-taken emails) would raise a generic error with no useful information.
At the moment, when filtering by group, the directory will unconditionally return the current user at the top of the list. This is quite unexpected, given that the user is deliberately trying to filter the list. This commit makes sure the 'include current user' logic only triggers for unfiltered directories
Themes marked for auto update will be automatically updated when
Discourse is updated. This is triggered by discourse_docker or
docker_manager running Rake task 'themes:update'.
Allowing the editing of remote themes has been something Discourse has advised against for some time. This commit removes the ability to edit or upload files to remote themes from Admin > Customize to enforce the recommended practice.
Previously, `/u/by-external/{id}` would only work for 'Discourse SSO' systems. This commit adds a new 'provider' parameter to the URL: `/u/by-external/{provider}/{id}`
This is compatible with all auth methods which have migrated to the 'ManagedAuthenticator' pattern. That includes all core providers, and also popular plugins such as discourse-oauth2-basic and discourse-openid-connect.
The new route is admin-only, since some authenticators use sensitive information like email addresses as the external id.
disable_search_queue_threshold needs to be coerced to a float so it is not
treated as a string when sub second values are provided.
Longer term fix is to possibly provide hints in the config so we do the
coersion automatically. However this would be a far more complex change.
There are issues around displaying images on published pages when secure media is enabled. This PR temporarily makes it appear as if published pages are enabled if secure media is also enabled.
* FEATURE - allow category group moderators to delete topics
* Allow individual posts to be deleted
* DEV - refactor for new `can_moderate_topic?` method
Users could be silenced or suspended by two staff members at the same time and
would not be aware of it. This commit shows an error message if another penalty
has been applied.
- IgnoredUser records should all now have an expiring_at value. This commit enforces that in the DB, and fixes any corrupt rows
- Changes to the ignored user list are now handled by the `/u/{username}/notification_level` endpoint. This allows setting expiration dates on the ignore. This commit removes the old logic for saving a list of usernames in the user preferences.
- Many specs were calling `IgnoredUser.create`. This commit changes them to use `Fabricate(:ignored_user)` for consistency
This reverts commit e3de45359f.
We need to improve out strategy by adding a cache breaker with this change ... some assets on CDNs and clients may have incorrect CORS headers which can cause stuff to break.
Previously if a onebox timed out we would not present the users in the log
with any information regarding the onebox. This makes it very difficult to
debug.
This adds url/topic/user in the debugging output.
Adds a new slow mode for topics that are heating up. Users will have to wait for a period of time before being able to post again.
We store this interval inside the topics table and track the last time a user posted using the last_posted_at datetime in the TopicUser relation.
Dependency on gifsicle, allow_animated_avatars and allow_animated_thumbnails
site settings were all removed. Animated GIF images are still allowed, but
the generated optimized images are no longer animated for those (which were
used for avatars and thumbnails).
The added 'animated' is populated by extracting information using FastImage.
This field was used to selectively reoptimize old animations. This process
happens in the background.
DEV: Replace instances of Discourse.base_uri with Discourse.base_path
This is clearer because the base_uri is actually just a path prefix. This continues the work started in 555f467.
See https://meta.discourse.org/t/changing-a-users-email/164512 for additional context.
Previously when an admin user changed a user's email we assumed that they would need a password reset too because they likely did not have access to their account. This proved to be incorrect, as there are other reasons a user needs admin to change their email. This PR:
* Changes the admin change email for user flow so the user is sent an email to confirm the change
* We now record who the email change request was requested by
* If the requested by user is admin and not the user we note this in the email sent to the user
* We also make the confirm change email route open to anonymous users, so it can be clicked by the user even if they do not have access to their account. If there is a logged in user we make sure the confirmation matches the current user.
`BasicGroupSerializer` includes `flair_url` which uses `flair_upload` relation, so the N in N+1 in this case was the number of groups with flair in the forum.
Error messages for exceeded rate limits and invalid parameters always used the English locale instead of the default locale or the current user's locale.
The download link on the lightbox for images was not downloading the image if the upload was marked secure, because the code in the upload controller route was not respecting the dl=1 param for force download.
This PR fixes this so the download link works for secure images as well as regular ligthboxed images.
This allows administrators to stop automatic redirect to an external authenticator. It only takes effect when there is a single authentication method, and the site is login_required
When a category is removed from `auto_watch_category` we are removing
CategoryUser. However, there are still TopicUser with notification level
set to `watching` which was inherited from Category.
We should move them back to `regular` unless they were modified by a user.
These fields are required when using the UI and if `suspend_until`
params isn't used the user never is actually suspended so we should
require these fields when suspending a user.
This commit is addressing an issue where it is possible that there could
be multiple topic timer jobs running to close a topic or a weird race
condition state causing a topic that was just closed to be re-opened.
By removing the logic from the Topic Timer model into the Topic Timer
controller endpoint we isolate the code that is used for setting an
auto-open or an auto-close timer to just that functionality making the
topic timer background jobs safer if multiple are running.
Possibly in the future if we would like this logic back in the model a
refactor will be needed where we actually pass in the auto-close and
auto-open action instead of mixing it with the close and open
action that is currently being passed to the controller.
When someone wants to add > 1000 users at once they will hit a timeout.
Therefore, we should introduce limit and inform the user when limit is exceeded.
Like "default watching" and "default tracking" categories option now the "regular" categories support is added. It will be useful for sites that are muted by default. The user option will be displayed only if `mute_all_categories_by_default` site setting is enabled.
Enabling the moderators_manage_categories_and_groups site setting will allow moderator users to create/manage groups.
* show New Group form to moderators
* Allow moderators to update groups and read logs, where appropriate
* Rename site setting from create -> manage
* improved tests
* Migration should rename old log entries
* Log group changes, even if those changes mean you can no longer see the group
* Slight reshuffle
* RouteTo /g if they no longer have permissions to view group
For sites that are configured to mute some or all categories and tags
for users by default, groups can now be configured to set members'
notification level to normal from the group manage UI.
The category model already has a default value for `color` and
`text_color` so they don't need to be required via the API. The ember UI
already requires that colors be selected.
The name of the category also doesn't need to be required when updating
the category either because we are already passing in the id for the
category we want to change.
These changes improve the api experience because you no longer have to
lookup the category name, color, or text color before updating a single
category attribute. When creating a category the name is still required.
https://meta.discourse.org/t/-/132424/2
Adds functionality to reflect topic delete in Discourse to IMAP inbox (Gmail only for now) and reflecting Gmail deletes in Discourse.
Adding lots of tests, various refactors and code improvements.
When Discourse topic is destroyed in PostDestroyer mark the topic incoming email as imap_sync: true, and do the opposite when post is recovered.
