### Background
When SSRF detection fails, the exception bubbles all the way up, causing a log alert. This isn't actionable, and should instead be ignored. The existing `rescue` does already ignore network errors, but fails to account for SSRF exceptions coming from `FinalDestination`.
### What is this change?
This PR does two things.
---
Firstly, it introduces a common root exception class, `FinalDestination::SSRFError` for SSRF errors. This serves two functions: 1) it makes it easier to rescue both errors at once, which is generally what one wants to do and 2) prevents having to dig deep into the class hierarchy for the constant.
This change is fully backwards compatible thanks to how inheritance and exception handling works.
---
Secondly, it rescues this new exception in `UserAvatar.import_url_for_user`, which is causing sporadic errors to be logged in production. After this SSRF errors are handled the same as network errors.
This table holds associations between uploads and other models. This can be used to prevent removing uploads that are still in use.
* DEV: Create upload_references
* DEV: Use UploadReference instead of PostUpload
* DEV: Use UploadReference for SiteSetting
* DEV: Use UploadReference for Badge
* DEV: Use UploadReference for Category
* DEV: Use UploadReference for CustomEmoji
* DEV: Use UploadReference for Group
* DEV: Use UploadReference for ThemeField
* DEV: Use UploadReference for ThemeSetting
* DEV: Use UploadReference for User
* DEV: Use UploadReference for UserAvatar
* DEV: Use UploadReference for UserExport
* DEV: Use UploadReference for UserProfile
* DEV: Add method to extract uploads from raw text
* DEV: Use UploadReference for Draft
* DEV: Use UploadReference for ReviewableQueuedPost
* DEV: Use UploadReference for UserProfile's bio_raw
* DEV: Do not copy user uploads to upload references
* DEV: Copy post uploads again after deploy
* DEV: Use created_at and updated_at from uploads table
* FIX: Check if upload site setting is empty
* DEV: Copy user uploads to upload references
* DEV: Make upload extraction less strict
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.
This reverts commit 20780a1eee.
* SECURITY: re-adds accidentally reverted commit:
03d26cd6: ensure embed_url contains valid http(s) uri
* when the merge commit e62a85cf was reverted, git chose the 2660c2e2 parent to land on
instead of the 03d26cd6 parent (which contains security fixes)
Previously the image was imported from a Discourse hosted CDN but the
URL has since become invalid. However, it was not caught since all
errors are rescued. This commit fixes the issue by shipping the user
avatar with the plugin.
Users noticed that sometimes, avatar from Gravatar is not correctly updated - https://meta.discourse.org/t/updated-image-on-gravatar-not-seeing-it-update-on-site/54357
A potential reason for that is that even if you update your avatar in Gravatar, URL stays the same and if the cache is involved, service is still receiving the old photo.
For example. In my case, when I click the button to refresh avatar the
new Upload record is created with `origin` URL to new avatar, and `url` to
old one
I made some tests in the rails console and adding random param to Gravatar URL is deceiving cache and correct, the newest avatar is downloaded
Adds 3 config values that allow to set a custom provider of Gravatar-like API accessible from gravatar_base_url. The gravatar_name is purely cosmetic, but helps with associating name with the service that actually provides the avatars. gravatar_login_url is a link relative to gravatar_base_url, which provides the user with the login to the Gravatar service
Zeitwerk simplifies working with dependencies in dev and makes it easier reloading class chains.
We no longer need to use Rails "require_dependency" anywhere and instead can just use standard
Ruby patterns to require files.
This is a far reaching change and we expect some followups here.
This reduces chances of errors where consumers of strings mutate inputs
and reduces memory usage of the app.
Test suite passes now, but there may be some stuff left, so we will run
a few sites on a branch prior to merging
It seems that due to jobs being asynchronous and wrapping code in a
DistributedMutex that by the time we run the
`UserAvatar#update_gravatar!` job that the user/user email might be
destroyed.
This patch checks before a call to `user.email_hash` to make sure
the user and primary email exist to prevent the exception. If not
present, the job exits as there's nothing to do because we are
probably running after the user was destroyed for some reason.
previously we would ignore socket error, but this would mean that
there could be conditions where we would keep trying to download
gravatars forever (in an hourly job)
* `rescue nil` is a really bad pattern to use in our code base.
We should rescue errors that we expect the code to throw and
not rescue everything because we're unsure of what errors the
code would throw. This would reduce the amount of pain we face
when debugging why something isn't working as expexted. I've
been bitten countless of times by errors being swallowed as a
result during debugging sessions.
FIX: warning about popup dimensions when using facebook login
Rules are:
- On account creation we always import
- If you already have an avatar uploaded, nothing is changed
- If you have no avatar uploaded, we upload from facebook on login
- If you have no avatar uploaded, we select facebook unless gravatar already selected
This also fixes SSO issues where on account creation accounts had missing avatar uploads
