closes https://github.com/TryGhost/Toolbox/issues/399
- The MemberCreatedEvent event is more accurate representation of the limit nature - counting the number of members created. The previous MemberSubscribeEvent was slightly hacky solution because a member could be subscribed/unsubscribed multiple times and distorting the limit counts.
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/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)
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
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/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
