All the data we need for the `info` and `credentials` auth hash
are obtained via the user info API, not the JWT. Using and verifying
the JWT can fail due to clock skew, so let's skip it completely.
PR opened to fix the upstream issue at https://github.com/zquestz/omniauth-google-oauth2/pull/392
Previously it matched the behavior of standard ActiveRecord after_commit callbacks. They do not work well within `joinable: false` nested transactions. Now `DB.after_commit` callbacks will only be run when the outermost transaction has been committed.
Tests always run inside transactions, so this also introduces some logic to run callbacks once the test-wrapping transaction is reached.
* DEV: More robust processing of URLs
The previous `UrlHelper.encode_component(CGI.unescapeHTML(UrlHelper.unencode(uri))` method would naively process URLs, which could result in a badly formed response.
`Addressable::URI.normalized_encode(uri)` appears to deal with these edge-cases in a more robust way.
* DEV: onebox should use UrlHelper
* DEV: fix spec
* DEV: Escape output when rendering local links
You can let non-staff users use shared drafts by modifying the `shared_drafts_min_trust_level` site setting. These users must have access to the shared draft category.
* FEATURE: Allow Category Group Moderators to edit topic titles
Adds category group moderators to the topic guardian’s `can_edit` method.
The value of `can_edit` is returned by the topic view serializer, and this value determines whether the current user can edit the title/category/tags of the topic directly (which category group moderators could already do by editing the first post of a topic).
Note that the value of `can_edit` is now always returned by the topic view serializer (ie, for both true and false values) to cover the case where a topic is moved out of a category that a category group moderator has permissions on, so that when the topic is reloaded the UI picks up that `can_edit` is now false, and thus the edit icon should no longer be displayed.
* DEV: Add a comment explaining why `can_edit` is always returned
If a category and a sub-category have the same slug, adding a background to one of them will also show it on the other one. This was introduced in 8e3f667 to fix a discrepancy, which was later fixed in 214b4c3.
We can't use erb in ember-cli, and it seems the emoji groups rarely
change anyway. This commit migrates the ERB to pre-rendered javascript
that is updated via the `rake javascript:update_constants` task.
- frowning was using slighty_frowning
- slightly_frowning was using frowning
- grinning_face_with_smiling_eyes was not defined
- fronwing_face_with_open_mouth was not defined
* FEATURE: onebox for local categories
This commit adjusts the category onebox to look more like the category boxes do on the category page.
Co-authored-by: Jordan Vidrine <jordan@jordanvidrine.com>
Force pushing a commit to a theme repository used to break the updater,
because the system was not able to count the commits behind the old and
new version. This operation failed because a force push deleted the old
commits.
The user was prompted with a simple "500 server error" message.
When calculating whether the attached uploads went over the SiteSetting.email_total_attachment_size_limit_kb.kilobytes limit, we were using the original_upload for the calculation instead of the actually attached_upload, which will be smaller in most cases because it can be an optimized image.
This adds a new min_trust_level_to_allow_ignore site setting that enables admins to control the point at which a user is allowed to ignore other users.
After the search term is parsed for advanced search filters, the term may
become empty. Later, the same term will be passed to Discourse.route_for
which will raise an ArgumentError.
> URI(nil)
ArgumentError: bad argument (expected URI object or URI string)
Partial revert of b143412be4 (the refactoring is not reverted)
This broke a few things, so we need to investigate and make some changes before reinstating the error
Hopefully this will prevent fellow developers from spending a morning
trying to figure out why the jobs that are run after a post is created
are not finding the post in the database.
That's because if you're inside a transaction when creating a post, the jobs
will likely run before the transaction is being commited to the database.
There was a "warning" in the comments of the PostCreator class but raising an
exception should hopefully help catch those issue much faster :fingers_crossed:.
This is hard to test since all the tests are run in a transaction...
* FEATURE: display error if Oneboxing fails due to HTTP error
- display warning if onebox URL is unresolvable
- display warning if attributes are missing
* FEATURE: Use new Instagram oEmbed endpoint if access token is configured
Instagram requires an Access Token to access their oEmbed endpoint. The requirements (from https://developers.facebook.com/docs/instagram/oembed/) are as follows:
- a Facebook Developer account, which you can create at developers.facebook.com
- a registered Facebook app
- the oEmbed Product added to the app
- an Access Token
- The Facebook app must be in Live Mode
The generated Access Token, once added to SiteSetting.facebook_app_access_token, will be passed to onebox. Onebox can then use this token to access the oEmbed endpoint to generate a onebox for Instagram.
* DEV: update user agent string
* DEV: don’t do HEAD requests against news.yahoo.com
* DEV: Bump onebox version from 2.1.5 to 2.1.6
* DEV: Avoid re-reading templates
* DEV: Tweaks to onebox mustache templates
* DEV: simplified error message for missing onebox data
* Apply suggestions from code review
Co-authored-by: Gerhard Schlager <mail@gerhard-schlager.at>
This moves the library into our lib folder, and refactored it to more
modern Javascript. I've kept the MIT license at the top of the file.
Doing this allows us to import it as a library in Ember CLI and ditch
yet another global variable.
Previously thumbnails were only preloaded for queries using `TopicQuery#default_results`, which meant that requests for PM topic lists would lead to N+1 queries.
This commit moves the preloading into TopicList#load_topics, along with other similar preloads (e.g. plugin custom fields)
The direct call to `ActiveRecord::Associations::Preloader#preload` is necessary because `@topics` can be an array, not an `ActiveRecord::Relation`
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'.
CookedPostProcessor replaces all large images with their optimized
versions, but for GIF images the optimized version is limited to first
frame only. This caused animations it cooked posts to require a click
to show up the lightbox and start playing.
Animated emojis were converted to static images. This commit moves the
responsability on site admins to optimize their animated emojis before
uploading them (gifsicle is no longer used).
Plugin client.en.yml and server.en.yml can now be client/server-(1-100).en.yml. 1 is the lowest priority, and 100 is the highest priority. This allows plugins to set their priority higher than other plugins, so that they can override eachothers' translations.
It used to simply say "title is invalid" without giving any hint what
the problem could be. This commit adds different errors messages for
all caps titles, low entropy titles or titles with very long words.
- Bump rails_failover for new per-backend callback feature
- If the master backend fails over, make all sites readonly. And vice-versa for fallback
- If a single backend fails over, make that individual site readonly. And vice-versa for fallback
- When a single backend fails, also check connection to the master backend
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.
This commit adds an additional find_user_by_email hook to ManagedAuthenticator so that GitHub login can continue to support secondary email addresses
The github_user_infos table will be dropped in a follow-up commit.
This is the last core authenticator to be migrated to ManagedAuthenticator 🎉
PostDestroyer should accept the option to permanently destroy post from the database. In addition, when the first post is destroyed it destroys the whole topic.
Currently, that feature is limited to private messages and creator of the post. It will be used by discourse-encrypt to explode encrypted private messages.
When embedding secure images which have been oneboxed, we checked to see if the image's parent's parent had the class onebox-body. This was not always effective as if the image does not get resized/optimized then it does not have the aspect-image div wrapping it. This would cause the image to embed in the email but be huge.
This PR changes the check to see if any of the image's ancestors have the class onebox-body, or if the image has the onebox-avatar class to account for variations in HTML structure.
Per Google, sites are encouraged to upgrade from Universal Analytics v3 `analytics.js` to v4 `gtag.js` for Google Analytics tracking. We're giving admins the option to stay on the v3 API or migrate to v4. Admins can change the implementation they're using via the `ga_version` site setting. Eventually Google will deprecate v3, but our implementation gives admins the choice on what to use for now.
We chose this implementation to make the change less error prone, as many site admins are using custom events via the v3 UA API. With the site stetting defaulted to `v3_analytics`, site analytics won't break until the admin is ready to make the migration.
Additionally, in the v4 implementation, we do not enable automatic pageview tracking (on by default in the v4 API). Instead we rely on Discourse's page change API to report pageviews on transition to avoid double-tracking.
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
Previous to this change our anonymous rate limits acted as a throttle.
New implementation means we now also consider rate limited requests towards
the limit.
This means that if an anonymous user is hammering the server it will not be
able to get any requests through until it subsides with traffic.
See #10794 for original context.
I did not mean to add invite to the BYPASS_TYPES for Email::Sender, it was supposed to be invite_password_instructions.
A site owner attempting to use both the email_subject site setting and translation overrides for normal post notification
email subjects would find themselves frusturated at the lack of template argument parity.
Make all the variables available for translation overrides by adding the subject variables to the custom interpolation keys list and applying them.
Reported at https://meta.discourse.org/t/customize-subject-format-for-standard-emails/20801/47?u=riking
This commit adds a site setting `auto_close_topics_create_linked_topic`
which when enabled works in conjunction with `auto_close_topics_post_count`
setting and creates a new linked topic for the topic just closed.
The auto-created new topic contains a link for all the previous topics
and the topic titles are appended with `(Part {n})`.
The setting is enabled by default.
We had an issue where onebox thumbnail was too large and thus was optimized, and we are using the image URLs in post to redact and re-embed, based on the sha1 in the URL. Optimized image URLs have extra stuff on the end like _99x99 so we were not parsing out the sha1 correctly. Another issue I found was for posts that have giant images, the original was being used to embed in the email and thus would basically never get included because it is huge.
For example the URL 787b17ea61_2_690x335.jpeg was not parsed correctly; we would end up with 787b17ea6140f4f022eb7f1509a692f2873cfe35_2_690x335.jpeg as the sha1 which would not find the image to re-embed that was already attached to the email.
This fix will use the first optimized image of the detected upload when we are redacting and then re-embedding to make sure we are not sending giant things in email. Also, I detect if it is a onebox thumbnail or the site icon and force appropriate sizes and styles.
We remove the slow mode composer message and provide better messages when rejecting new posts and edits. The client now validates if the user tries to post again immediately. Finally, we replaced the `hourglass-end` icon with the `hourglass-start` one.
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.
There is a site setting reply_by_email_enabled which when combined with reply_by_email_address creates a Reply-To header in emails in the format "test+%{reply_key}@test.com" along with a PostReplyKey record, so when replying Discourse knows where to route the reply.
However this conflicts with the IMAP implementation. Since we are sending the email for a group via SMTP and from their actual email account, we want all replys to go to that email account as well so the IMAP sync job can pick them up and put them in the correct place. So if the group has IMAP enabled and configured, then the reply-to header will be correct.
This PR also makes a further fix to 64b0b50 by using the correct recipient user for the PostReplyKey record. If the post user is used we encounter this error:
if destination.user_id != user.id && !forwarded_reply_key?(destination, user)
raise ReplyUserNotMatchingError, "post_reply_key.user_id => #{destination.user_id.inspect}, user.id => #{user.id.inspect}"
end
This is because the user above is found from the from_address, but the destination which is the PostReplyKey is made by the post.user, which will be different people.
When plugin spec is evaluated for the first time, it took 30 seconds to run:
```
rm -rf tmp/* && LOAD_PLUGINS=1 be rspec ./plugins/discourse-solved/spec/requests/topics_controller_spec.rb
```
Applying sprocket patch in test environment solves that issue
On forums with a large amount of posts when a user had a bookmark in the topic, PostgreSQL was using an inefficient query plan to fetch the first post of the topic. When running this ActiveRecord query:
```
topic.posts.with_deleted.where(post_number: 1).first
```
The following query plan was produced:
```
Limit (cost=0.43..583.49 rows=1 width=891) (actual time=3850.515..3850.515 rows=1 loops=1)
-> Index Scan using posts_pkey on posts (cost=0.43..391231.51 rows=671 width=891) (actual time=3850.514..3850.514
rows=1 loops=1)
Filter: ((topic_id = 160918) AND (post_number = 1))
Rows Removed by Filter: 2274520
Planning time: 0.200 ms
Execution time: 3850.559 ms
(6 rows)
```
The issue here is the combination of ORDER BY and LIMIT causing the ineficcient Index Scan using posts_pkey on posts to be used. When we correct the AR call to this:
```
topic.posts.with_deleted.find_by(post_number: 1)
```
We end up with a query that still has a LIMIT but no ORDER BY, which in turn creates a much more efficient query plan:
```
Limit (cost=0.43..1.44 rows=1 width=891) (actual time=0.033..0.034 rows=1 loops=1)
-> Index Scan using index_posts_on_topic_id_and_post_number on posts (cost=0.43..678.82 rows=671 width=891) (actua
l time=0.033..0.033 rows=1 loops=1)
Index Cond: ((topic_id = 160918) AND (post_number = 1))
Planning time: 0.167 ms
Execution time: 0.072 ms
(5 rows)
```
This query plan uses the correct index, `Index Scan using index_posts_on_topic_id_and_post_number on posts`. Note that this is only a problem on forums with a larger amount of posts; tiny forums would not notice the difference. On large forums a query for a topic that takes 1s without a bookmark can take 8-30 seconds, and even end up with 502 errors from nginx.
* FEATURE - Add SiteSettings to control JPEG image quality
`recompress_original_jpg_quality` - the maximum quality of a newly
uploaded file.
`image_preview_jpg_quality` - the maximum quality of OptimizedImages
* FEATURE: allow category group moderators to edit posts
If the `enable_category_group_moderation` SiteSetting is enabled, posts should be editable by those belonging to the appropraite groups.
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.
`max-width: 50%; max-height: 400px;` is a good fallback, however, if width and height are given and are smaller than fallback - we should persist that smaller size.
Our Email::Sender class accepts an optional user argument, which is used to create a PostReplyKey record when present. This record is used to sub out the %{reply_key} placeholder in the Reply-To mail header, so if we do not pass in the user we get a broken Reply-To header.
This is especially problematic in the IMAP group SMTP situation, because these emails go to customers that we are replying to, and when they reply to us the email bounces! This fixes the issue by passing user to the Email::Sender when sending a group_smtp email but there is still more to do in another PR.
This Email::Sender optional user is a bit of a footgun IMO, especially because most of the time we use it there is a user we can source. I would like to do another PR for this after this one to make the parameter not optional, so we don't end up with these reply issues down the line again.
This was made adjustable to allow rolling back quickly if problems came
up. The new behaviour was made default in 93137066 and no problems with
this have been reported.
This consolidates logic used to match routes in ApiKey, UserApiKey and DefaultCurrentUserProvider. This reduces duplicated logic, and will allow UserApiKeysScope to easily re-use the parameter matching logic from ApiKeyScope
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.
It pauses Sidekiq, clears Redis (namespaced to the current site), clears Sidekiq jobs for the current site, restores the database and unpauses Sidekiq. Previously it stayed paused until the end of the restore.
Redis is cleared because we don't want any old data lying around (e.g. old Sidekiq jobs). Most data in Redis is prefixed with the name of the multisite, but Sidekiq jobs in a multisite are all stored in the same keys. So, deleting those jobs requires a little bit more logic.
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.
Limit git log output to the first line of the commit message
when generating the list of commits in a release. Some commit messages
are including the names of all commits that were squashed, resulting
in duplicate and confusing lines in the release notes.
discourse-perspective-api was not successfully running tests via the
qunit:test rake task due to inconsistent naming between core and the
repo. As a result we no longer need the mapping in the plugin rake task, too.
Now that we have support for user-selectable color schemes, it makes sense
to simplify seeding and theme updates in the wizard.
We now:
- seed only one theme, named "Default" (previously "Light")
- seed a user-selectable Dark color scheme
- rename the "Themes" wizard step to "Colors"
- update the default theme's color scheme if a default is set
(a new theme is created if there is no default)
Per Google, sites are encouraged to upgrade from `analytics.js` to `gtag.js` for Google Analytics tracking. This commit updates core Discourse to use the new `gtag.js` API Google is asking sites to use. This API has feature parity with `analytics.js` but does not use trackers.
Creates a BabelHelper builder using a default list of plugins, to ensure the transpiled code is always using the same plugins instead of differents plugins in different cases.
When the server gets overloaded and lots of requests start queuing server
will attempt to shed load by returning 429 errors on background requests.
The client can flag a request as background by setting the header:
`Discourse-Background` to `true`
Out-of-the-box we shed load when the queue time goes above 0.5 seconds.
The only request we shed at the moment is the request to load up a new post
when someone posts to a topic.
We can extend this as we go with a more general pattern on the client.
Previous to this change, rate limiting would "break" the post stream which
would make suggested topics vanish and users would have to scroll the page
to see more posts in the topic.
Server needs this protection for cases where tons of clients are navigated
to a topic and a new post is made. This can lead to a self inflicted denial
of service if enough clients are viewing the topic.
Due to the internal security design of Discourse it is hard for a large
number of clients to share a channel where we would pass the full post body
via the message bus.
It also renames (and deprecates) triggerNewPostInStream to triggerNewPostsInStream
This allows us to load a batch of new posts cleanly, so the controller can
keep track of a backlog
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Previously, any errors in those files would e.g. blow up the update process in docker_manager.
Now it prints out an error and proceeds as if there was no compatibility file.
Includes:
* DEV: Extract setup_git_repo
* DEV: Use `Dir.mktmpdir`
* DEV: Default to `main` branch (The latest versions of git already do this, so to avoid problems do this by default)
* FIX: second factor cannot be enabled if SSO is enabled
If `enable_sso` setting is enabled then admin should not be able to
enable `enforce_second_factor` setting as that will lock users out.
Co-authored-by: Robin Ward <robin.ward@gmail.com>
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.
We can't use erb in Ember CLI (since it does not have Ruby) so this has
been ported to use our `javascript:update_constants` rake test instead.
Note we don't have to run this every time a notification type as it's
only used by fixtures to fill in some specific types we test against.
See https://meta.discourse.org/t/email-address-change-confirmation-email-not-sent-but-every-other-notification-emails-are/165358
In short: with disable emails set to non-staff, email address change confirmation emails (those sent to the new address) are not sent for staff or admin members.
This was happening because we were looking up the staff user with the to_address of the email, but the to address was the new email address because we are sending a confirm email change email, and thus the user could not be found. We didn't need to do this anyway because we are passing the user into the Email::Sender class anyway.
* PERF: avoid lookbehinds when indexing search
Previously we used a `EmailCook.url_regexp` this regex used lookbehinds
Unfortunately certain strings could lead to pathological behavior causing
CPU to skyrocket and regex replace to take a very very long time.
EmailCook still needs a fix, but it is less urgent cause it already splits
to single lines. That said we will correct that as well in a seperate PR.
New implementation is far more naive and relies on the extra spaces search
indexer inserts.
When that site setting is enabled, the category counts (new/unread)
include the subcategory definition topics, but the topics aren't included
in the list. This fixes that discrepancy.
This plugin is only useful for developers, however, making it core allows us to centralize any component modification in one commit.
This integration also adds a new site_setting: `styleguide_admin_only` which allows to enable a styleguide on a live site while restricting visibility to admins only.
By default, styleguide is disabled.
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.
Allows site administrators to pick different fonts for headings in the wizard and in their site settings. Also correctly displays the header logos in wizard previews.
When enable_personal_messages was disabled, moderators could not see
the private messages for the "moderators" group. The link was displayed
on the client side, but the checks on the server side did not allow it.
This is where they should be as far as ember is concerned. Note this is
a huge commit and we should be really careful everything continues to
work properly.
UploadRecovery only worked on missing Upload records. Now it also works with existing ones that have an invalid_etag status.
The specs (first that test the S3 path) are a bit of stub-a-palooza, but that's how much this class interacts with the the outside world. 🤷♂️
Upload.secure_media_url? raised an exceptions when the URL was invalid,
which was a issue in some situations where secure media URLs must be
removed.
For example, sending digests used PrettyText.strip_secure_media,
which used Upload.secure_media_url? to replace secure media with
placeholders. If the URL was invalid, then an exception would be raised
and left unhandled.
Now instead in UrlHelper.rails_route_from_url we return nil if there is something wrong with the URL.
Co-authored-by: Bianca Nenciu <nenciu.bianca@gmail.com>
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.
See https://meta.discourse.org/t/changing-a-users-email/164512 for context.
When admin changes an email for a user, we were incorrectly sending the password reset email to the user's old address. Also the new email does not come into effect until the reset password process is done, so this PR adds some notes to the admin to make this clearer.
