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.
We're planning to change this from a warning to an error and need to
clean the codebase up before we do so.
In all of these cases the shadowing was known about and was not
causing unexpected behaviour, so the refactor consists entirely of
renaming, rather than refactoring/bug fixes.
- I want to upgrade no-shadow to an error, but to do this I need to resolve the outstanding warnings
- This is not all the warnings, just the ones that were easy to fix
closesTryGhost/Team#2007
- uses request context to add referrer source and medium for a new member
- uses integration name as referrer medium if exists
- renames `refSource`, `refMedium` and `refUrl` to `referrerSource`, `referrerMedium` and `referrerUrl` respectively for consistent naming across files and usages
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 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
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
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/Ghost/issues/15190
refs https://github.com/TryGhost/framework/pull/76
- log output always uses UTC timestamps, but it may be desirable to
configure logs to use the local machine timezone
- a new config option has been added to `@tryghost/logging` so you can
switch the logs to the local timezone
- this commit bumps the package and sets the default config option to
`false`, so it doesn't suddenly change the timezone of the logs
- docs will be updated soon but if you'd like to use the
timezone-altered timestamps, you can set `logging.useLocalTime` to
`true`
- credits to https://github.com/levee223 for the implementation and PR
fixes https://github.com/TryGhost/Team/issues/1859
**Problem:**
When for some reason a member has an active subscription (or legacy comped subscription) for product A, and a comped subscription for product B. You cannot remove comped subscription B.
**Fixed by:**
Updating the API to allow more flexible product changes on members.
- Allow the removal of (comped) products on a member, as long as that product doesn't have a related subscription
- (still) allow the addition of comped products to a member, as long as that member doesn't have other active subscriptions. This matches the existing behaviour, but now this is only checked for added products.
- Includes tests for these edge cases
fixes https://github.com/TryGhost/Team/issues/1859
**Problem:**
When for some reason a member has an active subscription (or legacy comped subscription) for product A, and a comped subscription for product B. You cannot remove comped subscription B.
**Fixed by:**
Updating the API to allow more flexible product changes on members.
- Allow the removal of (comped) products on a member, as long as that product doesn't have a related subscription
- (still) allow the addition of comped products to a member, as long as that member doesn't have other active subscriptions. This matches the existing behaviour, but now this is only checked for added products.
- Includes tests for these edge cases
no issue
- Return was missing for `res.end` if an invalid subscription_id was passed
- Added explicit `text/plain` `Content-Type` headers to error messages to avoid MIME sniffing
Signed-off-by: Elijah Conners <business@elijahpepe.com>
Co-authored-by: Simon Backx <simon@ghost.org>
refs https://github.com/TryGhost/Team/issues/1826
Geolocation was prev. loaded after member was created and updated on existing member. this was mostly due to historical context where we couldn't store data on magic link token.
Since email alerts go out at the time of member creation, this flow missed out on attaching member's location to email.
This change -
- stores request ip when a member asks for magic link in the token
- loads request ip from token when member uses magic link, and for new members loads their geolocation and stores it with member creation
refs TryGhost/Team#1826
- triggers paid subscription cancellation alert for staff users
- passes tier and subscription information for the email - loads tier info from DB for the subscription tier
refs https://github.com/TryGhost/Toolbox/issues/387
- There will three distinct verification limits soon. To keep the naming clear "configThreshold" would be too generic/confusing to use.
- Introduced jsdoc descriptions for the "source" parameter, which will be corelating with each new config parameter ("apiTriggerThreshold", "importTriggerThreshold", "adminTriggerThreshold", etc.). This should give a better visibility into parameters we are dealing in this area.
refs https://github.com/TryGhost/Team/issues/1833
refs https://github.com/TryGhost/Team/issues/1834
We've added the attribution property to subscription and signup events when the
flag is enabled. The attributions resource is fetched by creating multiple relations
on the model, rather than polymorphic as we ran into issues with that as they can't
be nullable/optional.
The parse-member-event structure has been updated to make it easier to work with,
specifically `getObject` is only used when the event is clickable, and there is now a
join property which makes it easier to join the action and the object.
refs https://github.com/TryGhost/Team/issues/1833
refs https://github.com/TryGhost/Team/issues/1834
We've added the attribution property to subscription and signup events when the
flag is enabled. The attributions resource is fetched by creating multiple relations
on the model, rather than polymorphic as we ran into issues with that as they can't
be nullable/optional.
The parse-member-event structure has been updated to make it easier to work with,
specifically `getObject` is only used when the event is clickable, and there is now a
join property which makes it easier to join the action and the object.
closes https://github.com/TryGhost/Toolbox/issues/386
- When the API request was made using staff token the source attribution was "user" instead of "api". Misattribution caused ripple effects in limit service.
- The fix also adds a new combination of data available on the `req` object - both `user` and `api_key` can be present when the request is done using a staff (user) token. Having both pieces of data on the request object gives more context for business logic, did not find a good reason to keep it "pure" with either `api_key` or `user` property.
refs https://github.com/TryGhost/Toolbox/issues/386
- Reusing tontext mapping logic to improve maintainability. It seems like the `update` method was not updated properly or intentionally was left out from 'import' source as that should not ever happen theoretically. Probably the latter is most likely.
- My reasoning on reusing same context to source mapping is: it is better to attribute an appropriate "import" source here. Who knows, maybe we'll have logic in the future where the importer updates instead of skipping existing members. It would not make sense to attribute the source to 'member' in that case, amirite?
- This refactor also makes maintainability of this code way easier
refs https://github.com/TryGhost/Team/issues/1728
- previously, we allowed a member to be mapped to multiple tiers simultaneously as an edge case, in case they managed to signup via another subscription
- since this was always an edge case and not supported, to simplify the flows going forward now that complimentary members can also upgrade, in case of an active subscription we'll always just attach the associated tier to member and remove all other tiers mapped to it
refs https://github.com/TryGhost/Team/issues/1808
refs https://github.com/TryGhost/Team/issues/1809
refs https://github.com/TryGhost/Team/issues/1820
refs https://github.com/TryGhost/Team/issues/1814
### Changes in `member-events` package
- Added MemberCreatedEvent (event, not model)
- Added SubscriptionCreatedEvent (event, not model)
### Added `member-attribution` package (new)
- Added the AttributionBuilder class which is able to convert a url history to an attribution object (exposed as getAttribution on the service itself, which handles the dependencies)
```
[{
"path": "/",
"time": 123
}]
```
to
```
{
"url": "/",
"id": null,
"type": "url"
}
```
- event handler listens for MemberCreatedEvent and SubscriptionCreatedEvent and creates the corresponding models in the database.
### Changes in `members-api` package
- Added urlHistory to `sendMagicLink` endpoint body + convert the urlHistory to an attribution object that is stored in the tokenData of the magic link (sent by Portal in this PR: https://github.com/TryGhost/Portal/pull/256).
- Added urlHistory to `createCheckoutSession` endpoint + convert the urlHistory to attribution keys that are saved in the Stripe Session metadata (sent by Portal in this PR: https://github.com/TryGhost/Portal/pull/256).
- Added attribution data property to member repository's create method (when a member is created)
- Dispatch MemberCreatedEvent with attribution
### Changes in `members-stripe-service` package (`ghost/stripe`)
- Dispatch SubscriptionCreatedEvent in WebhookController on subscription checkout (with attribution from session metadata)
- cleaned up unused dependencies
- adds missing dependencies that are used in the code
- this should help us be more explicit about the dependencies a package
uses
- because of how the npm scripts were set up, we were running the full
Admin integration tests during the unit tests phase of CI
- this commit renames the majority of `test` to `test:unit` in the
package.json files, and aliases `test` to `test:unit`
- special packages like Admin have no-op'd `test:unit` scripts so we
don't end up running its tests
refs https://github.com/TryGhost/Team/issues/1726
Free trial offers don't have a Stripe coupon created for them, as the trial is directly added to checkout session. So for mapping a subscription to offer, we pass the offer id directly from checkout metadata to link the subscription in backend with right offer data. This also handles the case where the offer id against a subscription can get overwritten for a subsequent subscription event, as the sub event from Stripe doesn't has the trial offer info.
- handles storing an offer id for a subscription
- updates member detail in Admin to show the offer info for a subscription
refs https://github.com/TryGhost/Team/issues/1726
- free trial offers don't need a stripe coupon created for them
- checkout sessions for free trial offers ignore stripe coupon and directly pass the trial days value
- trial days of an offer take precedence over trial days added as default to a tier
refs https://github.com/TryGhost/Team/issues/1723
- Added count.replies to comments
- Added replies endpoint
- Limited returned replies to 3.
- Replaced likes_count with count.likes in comments
- Instead of fetching all the likes of a comment to determine the total count, we'll now use count.likes
- Instead of fetching all the likes of a comment to determine whether a member liked a comment, we'll now use count.liked (which returns the amount of likes of the current member, being 0 or 1). This is mapped to `liked` to make it more natural to work with.
The `members.test.snap` file changed because we no longer include `liked: false` if we didn't fetch the liked relation. And in the comments events of the activity feed the liked property is therefore removed.
These changes requires an update to the `bookshelf-include-count` plugin:
- Updated to also work for nested relations
- This moves the count queries from the `bookshelf-include-count` plugin to the `countRelations` method of each model.
- Updated to keep the counts after saving a model (crud.edit didn't return the counts before)
refs https://github.com/TryGhost/Toolbox/issues/345
- this commit bumps `eslint-plugin-ghost`, which bumps compatiblity to
2022
- this also removes a lot of the manually-added
`parserOptions.ecmaVersion` that we had in imported packages, in favor
of the value set in `eslint-plugin-ghost`
Without this check, an inactive price in our database will just be
reactivated each time it is required. This can cause issues when
prices have been deleted.
By adding this constraint to the query, we will create a new price in
Stripe and our database when attempting to use an inactive price, this
is particularly useful when trying to fix problems caused by Stripe
prices being deleted.
refs https://github.com/TryGhost/Team/issues/1724
With free trials, members can start subscriptions with a trial period. This change stores the information about trial start and end date for every subscription so it can be shown on Admin/Portal for member.
- adds new `trial_start_at` column for storing trial start date on Stripe subscription. Will in most cases match the start of subscription date.
- adds new `trial_end_at` column for storing trial end date on Stripe subscription.
- wires storing trial start and end values on stripe subscription
refs https://github.com/TryGhost/Team/issues/1724
- wires trial days stored on a tier to stripe checkout session creation
- removes deprecated `trial_from_plan` if trial days is set
refs https://github.com/TryGhost/Team/issues/1716
- Adds the bio field to the API output
- Allow setting bio when updating the member
- Includes new E2E tests for the members API that were missing
refs https://github.com/TryGhost/Team/issues/1709
- New event type `comment_event` (comments and replies of a member in the activity feed)
- Includes member, post and parent relation by default
- Added new output mapper for ActivityFeed events
**Changes to `Comment` model:**
* **Only limit comment fetched to root comments when not authenticated as a user:**
`enforcedFilters` is applied to all queries, which is a problem because for the activity feed we also need to fetch comments which have a parent_id that is not null (`Member x replied to a comment`). The current filter in the model is specifically for the members API, not the admin API (so checking the user should fix that, not sure if that is a good pattern but couldn’t find a better alternative).
* **Only set default relations for comments when withRelated is empty or not set:**
`defaultRelations`: Right now, for every fetch it would force all these relations. But we don’t need all those relations for the activity feed; So I updated the pattern to only set the default relations when it is empty (which we also do on a couple of other places and seems like a good pattern). I also updated the comments-ui frontend to not send ?include
refs https://github.com/TryGhost/Team/issues/1717
- Updates last_commented_at and last_seen_at (only once a day)
- Used the LastSeenAtUpdater, so we can combine updating last_commented_at and last_seen_at in one query + used same pattern
- Updated comments service to await emails in order to make E2E tests more stable (as we don't have any method to await emails and test emails otherwise). This removed the email sending logic from the `onCreated` hook of the model.
refs https://github.com/TryGhost/Toolbox/issues/354
- these repository links made sense when they were in different repos
and published to NPM but we don't publish these packages any more
- this commit deletes those keys from the files
- we're going to be pinning all dependencies within the monorepo
- this shouldn't change anything anyway because we're using the same
version across all packages
- these packages are split apart for local development, but will be
bundled into Ghost when publishing
- therefore, these packages won't be published so we are resetting the
versions to make them cleaner
refs https://github.com/TryGhost/Team/issues/1174
This paves the way for Ghost to be able to redirect to the referrer
page when dealign with signup magic links. We pass the referrer for
all types of magic links however, to allow extension of this
functionality in the future.
We've also removed the concept of `requestSrc` which has been unused
for a while now.
refs https://github.com/TryGhost/Team/issues/1674
- While preparing the changes had a look around and made small refactors to understand the codebase a little better. In general it's best to keep the method parameters as small and precise as possible instead of passing around a "bag-of-all-the-things" like "data" around
refs https://github.com/TryGhost/Team/issues/1674
- While preparing the changes had a look around and made small refactors to understand the codebase a little better. In general it's best to keep the method parameters as small and precise as possible instead of passing around a "bag-of-all-the-things" like "data" around
refs https://github.com/TryGhost/Team/issues/1526
We weren't using the `req.body.customerEmail` to load a member and
check their existing tiers, this meant that existing members which
were signed out and attempted to create a stripe checkout session were
able to.
refs https://ghost.slack.com/archives/C02G9E68C/p1652980792270029
When bulk unsubscribing members, the number of deleted newsletter relations are returned instead of the number of members with newsletters that were cleared. This update deletes newsletter relations on member_id, so we can return the count of members instead of newsletter relations that were deleted.
Tests in https://github.com/TryGhost/Ghost/pull/14871
- In case of an offer id present in stripe checkout session, the cadence and tier id values can be null
- this fixes the invalid 400 thrown on missing cadence value while creating stripe checkout session for offer
no issue
- Setting a dedicated source for imported members was already done with 455778662c
- This unifies the same source but keeps the usage of context setting on init alive
