refs https://github.com/TryGhost/Toolbox/issues/503
- There was an error thrown due to empty "model._changed" field
- When attached or detached events (e.g. tag.attached) are sent through, their models do not contain any _changed properties. This was taken into account when checking for route related resource changes
refs https://github.com/TryGhost/Toolbox/issues/503
- The listener was not covered during quick and dirty implementation. While in the area did some cleanup to the sitemap manager test
- One of the problems I've stumbled upon when adding a test is having multiple instances of SiteManager in the test, which in turn created multiple "subscribe" events and repeat handle executions. Fixed it by having just one site manager instance (a singleton) as that's the pattern that used in main codebase
refs https://github.com/TryGhost/Toolbox/issues/503
- Full URL regeneration process was happening even when only unrelated to URL generation fields were updated (e.g. 'plaintext' change in post does not affect the URL of the post). Stopping the "resource updated" event processing early circumvents full url regeneration inside of DynamicRouting, which can be quite heavy depending on routing configuration
- The URLResourceUpdatedEvent is supposed to be emmited whenever there's an update to the resource already associated with the URL and no url-affecting fields were touched.
no issue
Tests stopped working because the Mailgun mocker stopped working since we moved to the new email flow.
This also fixes a unit test that needed to get updated.
fixes https://github.com/TryGhost/Team/issues/2432
Adds outbound_link_tagging setting (enabled by default and behind
feature flag). If the feature flag is enabled, and the setting is
disabled, we won't add ?ref to links in emails.
This includes new E2E tests for email click tracking, which were also
extended to check outbound link tagging (for both MEGA and the new email
stability flow).
Also fixes a test fixture for the comments_enabled setting.
fixes https://github.com/TryGhost/Team/issues/2461
- Ignores 'edited' links when there is only one second differences.
- Make sure we don't set updatedAt when linking a post to a redirect
This allows us to share the implementation with other parts of the codebase, the
specific usecase here being fetching the metadata from webmention sources, for
display in the mentions UI, which will be borrowing a lot of stuff from the
bookmark card.
refs acf0baa8c7
Due to the bump in express-test, we now handle string bodies 'properly'. So they now pass all the Express middlewares. In the past this failing test did not really pass by the bodyParser.raw middleware,
so the content-type check on the `bodyParser.raw({type: 'application/json'})` middleware was not executed. Now it is, and the test fails because the content-type header was not set to application/json.
refs https://github.com/TryGhost/Team/issues/2400
- we've deemed it useful to start to return `Content-Version` for all
API requests, because it becomes useful to know which version of Ghost
a response has come from in logs
- this should also help us detect Admin<->Ghost API mismatches, which
was the cause of a bug recently (ref'd issue)
refs https://github.com/TryGhost/Toolbox/issues/499
- The mockManager's sentEmailCount is left here to avoid breaking many tests that already depend on this method. With future improvements to email snapshot tests this method should not be used. Instead, emailMockReceiver's own sentEmailCount method should be used directly.
refs https://github.com/TryGhost/Toolbox/issues/499
refs 6bcc47a0ad
- Using module directly caused issues with snapshots manager instance initialization (mocha hooks did not apply to a correct instance)
- See refed commit for more
refs https://github.com/TryGhost/Toolbox/issues/499
- Outgoing emails have been a weak point of Ghost's stability recently. The concept of "emailMockReceiver" similarly to "webhookMockReceiver", allows to test side-effects like outgoing emails.
- This is a first iteration which should lay groundwork for testing all outgoing emails in the future
- The change adds a new concept of "email mock receiver" which is very similar to how the "webhook mock receiver" works. The email mock receiver exposes two methods to record and verify snapshots:
- matchHTMLSnapshot - records and verifies only the HTML content of the outgoint email
- matchMetadataSnapshot - records and verifies all the non-HTML properties sent along an email content, e.g.: to address, plaintext, subject, etc.
- What's missing is matching content based on dynamic content like dates, links with JWT tokens, etc.
We've wrapped both changes in a try/catch to make sure this has no
adverse affects. The endpoint currently doesn't exist - we're only
adding this to get an idea of how much traffic we'll expect to see.
Long term we'll want to read the endpoint from the webmention service.
This introduces the new suppressions feature which will automatically
unsubscribe members from newsletters when their email is added to the
suppression list in Mailgun, this is usually due to emails either
permanently bouncing to the address, or the member making a spam
complaint.
Both Members and Admins are able to see that the email has been added to
the list, and Members are be able to request their email be removed from
the list via Portal.
Overall this feature should improve delivery rates of newsletters and
improve the rating of the domain you're sending from.
closes https://github.com/TryGhost/Team/issues/2338
If a site has the Free tier hidden from the Portal, and subsequently the Stripe connection is disconnected, this produces a dead-end state where no new members can sign up and the Free tier cannot be reactivated again in Portal settings as its hidden. This change -
- enables free tier toggle to be always shown on site irrespective of Stripe connection
fixes https://github.com/TryGhost/Team/issues/2339
The email service is now fully covered by tests, and this commit also forces the test coverage to remain 100% after future changes.
closes https://github.com/TryGhost/Team/issues/2011
- Gives publishers the ability to filter members based on which offer they used (redeemed) when they subscribed for a paid membership.
- On the offers page, the redemption count number links to a the members page with the filter already applied making it easy to have insight on which members used the offer / coupon.
We're seeing behaviour from Mailgun where permanent failures with a
5xx error code are not being added to their internal suppression list,
which is resulting in the Ghost list becoming out of sync with
Mailgun.
Rather than adding emails to the suppression list when Mailgun does,
we're instead going to add emails _after_ Mailgun does, by waiting for
an error code which tells us the email is already on the suppression
list.
Those codes are 605 for previous bounces and 607 for previous spam complaints.
fixes https://github.com/TryGhost/Team/issues/2398
There was an error when fetching the existing email recipient failure. It ended up matching all recipient failures. The result was that only one failure was stored in the database.
refs https://github.com/TryGhost/Team/issues/2371
- cleans up and adds comments for portal playwright tests
- updates data test attributes for portal trigger and popup selectors for consistency
- updates data attribute usage for offers
refs https://github.com/TryGhost/Team/issues/2371
- in case all tiers are archived before new tier is created, the add tier section can be collapsed and will need to be opened first before going through add tier flow
refs https://github.com/TryGhost/Team/issues/2393
- During boot and loading the active theme, we now cache the result of
the gscan validation. Cache configuration can happen in
`adapters.cache.gscan`
- We now also return non-fatal errors when activating or adding a theme.
- When the `themeErrorsNotification` feature flag is on, we fetch the
active theme (which includes the validation information) when loading
admin
- If the currently active theme has errors, we show an error
notification that can open the error modal
- Added a new endpoint: `/ghost/api/admin/themes/active/` that returns
the result of the last gscan validation of the active theme. If no cache
is available, it will run a new gscan validation.
- Added new permissions for the active action/endpoint (author, editor,
administrator)
refs https://github.com/TryGhost/Toolbox/issues/497
- During gscan fatal error downgrade to non-fatal some of the deprecated helpers were a bit vague to debug with no information on which exact "resource" was invalid
- Added resource name to the log for clarity. Should make life easier when debugging potential get helper misuses
- the test was using incorrect test state that was copied over from adding label test
- also adds guard for empty newsletters in member filters as in some cases it might not exist as found by test
When Mailgun fails to deliver an email to an address because the
address has already bounced before, it gives us a permanent fail event
with a 605 error code rather than a 5xx one. Because we want to
"backfill" our suppressions data with previously bounced email
addresses, we want to handle this specific error code.
We may update this logic in the future based on new information from
Mailgun with respect to their 6xx error codes and the
meanings/underlying cause of theme.
This also moves the tests which check for whether or not emails are
suppressed into their own fail so that we do not pollute the event
storage tests, and adds more tests cases.
We also fix a leaky sinon stub which we were not resetting in the email
event storage tests
The email_recipient fixtures were using duplicate and mismatched email addresses
rather than having them correctly map to the Members, which is required for testing
email suppressions.
no issue
With the increased usage of DomainEvents, it gets harder to build
reliable tests without having to resort to timeouts. This utility method
allows us to wait for all events to be processed before continuing with
the test.
This change should speed up tests and make them more reliable.
It only adds extra code when running tests and shouldn't impact
production.
closes https://github.com/TryGhost/Team/issues/2361
If a free trial tier existed on site and its set to 'Invite only' in membership settings, the free trial copy still showed on portal.
- removes free trial copy from portal if site is invite only
- adds playwright test to make sure free trial copy is not shown for invite only sites
There are currently two issues with the suppressions table:
- We have some incorrect rows
- We have missing UNIQUE constraints
We want to completely wipe the tables and start fresh, as well as make
sure that the UNIQUE constraints are added, so we drop the table
completely, and then re-add it, which should result in an empty
suppressions table with all expected constraints.
We've also renamed the `email_address` column to `email` to match our
`users` & `members` tables
fixes https://github.com/TryGhost/Team/issues/2366
refs https://ghost.slack.com/archives/C02G9E68C/p1670232405014209
Probem described in issue.
In the old MEGA flow:
- The `email_verification_required` check is now repeated inside the job
In the new email service flow:
- The `email_verification_required` is now checked (didn't happen
before)
- When generating the email batch recipients, we only include members
that were created before the email was created. That way it is
impossible to avoid limit checks by inserting new members between
creating an email and sending an email.
- We don't need to repeat the check inside the job because of the above
changes
Improved handling of large imports:
- When checking `email_verification_required`, we now also check if the
import threshold is reached (a new method is introduced in
vertificationTrigger specifically for this usage). If it is, we start
the verification progress. This is required for long running imports
that only check the verification threshold at the very end.
- This change increases the concurrency of fastq to 3 (refs
https://ghost.slack.com/archives/C02G9E68C/p1670232405014209). So when
running a long import, it is now possible to send emails without having
to wait for the import. Above change makes sure it is not possible to
get around the verification limits.
Refactoring:
- Removed the need to use `updateVerificationTrigger` by making
thresholds getters instead of fixed variables.
- Improved awaiting of members import job in regression test
The MailgunEmailSuppression list was incorrectly adding emails
to the suppression list for permanent failure events which have
an error code outside of the 5xx range.
fixes https://github.com/TryGhost/Team/issues/1996
**Issue**
Our Magic links are valid for 24 hours. After first usage, the token
lives for a further 10 minutes, so that in the case of email servers or
clients that "visit" links, the token can still be used.
The implementation of the 10 minute window uses setTimeout, meaning if
the process is interrupted, the 10 minute window is ignored completely,
and the token will continue to live for the remainder of it's 24 hour
validity period. To prevent that, the tokens are cleared on boot at the
moment.
**Solution**
To remove the boot clearing logic, we need to make sure the tokens are
only valid for 10 minutes after first use even during restarts.
This commit adds 3 new fields to the SingleUseToken model:
- updated_at: for storing the last time the token was changed/used). Not
really used atm.
- first_used_at: for storing the first time the token was used
- used_count: for storing the number of times the token has been used
Using these fields:
- A token can only be used 3 times
- A token is only valid for 10 minutes after first use, even if the
server restarts in between
- A token is only valid for 24 hours after creation (not changed)
We now also delete expired tokens in a separate job instead of on boot /
in a timeout.
refs https://github.com/TryGhost/Team/issues/2235
We found some cases which can cause a site to have member emails that have invalid characters like `member@example.com�`. This happened due to the `validator` version used by Ghost not able to catch some specific cases as invalid email, allowing members to be created with them either via Admin or Importer or direct signup. Portal UI already blocked these email as invalid. This change:
- updates `@tryghost/validator` to include a latest version of email validator that catches these invalid cases
- doesn't allow member creation with invalid email like above
- doesn't allow existing member emails to be edited to invalid
refs: https://www.getrevue.co/app/offboard
- Revue is stopping all paid subscriptions on 20th Dec, and shutting down on Jan 18th.
- This update allows Ghost to accept and handle the zip file Revue are providing as an export in Labs > Importer
- It will import posts (as best as we can with the data provided) and subscribers as free members
- At present it doesn't import paid subscribers, as we don't have that info, but you can disconnect Revue from your Stripe account to prevent all your subscriptions being cancelled & there's the option this can be fixed later
- There will be further updates to polish up this tooling - this is just a first pass to try to get something in people's hands
Co-authored-by: Paul Davis <PaulAdamDavis@users.noreply.github.com>
fixes https://github.com/TryGhost/Team/issues/2386
**Issue:**
- When trying to import a member that already exists, and has
'subscribed' set to 'true' in the CSV, the newsletters the member is
subscribed to are reset to the default newsletters.
- When ediging a member with the API and setting `subscribed` to true,
the same happens.
**Cause:**
A faulty check for the `status` property of a newsletter.
Fixed and added a new E2E test.
- Now that the importer runs in a job, it seems sensble that we should
do this
- If posts are imported with HTML set, but not mobiledoc, we now convert html -> mobiledoc
- Note: This also converts the mobiledoc -> html so _may_ be lossy
- Without this, imports that only have HTML, not mobiledoc, would have
resulted in empty posts, so lossy > empty
refs https://ghost.slack.com/archives/C02G9E68C/p1670960248186789
This reverts a change that was made here:
f4fdb4fa6c (r93071549),
but it still moved the original code to a new location in the
LastSeenAtUpdater
It includes a new E2E test to make sure timezones are supported
correctly.
- By not using Bookshelf, we no longer fire webhook calls
- By not using the member repository, we don't fetch and update the
member model and the labels relation in a forUpdate transaction, which
caused deadlock issues on the labels/members_labels tables which were
hard to resolve. Until now I was unable to find the other conflicting
transaction that caused this deadlock. Moving to raw knex (instead of
Bookshelf) and only updating the last_updated_at column should remove
the deadlock issue.
This removed the test for the email service wrapper, since it started
failing for an unknown reason and the test didn't make much sense (was
added earlier only to bump test threshold).
- The get helper can sometimes take a long time, and in themes that have many get helpers, the request can take far too long to respond
- This adds a timeout to the get helper, so that the page render doesn't block forever
- This won't abort the request to the DB, but instead just means the page will render sooner, and without the get block
refs https://github.com/TryGhost/Team/issues/2371
- test publishes a post with access for a single tier then checks the front-end with no member, member on wrong tier, and member on right tier
- adds test that cover creating and signing up to multiple-month/forever offers
- checks that the offer information is shown to members during signup and in account detail
closes https://github.com/TryGhost/Team/issues/2376#event-8026429598
- if an offer is expired/in past, we no longer show it in member account info against the price
- one-time offers are never showed in portal in member account detail, as the payment information shown to member in Portal points to charge at next payment
- if trial days are over for a subscription, portal doesn't show any offer data on member account detail
refs https://github.com/TryGhost/Team/issues/2371
- playwright tests were broken due to state changes based on prev tests that were not accounted for
- in case of multiple newsletters, portal tests expected another step between stripe checkout for newsletter selection
- site settings test was disabling members, but not re-enabling it back
refs TryGhost/Team#2371
- check that members can unsubscribe from newsletters by toggling
preferences in their account settings
- check that member can log out
refs
f5aae1e2c5
refs
0f9ed54a6f
- changing playwright portal tests to work for single tier setup caused failure for comped upgrade tests as they were relying on button text that changed
refs. https://github.com/TryGhost/Team/issues/2371
- two extra assertion was needed for discount and free-trials to check
if the offers are listed in ‘Active’ offers and the URLs load portal
refs https://github.com/TryGhost/Team/issues/2371
- Test enabling private site and checking access with a password
- The test flow is lacking a check for site access through password
due to a Playwirght bug. This should be cleaned up in the future
refs. https://github.com/TryGhost/Team/issues/2371
- deleteAllMembers was an unnecessary step
- since there's a generated code appended to the name of the archived offer, it had to be shorter to avoid potential naming conflicts
refs https://github.com/TryGhost/Team/issues/2371
- Adds a test for publishing and sending
- Adds a test for email only sending
- Updated some util methods in the publishing spec to remove the dependency on the post bookmark (which is not present for email only posts)
refs https://github.com/TryGhost/Team/issues/2371
Note that the "Choose" button is "Continue" when running this test
standalone so currently it needs to run with the full suite.
refs https://github.com/TryGhost/Team/issues/2371
- extracting the re-used actions to utils allows tests to be self-descriptive rather than relying on comments and keeps the selectors and related actions in one place to help refactoring if/when they change
refs. https://github.com/TryGhost/Team/issues/2371
- Test for archived offers should be moved to ‘Archived’ view of the offer list in Admin, and the offer URL should redirect to the site's homepage for logged out visitors
refs https://github.com/TryGhost/Team/issues/2371
- bumped timeout between saving and refreshing to account for slower
speeds in CI
- increased specificity for the frontend text comparisons so the output
when failing is smaller and easier to parse
refs https://github.com/TryGhost/Team/issues/2371
- the 100ms timeout was enough for local tests to pass but was still failing on CI
- bumped to 200ms and skipped the creation of a new paragraph to reduce what the editor is doing
refs https://github.com/TryGhost/Team/issues/2371
- added timeout between clicking the editor and starting to type otherwise some of the typing events could be missed causing a mismatch in actual vs expected output
ref https://github.com/TryGhost/Team/issues/2371
- updated Member exports with csv validation
- added member fixtures to be loaded into Ghost to ensure filtering
works correctly when downloading / exporting members csv.
refs https://github.com/TryGhost/Team/issues/2371
- The "data-test-*" selectors in playwright did not work with publishing channel selectors. This is a quick hack to enable working around it
refs https://github.com/TryGhost/Team/issues/2367
This ensures that a Member is not considered subscribed to any emails, so that
counts for newsletter recipients are correct. Eventually we will filter members
on their email suppression status but this is not implemented yet.
Refs https://github.com/TryGhost/Team/issues/2371
- Tests whether the post access selection of public, members, or paid-members matches the expected post visibility on the frontend.
refs https://github.com/TryGhost/Team/issues/2371
- Adds a test that schedules a post 5 seconds in the future and waits
for it to be published
- Reduced the time restrictions for scheduling:
- The minimum time in the frontend is now 5 seconds in the future (came
from 5 minutes in the future)
- The time picker now suggests 10 minutes in the future instead of the
minimum scheduling time (came from 5 minutes)
- In the backend, a post will be allowed to be scheduled if it is at
least 2 minutes in the past (came from 2 minutes in the future)
- The scheduler will publish a post if it is at least 5 minutes in the
past, and maximum 5 minutes in the future (came from 2 minutes)
refs https://github.com/TryGhost/Team/issues/2371
- tests that a free member can upgrade to a paid tier via stripe checkout and the payment details are reflected in portal and member detail page on admin
refs https://github.com/TryGhost/Team/issues/2371
- tests modifying the content of a published post
- extracted publish flow into a `publishPost` function that returns a new browser page object with the newly created post loaded
refs https://github.com/TryGhost/Team/issues/2369
- this checks whether the Offer redemption count is set to 1, which
would be indicative that the Offer was successfully counted as
redeemed
refs https://github.com/TryGhost/Toolbox/issues/479
- this includes a handful of improvements to get Playwright working on a
local environment including:
- adding `testing-browser` environment so we don't nuke `development`
environments, and makes all the necessary changes to get Ghost to
behave when this is running
- stopped running one global instance of Ghost as this doesn't provide
a clean environment
- copies a few default fixtures that are needed for the new
environment
- we should start to keep tests grouped by their area, so first we split
by Admin tests and then Portal tests, and within that we split into
setup/Tiers/Offers etc
refs https://github.com/TryGhost/Toolbox/issues/476
- The email verification trigger and host settings related bugs have been a cause of bugs in past releases. The admin client verification source did not have any test coverage in the past.
- The members test suite size is getting out of hand. This test is quite verbose, because of the state it's trying to check.
- In the future we should consider splitting up Member API (and probably other) test suites into smaller pieces.
no issue
- The sleep method has been used in 8 modules reimplementing the same thing over and over again. It's usually a sign of async event processing outside of the request/response loop. It's good to have a single point of implementation for a "hack" like this, so we could track it easier and address the even processing delay in a more optimal way centrally if it ever becomes a bottleneck
no issue
This will need some work, since we are introducing a 500ms delay to wait for a network request to return. Ideally the tier expander should eventually populate itself.
no issue
Local tests can now setup Stripe during the global setup process, and the webhook server is run out-of-process.
Running tests in CI against localhost will use environment variables to setup Stripe.
Providing a test URL will avoid setting up Stripe and will assume that it is already done.
no issue
This commit allows tests to run remotely by replacing selectors with production-suitable ones (no [data-test...]).
It also allows running locally with Stripe webhooks by adding a new global setup function.
fixes https://github.com/TryGhost/Team/issues/2346
- Adds email batch browse endpoint
- Adds email recipient failures browse endpoint
- Adds new fixtures and E2E tests for the new API
- Added support for snapshot tests to have 'nullable' types.
refs https://github.com/TryGhost/Team/issues/2339
- Includes a new pattern in the job manager that allows us to properly
await jobs.
- Added new convenience mocking methods to stub settings
- Tests the main flows for bulk sending:
- Sending in multiple batches
- Sending to multiple segments
- Handling a failed batch and retrying that batch
- Fixes bug in batch generation (ordering not working)
In a different PR I'll add more detailed tests.
We can fetch the same event multiple times from Mailgun so we need to
be able to protect against inserting duplicate events in the
database. This will allow us to catch duplicate errors on insert when
handling complaint events.
fixes https://github.com/TryGhost/Team/issues/2332
Saves events in the database and collects error information.
Do note that we can emit the same events multiple times, and as a result
out of order. That means we should correctly handle that a delivered
event might be fired after a permanent failure. So a delivered event is
ignored if the email is already marked as failed. Also delivered_at is
reset to null when we receive a permanent failure.
closes https://github.com/TryGhost/Team/issues/2324
- It seemed like the "limit" query parameter did not work properly returning multiple entries from the endpoint. In reality the whole query string was ignored because of an error in the "filter" part of the query ^_^
refs https://github.com/TryGhost/Team/issues/2326
- The job takes considerably longer to run with MySQL, so needed a longer sleep time. It's a temporary fix to unblock a broken build. We should investigate why the job takes so long to run on MySQL
refs https://github.com/TryGhost/Team/issues/2225
- updated the `formatOnWrite` transform map for posts to include the new `nodes` and `transformMap` options used by `urlUtils` for transforming node payload data
- added `nodes` to the `lexicalLib` module that pulls in our default nodes to be passed in to the URL transform utilities
- added `urlTransformMap` to the `lexicalLib` module that maps transform type and data type to URL transform utility functions that accept a single URL argument
refs https://github.com/TryGhost/Team/issues/2317
This table is used for persisting the email suppression list.
We don't have a member_id column because emails, not members are suppressed.
fixes https://github.com/TryGhost/Team/issues/2308
- Still has some missing pieces, but mostly works.
- Uses new handlebars template for emails
- When sending emails with the new email stability flag enabled, one
test email is now sent via the default smtp ghost mailer.
fixes https://github.com/TryGhost/Team/issues/2310
This moves the processing of the events from the event-processor to a
new email-event-processor in the email-service package.
- The `EmailEventProcessor` only translates events from
providerId/emailId to their known emailId, memberId and recipientId, and
dispatches the corresponding events.
- Since `EmailEventProcessor` runs in a separate worker thread, we can't
listen for the dispatched events on the main thread. To accomplish this
communication, the events dispatched from the `EmailEventProcessor`
class are 'posted' via the postMessage method and redispatched on the
main thread.
- A new `EmailEventStorage` class reacts to the email events and stores
it in the database. This code mostly corresponds to the (now deleted)
subclass of the old `EmailEventProcessor`
- Updating a members last_seen_at timestamp has moved to the
lastSeenAtUpdater.
- Email events no longer store `ObjectID` because these are not
encodable across threads via postMessage
- Includes new E2E tests that test the storage of all supported Mailgun
events. Note that in these tests we run the processing on the main
thread instead of on a separate thread (couldn't do this because
stubbing is not possible across threads)
There are some missing pieces that will get added in later PRs (this PR
focuses on porting the existing functionality):
- Handling temporary failures/bounces
- Capturing the error messages of bounce events
refs https://github.com/TryGhost/Team/issues/2291
When sending out mails to individual recipients, its possible that recipient gets a temporary or permanent failure for receiving the mail. Temporary failures can generally get resolved after a bit when the recipient’s mail server accepts the email, unlike permanent failures. For both customer visibility and easier debugging on what went wrong while delivering to a particular recipient, we’ll store the permanent/temporary failure for a recipient.
- migration adds a new table that stores the failure information for the recipients
fixes https://github.com/TryGhost/Team/issues/2326
When importing more than 500 members, we didn't testImportThreshold at
the right time. It was called too early because the importing job was
not awaited. This also adds an E2E test for this case.
closesTryGhost/Team#2313
- Added Sent event to Post analytics and Members feed. Now post can be
Sent or Received or Bounced.
- Excluded Delivered event from Sent filter on backend.
refs https://github.com/TryGhost/Ghost/security/advisories/GHSA-9gh8-wp53-ccc6
refs https://github.com/TryGhost/Toolbox/issues/465
- Bookshelf relations allows us to edit relational records by default, which was used liberally in the codebase.
- Not having a clear track record of editable relations left the model layer prone to triggering unwanted nested saves and created a vulnerability where members were able to edit newsletter settings.
- With explicit editable relations it's easier to keep track of relations having editable access to related records. Makes the relational data modification pattern safer to use too.
- Anyone running 5.x should update to 5.24.1
Credits: Dave McDaniel and other members of [Cisco Talos](https://talosintelligence.com/vulnerability_reports)
refs https://github.com/TryGhost/Ghost/security/advisories/GHSA-9gh8-wp53-ccc6
refs https://github.com/TryGhost/Toolbox/issues/465
- Bookshelf relations allows us to edit relational records by default, which was used liberally in the codebase.
- Not having a clear track record of editable relations left the model layer prone to triggering unwanted nested saves and created a vulnerability where members were able to edit newsletter settings.
- With explicit editable relations it's easier to keep track of relations having editable access to related records. Makes the relational data modification pattern safer to use too.
- Anyone running 5.x should update to 5.24.1
Credits: Dave McDaniel and other members of [Cisco Talos](https://talosintelligence.com/vulnerability_reports)
refs: https://github.com/TryGhost/Team/issues/1121
refs: 54574025e0
- The previous change to fall back to a generic error on the server side is resulting in lots of much less useful Sentry reports
- For unexpected errors, change what's sent to Sentry back to context
- This is done by adding a specific code, so we don't have to match on a string that might change
- Also add the error type, id, code & statusCode as tags to the events - these are searchable structured data
- Adding code as a tag also makes it possible to find all errors that showed the generic message
- As demonstrated by my comments in the boot file, I thought sentry was already depending on the version package
- IMO it's undesirable to require package.json directly esp when we have a tool setup and ready for tis
- Added a bunch of tests to show that Sentry does roughly what we think
fixes https://github.com/TryGhost/Team/issues/2284
New batch sending flow (still WIP). Logs the sent emails instead of actually sending them. Unit tests are coming in later commits.
refs https://github.com/TryGhost/Team/issues/2280
We are moving away from storing html and plaintext on email and instead will store the email data in source and source_type columns which allows us to store the email in other formats like mobiledoc and lexical. Storing in those formats allows greater flexibility for later html generation
- adds new `source` column that stores `mobiledoc`/`lexical`/`html` data for a newsletter
- adds new `source_type` column that stores one of `mobiledoc`/`lexical`/`html` to identify type of source
closes https://github.com/TryGhost/Team/issues/2290
Currently, if the whole batch of email fails to send we don’t capture
any errors directly tied to the batch. This makes it hard to debug which
and why a batch failed when debugging email errors. Going forward we'll
store the error information for a failing email batch directly that
allows easier debugging for batch.
- `error_status_code` : Captures statusCode returned by Mailgun,
available in error.status from the example batch error
- `error_message` : Captures short error message from Mailgun and
status, available in context object of batch error
- `error_data` : Captures while whole error json for a batch. As
mentioned in pitch, this will be huge data and we’ll figure out long
term how to best use this.
refs: https://github.com/TryGhost/Toolbox/issues/479
Framework includes:
* command to run tests
* command to record tests
* mechanism for starting and stopping Ghost before and after each suite of tests
* mechanism for loading fixtures into Ghost before starting tests
* sample test for controlling Ghost Admin
fixes https://github.com/TryGhost/Team/issues/2282
Added a new email service package that is used when the email stability
flag is enabled. Currently not yet implemented so will throw an error
for all entry points (if flag enabled).
Removed usage of `labs.isSet.bind` across the code, because that breaks
the stubbing of labs by `mockManager.mockLabsEnabled` and
`mockManager.mockLabsDisabled`. `flag => labs.isSet(flag)` should be
used instead.
All email depending tests now disable the `emailStability` feature flag
to keep the tests passing + make sure we still run all the tests for the
old flow while the email stability package is being built.
refs https://github.com/TryGhost/Team/issues/2268
The approach of using the service to lead email suppression data as
opposed to bookshelf relations allows us to wire things up without
having implemented the database. The getBulkSuppressionData allows us to
do this without much of a DB performance hit.
closes https://github.com/TryGhost/Team/issues/2126
- Cleaned up the following GA flags: `newsletterPaywall`, `freeTrial`, `compExpiring`, `searchHelper`, `emailAlerts`, `fixNewsletterLinks`.
refs https://github.com/TryGhost/Team/issues/2246
- This change helps avoid race conditions due to a lack of a transaction
in the email job. It also moves the status check before creating the
email batches (can take a while) to prevent other timing issues in case
the job got scheduled multiple times.
- Sets the patch option to true when changing the status of an email
batch. If we don't do this, the bookshelf-relations plugin might try to
save relations too. This could have caused a 'no rows updated' error.
- Added a test that tests if the email job can only run once
- Added logging to batching logic
- updated the cover image to be simpler
- made the change in text fixtures as well, just to keep the fixtures in sync
Co-authored-by: Hannah Wolfe <github.erisds@gmail.com>
fixes https://github.com/TryGhost/Team/issues/2085
Don't load relations we don't need anymore for the posts table. And
reload the individual post when we open the analytics page with more
relations that we actually need.
refs https://github.com/TryGhost/Team/issues/2216
The `membersActivity` flag was an alpha feature to test the first versions of member analytics, and is no longer active or in use. This change removes the remaining pieces of code that are setup behind that flag and are no longer in use or accessible.
refs: https://github.com/TryGhost/Ghost/issues/14882
This commit totally removes Bluebird from the importer. Updated `@tryghost/promise` to use native async/await and refactored importer logic to avoid the need of `reflect()`.
fixes https://github.com/TryGhost/Team/issues/1903
MembersAgent.loginAs sends email, asynchronously via events. Which
conflicts with tests that also test emails. We cannot properly await
these events, so this is currently fixed with a timeout of 200ms. But
this was too random and unreliable.
closes https://github.com/TryGhost/Team/issues/2222
Whilst we were checking for Stripe objects being active, we were not
checking for them existing in Stripe. This adds handling to all read
request to Stripe in the payment link flow, so that we can gracefully
handle deleted objects.
We've also included an automated test which fails without this fix.
We've also improved the query to find Stripe Prices which will result
in less request to the Stripe API to check if it is valid.
closes https://github.com/TryGhost/Team/issues/2207
- adds conditional to the post email serializer to switch between
`mobiledocLib` and `lexicalLib` depending on which format the post
contains
closes https://github.com/TryGhost/Team/issues/2211
We were allowing paid Tiers to be imported with non-integer prices which was
causing the Admin to be bricked when attempting to load them. This adds some
validation to the price data of Tiers.
refs https://github.com/TryGhost/Toolbox/issues/461
- The 'vary' header with 'Origin' value should only be set when an OPTIONS header is processed. Otherwise we are prone to leaking the vary header modification to further down in the request pipeline
refs https://github.com/TryGhost/Toolbox/issues/461
- The unit test was never using the "OPTIONS" request method, which did not actually trigger the full logic of the "cors" module used under the hood.
- Using the correct request method triggers all the right pathways and tests the state that's closer to the real world - for example the response does get "ended" instead of calling the "next" middleware.
refs https://github.com/TryGhost/Toolbox/issues/461
- Having a 'Origin' in vary header value present on each `OPTIONS` allows to correctly bucket "allowed CORS" and "disallowed CORS" responses in shared caches
refs https://github.com/TryGhost/Toolbox/issues/461
- Having a 'Origin' in vary header value present on each `OPTIONS` allows to correctly bucket "allowed CORS" and "disallowed CORS" responses in shared caches
refs https://github.com/TryGhost/Toolbox/issues/461
- The codebase has ambiguous behavior with OPTIONS request. Adding tests covering edge cases for all possible variations of OPTIONS responses is the first step to solving cahceability of these requests.
- The obvious question if you look into the changeset itself would also be: "WTF did you do with test suite naming? What are these changes in admin and click tracking suites? You having a bad day Naz?". The answer is "yes" (╯°□°)╯︵ ┻━┻
- On a serious note. I've introduced multiple hacks here that should be fixed:
1. Forced test suite execution order for options request - extreme blasphemy. This was last resort decision. I went deep into trying to fixup the server shutdown in the "admin" test suite, which cascaded into failing "click tracking" suite, which has shortcomings on it's own (see notes left in that suite)
2. Exposed "ghostServer" from the e2e-framework's "getAgentsWithFrontend" method. Exposing ghostServer to be able to shut it down (or do other manipulations) was one of the pitfalls we had in the previous test utils, which ended up plaguing the test codebase. Ideally the framework should only be exposing the agents and the rest would happen behind the scenes.
- To fix the hacks above I've raised a cleanup issue (https://github.com/TryGhost/Toolbox/issues/471). I'm very sorry for this mess. The issue at hand has very little to do with fixing the e2e framework, so leaving things "as is".
refs https://github.com/TryGhost/Toolbox/issues/461
- When testing OPTIONS requests there is a need to get all possible agents available in the system. The "getAgentsWithFrontend" serves exactly this purpose - create all possible agents while starting Ghost instance only once
- This is groundwork for OPTIONS request caching tests and improvements
refs https://github.com/TryGhost/Ghost/commit/1f300fb781f0
The full customer object was not being passed to the StripeAPI service
when it already exists, this was resulting in inconsistent behaviour when
sending the customerEmail param to the API, causing `invalid_email`
errors to be thrown from Stripe and breaking the checkout.
closes https://github.com/TryGhost/Team/issues/2196
We were incorrectly assuming that all requests would have the
`customerEmail` passed in the body. Instead we were incorrectly
passing `undefined` or `''` as the `customerEmail` property to stripe,
which resulted in a validation error.
We've updated the code to pass `null` in the case of a falsy value,
which the Stripe API handles without error.
closes https://github.com/TryGhost/Team/issues/2195
The issue here is two-fold, and specific to using Offers so was not
caught by any automated tests. First, we were incorrectly comparing
the tier.id to the offer.tier.id - this is because the Tier objects id
property is an instance of ObjectID rather than a string.
Secondly we were passing through the cadence parameter from the
request body, but when using Offers this is not including in the
request, so we must pull the data off of the Offer object instead and
pass that to the payments service.
closes https://github.com/TryGhost/Ghost/issues/15740
The validation function for a Tier description was not returning the
validated value, which meant we were unable to set the Tier
description.
refs https://github.com/TryGhost/Toolbox/issues/464
Bceause the import does not use the API, any backwards compat code we put in the
API does not get run for imports, this means we need to update the importer to
map the stripe_prices data onto the products table so that we have valid data in
the database.
refs: a8b1676734
- Extended the newly created handlebars test utils with a shouldCompileToError method
- Updated the price helper tests tp use shouldCompileToExpected and shouldCompileToError
- This allows us to test our handlebars helpers in a much more conisstent way
no issue
- There are currently two patterns in our handlebars helper unit tests:
1. Treating the helper as a function, and doing a function call
- This is the original way the tests were done, and they're not great as they're approximating how the helpers are really used
2. Using a template string, and rendering the string using a method called shouldCompileToExpected
- These tests are more realistic and powerful and also easier to read
- The new method is only being used in a few places so far, and each place had re-created the `shouldCompileToExpected` method
- Therefore I've moved this method into a util that should make it easier to write unit tests for handlebars helpers
- I also renamed the method in the excerpt tests, because it doesn't do the same thing, it's just a wrapper around a function call rather than compiling a string
The aim is to refactor all of our handlebars helper tests to use `shouldCompileToExpected`
- These tests are very slow, and make the build fail about 2/3 times
- Temporarily skipping until we can fix, as I want to get all our outstanding hacktoberfest PRs merged
fixes https://github.com/TryGhost/Team/issues/2175
- New event type `aggregated_click_event` that is disabled by default in all the existing activity feeds
- This returns click events, but only the first click events for each member/post combination.
- It includes the total count of unique link clicks for that member on that post combination
- Had to resort to some custom knex queries to make this work easily
- Requires `@tryghost/bookshelf-pagination@0.1.31`, included in `@tryghost/bookshelf-plugins@0.6.1` (this fixes an issue with custom selects breaking the total count query of pages)
- Went a bit overboard with the pagination tests to cover as much unknown edge cases as possible
refs https://github.com/TryGhost/Team/issues/2168
- site owners can now disable tracking sources from analytics settings.
- this change removes the loading of attribution script if tracking is
turned off so we don't capture any post/page or external source
attributions
refs https://github.com/TryGhost/Team/issues/2168
- the new setting allows site owners to control if they want to track
the sources for new member signups and subscriptions
- its switched on by default, but can be toggled off from new analytics
settings page
fixes https://github.com/TryGhost/Team/issues/2129
- This changes how the activity feed API parses the filter.
- We now parse the filter early to a MongoDB filter, and split it in two. One of the filters is applied to the pageActions, and the other one is used individually for every event type. We now allow to use grouping and OR's inside the filters because of this change. As long as we don't combine filters on 'type' with other filters inside grouped filters or OR, then it is allowed.
- We make use of mongoTransformer to manually inject a mongo filter without needing to parse it from a string value again (that would make it a lot harder because we would have to convert the splitted filter back to a string and we currently don't have methods for that).
- Added sorting by id for events with the same timestamp (required for reliable pagination)
- Added id to each event (required for pagination)
- Added more tests for filters
- Added test for pagination
- Removed unsued getSubscriptions and getVolume methods
Used new mongo utility methods introduced here: https://github.com/TryGhost/NQL/pull/49
refs https://github.com/TryGhost/Team/issues/2077
- Members and Posts test suites were using a broad tiers property matcher, which is an anti-pattern for snapshot tests. Without more specific snapshots it would be very hard to track down tier-related breaking changes!
- This change is groundwork for a refactor coming in tier usage at API's output serializers
closesTryGhost/Team#2159
- Added column to email table
- Hide the feedback tab on frontend depending on the column value
Co-authored-by: Daniel Lockyer <daniellockyer@fastmail.com>
refs: https://github.com/TryGhost/Ghost/issues/15537
- snapshot test created to add confidence to webhook stability and increase overall test coverage.
Co-authored-by: Kritika Sharma <kritikasharma@Kritikas-MacBook-Pro-2.local>
fixes https://github.com/TryGhost/Team/issues/2137
For the analytics page, we need the sent events to show up immediately
after sending an email. Otherwise we need to wait for emails to be
marked as received (which takes too long) before being able to show them
on the analytics page.
This adds the email_sent_event, which is hidden by default everywhere
and used on the analytics page.
refs: https://github.com/TryGhost/Ghost/issues/14882
- Opted to use the in-house `sequence` function when refactoring Bluebird's `Promise.each` to avoid deadlock issues (see 734ef66e6c).
-It's hard to know without tonnes of context if any `Promise.each` are safe to refactor to `Promise.all`.
refs https://github.com/TryGhost/Team/issues/2116
- allows site owners to edit a link in a post that has already been sent out, fixing any typos or other mistakes
- resets click counter for the edited link back to 0 so site owners can see the clicks on new link, doesn't change the overall click count
refs https://github.com/TryGhost/Team/issues/2135
The email link redirects on Pro are cached as 302 redirects in Varnish, so we're missing further clicks after the first one for each member, until the cache is invalidated. This change invalidates cache on link edits to ensure that we correctly redirect members to updated link everytime
refs https://github.com/TryGhost/Team/issues/2034
- this table will be used to link Stripe subscriptions to Ghost
subscriptions via a foreign key that we add at a later point
- this also includes `constraintName` as the auto-generated one would be
too long for MySQL 8
refs https://github.com/TryGhost/Team/issues/2104
- adds edit permissions for links endpoints to fixtures
- new `bulkEdit` endpoint will use the permissions and allow fixing newsletter links via Admin
refs 5fcf5098a8
- links browse endpoint had permissions switched off unintentionally and was also missing the necessary permissions in fixtures.
- enables permissions for browse endpoint and adds migration insert permissions in DB
fixes https://github.com/TryGhost/Team/issues/2090
- This changes how sentiment is exposed in the API. Now it is exposed as a `sentiment` relation, directly on the model (no longer in counts). Internally we still use `count.sentiment`.
- Content API users (and themes) can include the 'sentiment' relation and order by sentiment.
- Updated Admin to use sentiment instead of count.sentiment
fixes https://github.com/TryGhost/Team/issues/2091
fixes https://github.com/TryGhost/Team/issues/2089
- Added new fixtures to make testing easier for the activity feed
- Improved E2E test coverage of activity feed with separate test file
- Added data.post_id filter to enable filtering by events related to a
given post
- Fixed return types in JSDoc of test agents (TypeScript interprets
these as `typeof Agent` if we don't add `InstanceType<Agent>`)
- Added total pagination metadata to activity feed API (to allow a basic
type of pagination using filters)
fixes https://github.com/TryGhost/Team/issues/2096
When generating the recipient data for emails, the email clicks
implementation is resulting in a recipient variable being added called
replacement_xxx once for each link containing the same UUID.
This generates a lot of unnecessary data overhead for emails, and it
turns out that mailgun has a 25MB message limit. We wouldn't have come
close if we only included the uuid once.
fixes https://github.com/TryGhost/Team/issues/2102
- this column was added with `nullable: true` but it should never be
nullable, so we should drop the nullable status whilst it's easy to
fixes https://github.com/TryGhost/Team/issues/2084
- When audience feedback is enabled, we use a single 'conversions' count instead of having separate ones for signups and paid conversions.
- The analytics component is separated so we can change it without breaking the existing page.
refs https://github.com/TryGhost/Team/issues/2072
Google is indexing our redirects and storign the redirected content
against the redirect URL in search results. This seems to be caused by
us using a 302 redirect rather than 301. We don't want to switch to a
301 however, so that we can support the ability to update redirects in
the future.
refs https://github.com/TryGhost/Team/issues/1765
In order to better handle deleted objects in Stripe we want to decouple
Members from Stripe.
These changes allow us to have the Tier concept completely independent
of the Stripe tables, such that the Stripe data can be generated as/when
it's needed - which will help to protect against missing data.
closes: https://github.com/TryGhost/Ghost/issues/14973
- When fetching content using a non-standard charset, characters were notproperly decoded to utf-8 resulting in mangled text in the editor -> Detect charset and use iconv to decode the page text
- When requesting a non bookmark card, if no oembed data could be foundand we fallback to bookmark, a second network request to fetch the content was issued. This seemed unnecessary -> refactored to avoid that
refs: https://github.com/TryGhost/Ghost/issues/15537
- snapshot test created to add confidence to webhook stability and increase overall test coverage.
Co-authored-by: Kritika Sharma <kritikasharma@Kritikas-MacBook-Pro-2.local>
refs https://github.com/TryGhost/Team/issues/2030
- adds `subscriptions` table to the DB schema
- this new table is aimed to support a native "subscription" primitive in Ghost
that most resembles previously used `members_stripe_customers_subscriptions` table
refs https://github.com/TryGhost/Toolbox/issues/441
- whilst reviewing another PR, I noticed we were incorrectly using
`maxLength` instead of `maxlength` in the schema column definition
- it turns out we've already been doing this wrong for a while with
other columns
- this key is not acted upon, so the maximum column length was not applied
- fixing up the DB to the correct maximum length is something to fix in the
future but right now, the schema does not reflect the size of the
column that actually got created
- the fallback when `maxlength` is not provided is currently 191 [0], so
this commit switches the schema and migrations to using the correct
key name and column length that they are using when applied
[0]: 24670aa555/ghost/core/core/server/data/schema/commands.js (L27)
refs https://github.com/TryGhost/Toolbox/issues/441
- we tend to have a mix of `bool` and `boolean` in the schema and
migrations, which has become a real nit for me at this point
- we don't do any special handling between `bool` and `boolean`, it's
just something we pass to Knex
- `bool` is an alias for `boolean` but `boolean` is actually documented - https://knexjs.org/guide/schema-builder.html#boolean
- this commit switches Ghost to only using `boolean` in the schema and
migrations, and removes `bool` from the allowlist in tests to prevent
us from adding it again in the future
- this should make absolutely no difference to the DB because both
resulted in the same column
refs https://github.com/TryGhost/Toolbox/issues/441
- I'm currently working on cleaning up our uses of `bool` and `boolean`
in favor of `boolean`, and I've noticed we only handle converting
numbers into booleans when the type is `bool`, so validation would
otherwise fail
- given these can be used interchangeably, we should also support
converting the numbers into booleans when the type is `boolean`
- this is going to get cleaned up again when I remove `bool` but this
fixes the validation bug for now
refs: https://github.com/TryGhost/Ghost/issues/15537
- snapshot test created to add confidence to webhook stability and increase overall test coverage.
Co-authored-by: Kritika Sharma <kritikasharma@Kritikas-MacBook-Pro-2.local>
refs: https://github.com/TryGhost/Ghost/issues/15537
- this adds an e2e test and test snapshot for the `tag.edited` webhook so we can prevent regressions and bugs in the future
Co-authored-by: Hannah Wolfe <github.erisds@gmail.com>
closes: https://github.com/TryGhost/Ghost/issues/14981
- Taxonomy-specific sitemaps were invalid xml when there was no data
- These invalid empty sitemaps were referenced in the index sitemap causing SEO tools to report errors
closes: https://github.com/TryGhost/Ghost/issues/15470
- When multiple browser tabs are open, each manipulate a different copy of ember data model, changes to the model in one tab are not reflected in the model of the other tab.
- When updating some settings, all current settings were sent to the API.
- As a result, when updating two different categories of settings (navigation/code inspection) in different tabs, the second update was overriding the first one.
- From a user perspective, this is not a natural behaviour. Only settings visible on-screen when clicking save should be modified.
Co-authored-by: Kevin Ansfield <kevin@lookingsideways.co.uk>
refs https://github.com/TryGhost/Toolbox/issues/441
- this is only v1 of the test I would like but it validates the keys on
a column definition are part of an allowlist
- this has already uncovered a bug with `maxLength` (vs `maxlength`)
fixes https://github.com/TryGhost/Team/issues/2054
This change adds the sentiment and positive_feedback counts to the posts models. This change isn't really ideal because there are some problems here:
- sentiment isn't really a count
- we don't need to include the sentiment and positive_feedback as a default for posts (but the same is true for attribution)
It would make sense to move this to separate endpoints that only fetch the analytics for a given post when the analytics page is opened. But for our initial skateboard version of audience feedback this should be a good start to already see the data.
refs https://github.com/TryGhost/Team/issues/2047
- We anticipate upcoming changes in the PUT /members/:id/subscriptions/:subscription_id endpoint , so covered it with a snapshot test to track the differences more precisely.
- Note, the test case contains a more explicit outgoing HTTP request mocking.
refs https://github.com/TryGhost/Team/issues/1967
This tests the full flow of publishing a newsletter, and then checking
that clicked links will increase the click count, generate events for
the member which clicked the link as well as the redirects contain the
correct query params.
refs https://github.com/TryGhost/Toolbox/issues/320
- Added more complex mobiledoc structure in the post.published test to check for correct transformation of special purpose `__GHOST_URL__`. The snapshot has a correct URL transformation, which gives confidence it works properly
refs https://github.com/TryGhost/Toolbox/issues/320
- There noe "roles" attached to the post's author when the 'post.added' event is fired. Webhooks function based of the model events and differ slightly with it's output comparing to the API response. For example, in case of Posts API, there'a an additional 'findOne' call (ref.: https://github.com/TryGhost/Ghost/blob/main/ghost/core/core/server/models/post.js#L1224-L1227) before returning the post to the endpoint handler and then passing that to the output serializer.
- If we want to have 1:1 copy of webhooks outputs and API outputs, we should rethink how we rely on model event data which is never the same as API controller level data.
refs a499f866f3
refs d817e5830d
- The user-agent used in outgoing Ghost requests (webhooks mostly) is dependent on the Ghost version - snapshots break if the matcher is not dynamic.
- There will be a few more webhooks tests coming soon, so makes sense to have this matcher moved to a common "framework matchers"
fixes https://github.com/TryGhost/Ghost/issues/14508
This change requires the frontend to send an explicit `emailType` when sending a magic link. We default to `subscribe` (`signin` for invite only sites) for now to remain compatible with the existing behaviour.
**Problem:**
When a member tries to login and that member doesn't exist, we created a new member in the past.
- This caused the creation of duplicate accounts when members were guessing the email address they used.
- This caused the creation of new accounts when using an old impersonation token, login link or email change link that was sent before member deletion.
**Fixed:**
- Trying to login with an email address that doesn't exist will throw an error now.
- Added new and separate rate limiting to login (to prevent user enumeration). This rate limiting has a higher default limit of 8. I think it needs a higher default limit (because it is rate limited on every call instead of per email address. And it should be configurable independent from administrator rate limiting. It also needs a lower lifetime value because it is never reset.
- Updated error responses in the `sendMagicLink` endpoint to use the default error encoding middleware.
- The type (`signin`, `signup`, `updateEmail` or `subscribe`) is now stored in the magic link. This is used to prevent signups with a sign in token.
**Notes:**
- Between tests, we truncate the database, but this is not enough for the rate limits to be truly reset. I had to add a method to the spam prevention service to reset all the instances between tests. Not resetting them caused random failures because every login in every test was hitting those spam prevention middlewares and somehow left a trace of that in those instances (even when the brute table is reset). Maybe those instances were doing some in memory caching.
fixes https://github.com/TryGhost/Ghost/issues/14508
This change requires the frontend to send an explicit `emailType` when sending a magic link. We default to `subscribe` (`signin` for invite only sites) for now to remain compatible with the existing behaviour.
**Problem:**
When a member tries to login and that member doesn't exist, we created a new member in the past.
- This caused the creation of duplicate accounts when members were guessing the email address they used.
- This caused the creation of new accounts when using an old impersonation token, login link or email change link that was sent before member deletion.
**Fixed:**
- Trying to login with an email address that doesn't exist will throw an error now.
- Added new and separate rate limiting to login (to prevent user enumeration). This rate limiting has a higher default limit of 8. I think it needs a higher default limit (because it is rate limited on every call instead of per email address. And it should be configurable independent from administrator rate limiting. It also needs a lower lifetime value because it is never reset.
- Updated error responses in the `sendMagicLink` endpoint to use the default error encoding middleware.
- The type (`signin`, `signup`, `updateEmail` or `subscribe`) is now stored in the magic link. This is used to prevent signups with a sign in token.
**Notes:**
- Between tests, we truncate the database, but this is not enough for the rate limits to be truly reset. I had to add a method to the spam prevention service to reset all the instances between tests. Not resetting them caused random failures because every login in every test was hitting those spam prevention middlewares and somehow left a trace of that in those instances (even when the brute table is reset). Maybe those instances were doing some in memory caching.
no issue
- All content-length snapshots should be using the same matcher for consistency - anyContentLength. It's more explicit about what the matcher is all about and might be useful to have content-length matchers in one place if it ever changes (the header value should be a damn digit after all, not a string!) (ref. https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2)
refs https://github.com/TryGhost/Team/issues/2024
Without validation it was possible to send a string of comma separated
email addresses to the endpoint, and an email would be sent to each
address, bypassing any rate limiting.
This bug does not allow for an authentication bypass exploit. It is purely a
spam email concern.
Credit: Sandip Maity <maitysandip925@gmail.com>
refs https://github.com/TryGhost/Toolbox/issues/320
- The URL matcher is very likely to be reused in the future, so having it abstracted away gives two benefits:
1. Central place to document hacky behavior and easier future cleanup
2. The implementer of the e2e test does not have to see the "hacky note" and just concentrate on the implementation of the test
refs https://github.com/TryGhost/Toolbox/issues/320
- Header snapshot matching was missing from webhook e2e tests. With a bumped version of webhook-mock-receiver it's now possible to record and match webhook request headers.
closesTryGhost/Team#2007
- uses request context to add referrer source and medium for a new member
- uses integration name as referrer medium if exists
fixes https://github.com/TryGhost/Team/issues/2008
- New column that stores email click tracking at the time it was created
- Improved frontend side checks for when to show analytics
refs https://github.com/TryGhost/Toolbox/issues/425
refs https://github.com/TryGhost/Toolbox/issues/280
- The versioned API responses vary based on requested version (passed in request's 'accept-version' header). shared caches that sit between Ghost's origin server and the browser would be putting responses with same Vary into the same caching bucket, which is incorrect.
- This change makes response's Vary more granular and tells caching mechanisms to take 'Accept-Version' request header into account when caching.
- Informative read on the topic - https://www.fastly.com/blog/getting-most-out-vary-fastly
- renames `refSource`, `refMedium` and `refUrl` to `referrerSource`, `referrerMedium` and `referrerUrl` respectively for consistent naming across files and usages
- bumps member attribution script from alpha feature to now load for all sites. The script captures recent url history in localstorage to capture correct attribution for members.
- script is only loaded on the site if members is enabled
refs https://github.com/TryGhost/Team/issues/1967
- Test is good to test if the whole flow works as expected, and works together
- We can test independent parts in separate tests that have better coverage of more edge cases
- Adds a basic helper to get an agent for the frontend (spent too much time on a better solution so I decided to keep the existing supertest agent)
refs https://github.com/TryGhost/Ghost/pull/15471#discussion_r979902374
- the accent color value used by default content cta was copying the global site property which is redundant, and can be directly used
- originally, the accentColor property was extended to allow a fallback value for content ctas, but was later removed as we added default value to global site property directly
- the accentColor property is now deprecated and will be removed in next version, as existing themes might be relying on it for custom cta helpers
closes https://github.com/TryGhost/Team/issues/1898
- the default content cta always used the terminology as `post` when showing message that users don't have access to some content
- this caused confusion when users were looking at a page and message showed "This post is for subscribers only"
- updates the message to correctly reflect `page` vs `post` on the default cta
refs https://github.com/TryGhost/Toolbox/issues/410
- Private cache control was preventing browser or shared caches from storing Content APIs response. The type of data served through the Content API is very much of a "public" nature, so should be cacheable.
- Right now the 'max-age' value of 'cache-control' header is hardcoded to '0', without 'must-revalidate' value, to allow browsers to cache content slightly more aggressively. In the future the 'max-age' value will most-likely become configurable to allow even more aggressive HTTP caching.
refs https://github.com/TryGhost/Toolbox/issues/410
- The 'private' value in 'Cache-Control' response header for all errors made it impossible for shared caches (e.g.: Fastly, Cloudflare) to cache 404 responses efficiently.
- The change substitutes 'max-age=0' which should not effect the browser cache behavior but would allow shared caches to process such requests efficiently.
- A more loose caching logic only applies to 404 responses from GET requests that are not user-specific (non-authenticated, non-cookie containing requests)
closes https://github.com/TryGhost/Toolbox/issues/372
- The admin assets are served with a unique hash depending on the build with a year-long "max-age" value in the response cache-control header. The client browsers still do send 'If-None-Match' requests when there is a hard-refresh on the client side. There's no need for 'If-None-Match' requests though!
- With 'immutable' value in the cache-control header, the browser caches are treating responses as "hard-fresh" without sending redundant requests.
- For more about 'immutable' value read https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#immutable
closes https://github.com/TryGhost/Team/issues/1942
- Added data fixtures for referrers
- Added new endpoint to fetch referrer stats for a given post: `/stats/referrers/posts/:id`
- Added new ReferrersStatsService, responsible for calculating referrer stats
refs TryGhost/Team#1931
- referrer source, medium and url will be stored in the events table along with rest of attribution data
- stores referrer information on two tables
- `members_created_events` for signups
- `members_subscription_created_events` for paid conversions
closes https://github.com/TryGhost/Team/issues/1933
- Added click_events to activity feed
- Added support for parsing click_events in the frontend
- Moved url parsing (transform ready) to model layer of LinkRedirect
- Moved `getEventTimeline` method to the top of the event repository
- Added description field to parsed events in the frontend (because we need a second line)
- Fixed: member email not returned in comment_event
refs d5f03ec0b1
- underlying error message varies across node versions so the content-length can't be fixed
- applied any-content-length matcher to the right test this time
no issue
- bumped `@tryghost/url-utils` to get access to the new lexical transform utilities
- updated the Post model's `parse()` and `formatOnWrite()` methods to transform the `lexical` field contents when reading/writing to ensure any links in content point at the correct place with `site.url` config changes
refs TryGhost/Team#1907
- calculates final attribution source and medium using captured referrer information in history
- adds new referrer-translator that goes through available history and based to determine most valid referrer info
- includes referrer url, source and medium in the attribution data for storage
no issue
- The explore endpoint needs to expose the total amount of published posts
- To be more consistent, this PR creates a PostStats class which is exposed as `stats` method within the PostService; just like it's done with the MemberService
- Moved existing method to return the date of the most recently published post into the stats service
- Updated the explore service test to reflect the new return property
no issue
- added `PostRevsion` model
- duplicated `mobiledoc_revision` creation routine in Post model's onSaving hook to create `post_revision` when model's `lexical` field has changed
- updated `mobiledoc_revision` creation to skip when `lexical` field is populated
no issue
- initially this will perform the same function as `mobiledoc_revisions` but storing `lexical` instead of `mobiledoc`
- naming is intentionally generic ready for later expansions
closes https://github.com/TryGhost/Team/issues/1908
### Problem:
- We need tracking on the paywall links in each email. (we cannot ignore them because those buttons are probably gonna have a higher paid conversion attribution than others).
- Currently we only add the paywall HTML to an email when processing each batch. So if we batch an email to 1.000 recipients per 100, we'll generate the paywall HTML 10 times.
- We cannot replace links in `renderEmailForSegment` because that methods will get called multiple times. We don't want to have multiple redirect instances created for the same link in the same email.
### Solution:
- Move the generation of the paywall to the `serialize` method of the post email serializer.
- Surround the generated paywall with HTML-comments so we can remove it if required in `renderEmailForSegment` depending on the member segment we are sending the email to.
---
### Before:
**Serialize output:**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<!--members-only-->
<p>Content visible for paid members only</p>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
To be modified later by `renderEmailForSegment`:
**Paid members (nothing changed):**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<!--members-only-->
<p>Content visible for paid members only</p>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
**Free members (paywall _added_):**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<h2>Generated paywall here</h2>
<a href="https://subscribe.com">Subscribe to read the full post</a>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
### After this change:
**Serialize output:**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<!--members-only-->
<p>Content visible for paid members only</p>
<!-- PAYWALL -->
<h2>Generated paywall here</h2>
<a href="https://subscribe.com/?tracked">Subscribe to read the full post</a>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
To be modified later by `renderEmailForSegment`:
**Paid members (paywall removed):**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<!--members-only-->
<p>Content visible for paid members only</p>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
**Free members (members-only content removed):**
```html
<html>
<body>
<h1>Generated email header</h1>
<p>Generated text</p>
<div>
<!-- POST CONTENT START -->
<h1>Post title</h1>
<p>Content visible for all members</p>
<!-- PAYWALL -->
<h2>Generated paywall here</h2>
<a href="https://subscribe.com/?tracked">Subscribe to read the full post</a>
<!-- POST CONTENT END -->
</div>
</body>
</html>
```
no issue
- added `@tryghost/kg-lexical-html-renderer` dependency
- added `lexical` lib following the same pattern as our `mobiledoc` lib
- updated the Post model's `onSaving` hook to generate the `html` value from `lexical` when present
fixes https://github.com/TryGhost/Team/issues/1909
- The feature image caption is already escaped on the frontend
- Doing it again in the backend breaks the possibility to add links to the caption
- I checked and the `feature_image_alt` is not escaped in the frontend.
fixes https://github.com/TryGhost/Team/issues/1900
refs https://github.com/TryGhost/Team/issues/1901
- Defaults to the same value as the current email_track_opens setting for existing installations, otherwise defaults to true
- Had to use a custom migration because the `addSetting` helper doesn't support using an existing setting as current value
- Added a minimal UI to change the setting, but this still needs some design magic 🪄✨
- Link replacement is disabled if `email_track_clicks` is disabled. In the future we might consider to still do parial additions, such as source attribution and maybe redirects (to discuss).
refs https://github.com/TryGhost/Team/issues/1899
- Added `addEmailAttributionToUrl` method to MemberAttributionService. This adds both the source attribution (`rel=newsletter`) and member attribution (`?attribution_id=123&attribution_type=post`) to a URL.
- The URLHistory can now contain a new sort of items: `{type: 'post', id: 'post-id', time: 123}`.
- Updated frontend script to read `?attribution_id=123&attribution_type=post` from the URL and add it to the URLHistory + clear it from the URL.
- Wired up some external dependencies to LinkReplacementService and added some dummy code.
- Increased test coverage of attribution service
- Moved all logic that removes the subdirectory from a URL to the UrlTranslator instead of the AttributionBuilder
- The UrlTranslator now parses a URLHistoryItem to an object that can be used to build an Attribution instance
- Excluded sites with different domain from member id and attribution tracking
We're going to be adding more redirection logic into Ghost and it's
going to get confusing if we have names this generic. This makes it
clear which feature this service is related to.
Ideally in the future we can combine all of these into one redirects
service, but for now we will be running a specific service per feature
no issue
- fixed API returning "Invalid mobiledoc structure" errors when `mobiledoc:null` is sent in the payload alongside `lexical: '{...}'`
- updated Admin's `posts` and `pages` adapters to always add `?formats=mobiledoc,lexical` because the API doesn't return `lexical` by default
- added `lexical` attribute to Admin's Post model
- updated `lexical-editor` controller and related components to work with `lexical` always being a JSON string rather than a parsed object
- updated `<KoenigLexicalEditor>` to pass through the lexical state string as initial state and wired up the `onChange` prop
no issue
- bumped `@tryghost/admin-api-schema` to allow passthrough of the `lexical` property on post and page API endpoints
- prevented saving of blank document in the `mobiledoc` field if `lexical` is provided
- prevented API input containing both `mobiledoc` and `lexical` fields to avoid issues when both are present:
- not possible to know which content is latest/has precedence
- not possible to know which editor should be displayed in Admin
refs 9471384020
- previously added tests (any subsequent matcher updates) for browse endpoint were not using matchers that sufficiently covered the dynamic portions of the body
no issue
- left `mobiledoc` as the only default format added in the post/page input serializers for now to minimize API/test churn during these early stages of lexical development
- tested that the `lexical` field is not returned by default but can be requested via `?formats=lexical`
no issue
- similar to the `mobiledoc` field, the Content API should not return the source `lexical` field if requested via `?formats=`
- renamed `removeMobiledocFormat()` to `removeSourceFormats()` to better match it's behaviour
closes https://github.com/TryGhost/Team/issues/1884
- adds `post.lexical` ready for use by the lexical-powered editor re-write
- fulfils the same purpose as `posts.mobiledoc` so uses the same field properties
- added `lexical` to allowed formats in Post model so it won't be included by default in API responses meaning tests/snapshots don't need updating at present
closes https://github.com/TryGhost/Team/issues/1864
refs https://github.com/TryGhost/Team/issues/1881
- triggers free member email alert via event dispatch from member create method
- passes subscription/stripe data to member creation for paid members so free member alert can be ignored for them
- moves subscription created event being called from webhook controller to `linkSubscription`, allows creating subscription events for all new subscriptions instead of ones just via webhooks
refs https://github.com/TryGhost/Team/issues/1865
- refactors staff service to listen to member and subscription events
- triggers email alerts based on events instead of directly calling the service
- removes staff service dependency for members api
no issue
- Bumped into these tests when doing cleanup in the notifications service. Having full snapshot of requests is useful to have as a sanity check, so migrated this test suite quickly.
closes https://github.com/TryGhost/Team/issues/1772
- The user facing side of comments recently replaced `bio` with `expertise`.
- To remain consistent we replaced all the references of `bio` with `expertise` throughout the codebase.
- This includes a database column name changing migration, within the `members` table.
- Bumped up the comments-ui version to a new minor (0.10.x) as its a breaking change.
fixes https://github.com/TryGhost/Toolbox/issues/356
- this feature allows site Administrators to view a history log of staff
actions on their site so they can audit when and by whom that something happened
- this commit promotes the History log to GA
- this prevents the referrer/referer header being sent for requests that go to external domains
- this in turn prevents preview URLs from appearing in the analytics of sites that are linked to and clicked on from previews
- otherwise, preview URLs can be leaked to the owners of the linked and clicked sites
refs https://github.com/TryGhost/Team/issues/1795
- Snapshots help us detect unexpected changes in the `<head>` of all sites (e.g., newly introduced script tags)
- Added ghost_head tests for comment count helper
refs https://github.com/TryGhost/Team/issues/1871
This commit adds a test to the serialize method of `post-emaiserializer`. It checks whether the generated email HTML is valid and standard HTML5 and that all properties are escaped.
To do this validation, I depend on the new `html-validate` dev dependency. Just parsing the HTML with a HTML parser is not enough to guarantee that the HTML is okay.
Apart from that this fixes:
- Removed the sanitizeHTML method and replaced it with normal HTML escaping. We don't want to allow any HTML in the escaped fields. Whereas `sanitizeHTML` still allows valid HTML, but we don't want that and want the same behaviour as on the site. E.g., a post with a title `All your need to know about the <br /> tag` should actually render the same title and non-html content, being `All your need to know about the <br /> tag`
- The file, nft and audio card didn't (always) escape the injected HTML fields (new version @tryghost/kg-default-cards)
- `@tryghost/string` is bumped because it contains the new escapeHtml method
refs https://github.com/TryGhost/Ghost/pull/15375
- we currently pass all properties for the `tags` property of a
`page`/`post` body down further into Ghost, which is causing issues
because it's handling properties it doesn't expect
- this is showing up because it's triggering save history events for
tags when a post is edited
- this commit introduces a clean util which has an allowlist of
properties allows on tag relations
- this list was taken from the schema: 128f8fb006/packages/admin-api-schema/lib/schemas/posts.json (L214-L227)
