Some users somehow manage to keep a topic open for a very long time that it causes the post read time to exceed the max integer value (2^31 - 1) which causes errors when we try to update the read time in the database to values above the integer limit.
This PR will cap posts read time at 2^31 - 1 to prevent these errors.
The bug was mentioned on meta: https://meta.discourse.org/t/pressing-dismiss-new-doesnt-clear-new-topics/179858
Problem is that sometimes the user has TopicUser records with `last_read_post_number` set as NULL. In that case, the topic is still "new" to them and should be dismissed when they click dismiss button.
In addition, I added that condition to post_migration and bumped the number to fix existing records. Migration is written to be idempotent so it will make no harm to already deployed instances.
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.
Previously when inheriting category auto-close settings for a topic, those settings were disrupted if another topic timer was assigned or if a topic was closed then manually re-opened.
This PR makes it so that when a topic is manually re-opened the topic auto-close settings are inherited from the category. However, they will now be based on the topic created_at date. As an example, for a topic with a category auto close hours setting of 72 (3 days):
* Topic was created on 2021-02-15 08:00
* Topic was closed on 2021-02-16 10:00
* Topic was opened again on 2021-02-17 06:00
Now, the topic will inherit the auto close timer again and will close automatically at **2021-02-18 08:00**, which is based on the creation date. If the current date and time is greater than the original auto-close time (e.g. we were at 2021-02-20 13:45) then no auto-close timer is created.
Note, this will not happen if the topic category auto-close setting is "based on last post".
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).
Updating a topic's visibility did not increase or decrease the
topic_count of a category, but Category.update_stats does ignore
unlisted topics which resulted in inconsistencies when deleting
such topics.
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
`convert_to_jpeg!` is only called if `convert_png_to_jpeg?` and/or `should_alter_quality?` is true.
`convert_png_to_jpeg?` can be disabled by setting `SiteSetting.png_to_jpg_quality` to 100.
However, `should_alter_quality?` could be true if `SiteSetting.recompress_original_jpg_quality` was lower than the quality of the uploaded file, regardless of file type.
This commits changes `should_alter_quality?` so that uploaded png files will use the `SiteSetting.png_to_jpg_quality` value, rather than ``SiteSetting.recompress_original_jpg_quality` value.
This test failure was caused by rails calling `.debug` on our FakeLogger which was not supporting it, resulting in more errors than what the test was expecting.
* DEV: Show warning message when using ember css selectors
When editing the theme css via the admin UI a warning message
will be displayed if it detects that the `#emberXXX` or `.ember-view`
css selectors are being used. These are dynamic selectors that ember
generates, but they can change so they should not be used.
* Update error message text to be more helpful
* Display a warning instead of erroring out
This allows the theme to still be saved, but a warning is displayed.
Updated the tests to check for the error message.
Updated the pre tags css so that it wraps for long messages.
If no email is provided, email_valid should be set false, so that
Discourse can prompt the user for an email and verify it.
This fixes signups via twitter for accounts with no email address.
Previously we would always take the first image in a post to use as the
thumbnail. On media-heavy sites, users may want to manually select a
specific image as the topic thumbnail. This commit allows this to be
done via a `|thumbnail` attribute in markdown.
For example, in this case, bbb would be chosen as the thumbnail:
```
```
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`.
The bug was mentioned on meta https://meta.discourse.org/t/users-are-seeing-handling-of-unhandled-tag-again/155367
It was related to users who are watching a specific topic. In that case, when the hidden tag was added or removed to the topic they were notified by `NotifyTagChangeJob`.
That job should take hidden tags into consideration. If all changed tags are in a hidden group, it should exclude user not belong to that group.
At the same time, if visible to anyone tag is added or removed users watching topic should be notified.
You can use `discourse restore --location=local FILENAME` if you want to restore a backup that is stored locally even though the `backup_location` has the value `s3`.
* 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
* DEV: Escape backslashes in curl example
We need to escape these backslashes otherwise they get filtered out when
generating the api docs.
* FIX: uniqItems should be uniqueItems
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.
A more general, lower-level change in addition to #11950.
Most code paths already check if SSO is enabled or if local logins are disabled before trying to create an email invite.
This is a safety net to ensure no invalid invites sneak by.
Also includes:
FIX: Don't allow to bulk invite when SSO is on (or when local logins are disabled)
This mirrors can_invite_to_forum? and other email invite code paths.
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
This PR revamps the topic timer UI, using the time shortcut selector from the bookmark modal.
* Fixes an issue where the duration of hours/days after last reply or auto delete replies was not enforced to be > 0
* Fixed an issue where the timer dropdown options were not reloaded correctly if the topic status changes in the background (use `MessageBus` to publish topic state in the open/close timer jobs)
* Moved the duration input and the "based on last post" option from the `future-date-input` component, as it was only used for topic timers. Also moved out the notice that is displayed which was also only relevant for topic timers.
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.
Using "UrlHelper#absolute" returns the S3 URL, which is fine for the client because it modifies it to use the CDN instead. On the other hand, this replacement doesn't happen when the URL is server-side rendered, returning a 403 for the system's avatar.
* document user endpoints, allow for empty request/response bodies
* document more user endpoints, improve debugging output if no details are specified
* document some more user endpoints
* minor cleanup
* FIX: flakey tests due to bad regex
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).
This PR adds security_last_changed_at and security_last_changed_reason to uploads. This has been done to make it easier to track down why an upload's secure column has changed and when. This necessitated a refactor of the UploadSecurity class to provide reasons why the upload security would have changed.
As well as this, a source is now provided from the location which called for the upload's security status to be updated as they are several (e.g. post creator, topic security updater, rake tasks, manual change).
Not when doing a site-wide search like we do in the Directory.
This solves the following specfailure:
1) DirectoryItemsController with data finds user by name
Failure/Error: expect(json['directory_items'].length).to eq(1)
expected: 1
got: 0
(compared using ==)
# ./spec/requests/directory_items_controller_spec.rb:88:in `block (3 levels) in <main>'
# ./spec/rails_helper.rb:271:in `block (2 levels) in <top (required)>'
# ./bundle/ruby/2.7.0/gems/webmock-3.11.1/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
When doing a user search (eg. when mentioning a user) we will not prioritie
users who hasn't been seen in over a year.
REFACTOR the user-search specs to be more precise regarding the ordering
This cookie is only used during login. Having it persist after that can
cause some unusual behavior, especially for sites with short session
lengths.
We were already deleting the cookie following a new signup, but not for
existing users.
This commit moves the cookie deletion logic out of the erb template, and
adds logic and tests to ensure it is always deleted consistently.
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
A simplified version of the logic used in the function before my fix is as follow:
```ruby
result = []
things = [0,1,2,3]
max_values = 2
every = (things.size.to_f / max_values).ceil
things.each_with_index do |t, index|
next unless (t % every) === 0
result << t
end
p result # [0, 2]
# 3 doesn’t get included
```
The problem is that if you get unlucky two times you won't get last tuple(s) and might get a very erroneous date.
Double unlucky:
- last tuple index % computed every !== 0 and you don't get the last tuple
- the last tuple is related to a post with a very different date than the previous tuples (on year difference in our case)
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.
- Read in schemas from actual json files instead of a ruby hash. This is
helpful because we will be automatically generating .json schema files
from json responses and don't want to manually write ruby hash schema
files.
- Create a helper method for rspec schema validation tests to dry up code
* DEV: Add schema checking to api doc testing
This commit improves upon rswag which lacks schema checking. rswag
really only checks that the https status matches, but this change adds
in the json-schema_builder gem which also has schema validation.
Now we can define schemas for each of our requests/responses in the
`spec/requests/api/schemas` directory which will make our documentation
specs a lot cleaner.
If we update a serializer by either adding or removing an attribute the
tests will now fail (this is a good thing!). Also if you change the type
of an attribute say from an array to a string the tests will now fail.
This will help significantly with keeping the docs in sync with actual
code changes! Now if you change how an endpoint will respond you will
have to update the docs too in order for the tests to pass. :D
This PR is inspired by:
https://www.tealhq.com/post/how-teal-keeps-their-api-tests-and-documentation-in-sync
* Swap out json schema validator gem
Swapped out the outdated json-schema_builder gem with the json_schemer
gem.
* Add validation fields to schema
In order to have "strict" validation we need to add
`additionalProperties: false` to the schema, and we need to specify
which attributes are required.
Updated the debugging test output to print out the error details if
there are any.
Improvements to make console access to IncomingEmail more pleasant, and stopping certain IMAP logs from landing in the DB because they just create too much noise,
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
There was an issue that occurred with this order of operations:
* An IMAP topic was created by emailing a group
* A second user was invited to the topic (not the OP and not the group)
* A user with access to the group replies to the topic
* The second user receives a user_private_message notification email because of their involvement in the topic
* The second user replies to the email via email
This new reply would then go and notify the other group PM users, except for those who emailed the group topic directly, which is handled via the group SMTP mailer. However because the new post already has an incoming email because it is parsed via the Email::Receiver via POP3 the group SMTP section of the post alerter is skipped, and the group's email address is not ignored for the user_private_message notification.
This PR fixes it so the group is not ever sent an email via the PM notification. This is important because any new emails in the group's IMAP inbox will be picked up by the Imap::Sync code and created as a new topic which is not at all desirable.
Also in this PR I split up the specs a bit more for group SMTP in the post alerter to make them easier to read and they each only test one thing.
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.
It returned a 429 error code with a 'Retry-After' header if a
RateLimiter::LimitExceeded was raised and unhandled, but the header was
missing if the request was limited in the 'RequestTracker' middleware.
Moves the topic timer jobs from being scheduled ahead of time with enqueue_at to a 5 minute scheduled run like bookmark reminders, in a new job called Jobs::EnqueueTopicTimers. Backwards compatibility is maintained by checking if an existing topic timer job is enqueued in sidekiq for the timer, and if it is not running it inside the new job.
The functionality to close/open a topic if it is in the opposite state still remains in the after_save block of TopicTimer, with further commentary, which is used for Open/Close Temporarily.
This also removes the ensure_consistency! functionality of topic timers as it is no longer needed; the new job will always pick up the timers because they are not stored in a fragile state of sidekiq.
This adds a safe default to not process pop3 emails when the pop3 polling option is set up that are > 1 week old. This is to avoid the situation where an older mailbox is used, which causes us to go and process all emails in that mailbox, sending out error emails to the senders of emails which cannot be parsed successfully.
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.
Transient errors in migration are ignored, silently corrupting
data, and the migration is incomplete and misses many sources of
uploads, which will lead to an incorrect expectation of independence
from the remote object storage after announcing that the migration
was successful, regardles of whether transient errors permanently
corrupted the data.
Remove this migration until such time as it is re-written to
follow the same pattern has the migration to s3, moving the
core logic out of the task.
Fixes bug introduced by bd25627198
What happens is we send notifications to everyone involved in the group inbox topic about new posts, however we pass the param `skip_send_email_to: email_addresses`. In the above commit I removed the group email address from this `email_addresses` array. This breaks the IMAP inbox because we email the group with the reply, and the IMAP sync tool finds this email and opens a new unrelated topic with it.
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.
This PR fixes a race condition with the IMAP notification code. In the `Email::Receiver` we call the `NewPostManager` to create the post and enqueue jobs and sends alerts via `PostAlerter`. However, if the post alerter reaches the `notify_pm_users` and the `group_notifying_via_smtp` method _before_ the incoming email is updated with the post and topic, we unnecessarily send a notification to the person who just posted. The result of this is that the IMAP syncer re-imports the email sent to the user about their own post, which looks like this in the group inbox:
To fix this, we skip the jobs enqueued by `NewPostManager` and only enqueue them with `PostJobsEnqueuer` manually _after_ the incoming email record has been updated with the post and topic.
Other improvements:
* Moved code to calculate email addresses from `IncomingEmail` records into the topic, with a group passed in, for easier testing and debugging. It is not the responsibility of the post alerter to figure this stuff out.
* Add shortcut methods on `IncomingEmail` to split or provide an empty array for to and cc addresses to avoid repetition.
Added GET user by external_id to the api docs.
Fixed `/users/{username}` docs to be `/u/{username}`
Extracted out common user response into a shared helper.
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
* FEATURE: Import attachments
* FEATURE: Add support for importing multiple forums in one
* FEATURE: Add support for category and tag mapping
* FEATURE: Import groups
* FIX: Add spaces around images
* FEATURE: Custom mapping of user rank to trust levels
* FIX: Do not fail import if it cannot import polls
* FIX: Optimize existing records lookup
Co-authored-by: Gerhard Schlager <mail@gerhard-schlager.at>
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
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).
This PR adds functionality for the IMAP sync code to detect if a UID that is missing from the mail group mailbox is in the Spam/Junk folder for the mail account, and if so delete the associated Discourse topic. This is identical to what we do for emails that are moved for Trash.
If an email is missing but not in Spam or Trash, then we mark the incoming email record with imap_missing: true. This may be used in future to further filter or identify these emails, and perhaps go hunting for them in the email account in bulk.
Note: This adds some code duplication because the trash and spam email detection and handling is very similar. I intend to do more refactors/improvements to the IMAP sync code in time because there is a lot of room for improvement.
If a group you're a member of is invited to a PM, you can no longer remove yourself from it. This means you won't be able to remove the message from your inbox, and even if you archive it, it'll come back once someone replies.
Splits the `ToggleTopicClosed` job into two distinct `OpenTopic` and `CloseTopic` jobs to make the code clearer. The old job cannot be deleted yet because of outstanding sidekiq schedules, so a todo has been added to do so later this year.
Also replaced mentions of `topic_status_update` with `topic_timer` in some files, because the `topic_status_update` model is obsolete and replaced by topic timer.
Added some shortcut methods for checking if a topic is open/whether a user can change an open topic.
If the sliding window size is N seconds, then a moment at the Nth second
should be considered as the moment outside of the sliding window.
Otherwise, if the sliding window is already full, at the Nth second,
a new call wouldn't be allowed, but a time to wait before the next call
would be equal to zero, which is confusing.
In other words, the end of the time range shouldn't be included in the
sliding window.
Let's say we start at the second 0, and the sliding window size is 10
seconds. In the current version of rate limiter, this sliding window will
be considered as a time range [0, 10] (including the end of the range),
which actually is 11 seconds in length.
After this fix, the time range will be considered as [0, 10)
(excluding the end of the range), which is exactly 10 seconds in length.
It was a problem because during this operation only the first frame
is kept. This commit removes the alternative solution to check if a GIF
image is animated.
* DEV: TopicTrackingState calls should happen in the background
It was observed that calling TopicTrackingState on popular topics could result in a large number of calls to redis, resulting in slow response times when posting replies.
These calls should be moved to a background job.
* DEV: PostUpdateTopicTrackingState should execute on default queue
Those fail on the buggy i18n release (1.8.6) and pass on 1.8.5, 1.8.7 (the revert release), and with the second stab at thread safety on the current master (63a79cb929)
Fixes a rare race condition causing the `Imap::Sync` class to create an incoming email and associated post/topic, which then kicks off the PostAlerter to notify others in the PM about a reply in the topic, but for the OP which is not necessary (because the person emailing the IMAP inbox already knows about the OP). Basically, we should never be sending the group SMTP email for the first post in a topic.
Also in this PR:
* Custom attribute accessors for the to/from/cc addresses on `IncomingEmail`, to parse them from an array to a joined string so the logic for this is only in one place.
* Store extra detail against the `IncomingEmail` created in `GroupSmtpMailer`
* regex test Mail header Reply-To as string instead of Field, which fixes `warning: deprecated Object#=~ is called on Mail::Field; it always returns nil`
* Add DEBUG_IMAP to log all IMAP logs as warnings for easier debugging
* Changed the Rails logging to `ImapSyncLog` in the `GroupSmtpMailer`
* FIX: Inline onebox should use encoding from Content-Type header when present
* Use Regexp.last_match(1)
Signed-off-by: OsamaSayegh <asooomaasoooma90@gmail.com>
- Only initialize the S3Helper when needed
- Skip initializing the S3Helper for S3Store#cdn_url
- Allow cook_url to be passed a `local` hint to skip unnecessary checks
osts from topics with 'auto delete replies timer' with more than
skip_auto_delete_reply_likes likes will no longer be deleted. If 0,
all posts will be deleted.
Final sigma is not lower cased correctly in Ruby causing issues with routing.
This works around the issue by downcasing all usernames containing a sigma using JS.
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.
My initial implementation didn't consider this case. We should skip imported users if the "imported_id" field is present, even if there're other custom fields.
