Ensured Admin API members resource only returns known fields (#12240)

refs #12055

As part of the work in TryGhost/Members#206 we load the stripeCustomers relation on the member model, and we do not want this to be part of the API response. The changes here include a refactor but the main thing is that the serialized object is explicit and does not include unexpected or unknown fields.

* Moved mapMember out of mapper file

This cleans up the serializer a bit by keeping it's functionality all in
one place, rather than a shared mapper file

* Refactored members controller to return models

Previously the controller was calling toJSON, which is serialization,
this updates the controller to only deal with models, leaving all of the
serialization to the serializer!

* Refactored members serializer

This adds typings to all of the methods/functions in the serializer, as
well as making the serializating explicit, rather than returning the
result of toJSON, we explicitly set the properties we expect to be on
the output object. This protects us against accidental API changes in
the future.

core/server/api/canary/members.js

@@ -165,12 +165,8 @@ module.exports = {
         async query(frame) {
             frame.options.withRelated = ['labels', 'stripeSubscriptions', 'stripeSubscriptions.customer'];
             const page = await membersService.api.members.list(frame.options);
-            const members = page.data.map(model => model.toJSON(frame.options));
 
-            return {
-                members: members,
-                meta: page.meta
-            };
+            return page;
         }
     },
 
@@ -192,7 +188,7 @@ module.exports = {
                 });
             }
 
-            return model.toJSON(frame.options);
+            return model;
         }
     },
 
@@ -240,7 +236,7 @@ module.exports = {
                     await membersService.api.sendEmailWithMagicLink({email: member.get('email'), requestedType: frame.options.email_type});
                 }
 
-                return member.toJSON(frame.options);
+                return member;
             } catch (error) {
                 if (error.code && error.message.toLowerCase().indexOf('unique') !== -1) {
                     throw new errors.ValidationError({
@@ -305,7 +301,7 @@ module.exports = {
 
                 await member.load(['stripeSubscriptions.customer']);
 
-                return member.toJSON(frame.options);
+                return member;
             } catch (error) {
                 if (error.code && error.message.toLowerCase().indexOf('unique') !== -1) {
                     throw new errors.ValidationError({
@@ -363,7 +359,7 @@ module.exports = {
                 });
             }
 
-            return model.toJSON(frame.options);
+            return model;
         }
     },
 
@@ -426,12 +422,8 @@ module.exports = {
         async query(frame) {
             frame.options.withRelated = ['labels', 'stripeSubscriptions', 'stripeSubscriptions.customer'];
             const page = await membersService.api.members.list(frame.options);
-            const members = page.data.map(model => model.toJSON(frame.options));
 
-            return {
-                members: members,
-                meta: page.meta
-            };
+            return page;
         }
     },
 

core/server/api/canary/utils/serializers/output/members.js

@@ -1,76 +1,186 @@
-const {i18n} = require('../../../../../lib/common');
-const errors = require('@tryghost/errors');
+//@ts-check
 const debug = require('ghost-ignition').debug('api:canary:utils:serializers:output:members');
-const mapper = require('./utils/mapper');
 const {unparse} = require('@tryghost/members-csv');
 
 module.exports = {
-    hasActiveStripeSubscriptions(data, apiConfig, frame) {
-        frame.response = data;
-    },
-    browse(data, apiConfig, frame) {
-        debug('browse');
+    hasActiveStripeSubscriptions: createSerializer('hasActiveStripeSubscriptions', passthrough),
 
-        frame.response = {
-            members: data.members.map(member => mapper.mapMember(member, frame)),
-            meta: data.meta
-        };
-    },
+    browse: createSerializer('browse', paginatedMembers),
+    read: createSerializer('read', singleMember),
+    edit: createSerializer('edit', singleMember),
+    add: createSerializer('add', singleMember),
+    editSubscription: createSerializer('editSubscription', singleMember),
 
-    add(data, apiConfig, frame) {
-        debug('add');
+    exportCSV: createSerializer('exportCSV', exportCSV),
 
-        frame.response = {
-            members: [mapper.mapMember(data, frame)]
-        };
-    },
-
-    edit(data, apiConfig, frame) {
-        debug('edit');
-
-        frame.response = {
-            members: [mapper.mapMember(data, frame)]
-        };
-    },
-
-    read(data, apiConfig, frame) {
-        debug('read');
-
-        if (!data) {
-            return Promise.reject(new errors.NotFoundError({
-                message: i18n.t('errors.api.members.memberNotFound')
-            }));
-        }
-
-        frame.response = {
-            members: [mapper.mapMember(data, frame)]
-        };
-    },
-
-    exportCSV(data, apiConfig, frame) {
-        debug('exportCSV');
-
-        const members = data.members.map((member) => {
-            return mapper.mapMember(member, frame);
-        });
-
-        frame.response = unparse(members);
-    },
-
-    importCSV(data, apiConfig, frame) {
-        debug('importCSV');
-        frame.response = data;
-    },
-
-    stats(data, apiConfig, frame) {
-        debug('stats');
-        frame.response = data;
-    },
-
-    editSubscription(data, apiConfig, frame) {
-        debug('editSubscription');
-        frame.response = {
-            members: [mapper.mapMember(data, frame)]
-        };
-    }
+    importCSV: createSerializer('importCSV', passthrough),
+    stats: createSerializer('stats', passthrough)
 };
+
+/**
+ * @template PageMeta
+ *
+ * @param {{data: import('bookshelf').Model[], meta: PageMeta}} page
+ * @param {APIConfig} _apiConfig
+ * @param {Frame} frame
+ *
+ * @returns {{members: SerializedMember[], meta: PageMeta}}
+ */
+function paginatedMembers(page, _apiConfig, frame) {
+    return {
+        members: page.data.map(model => serializeMember(model, frame.options)),
+        meta: page.meta
+    };
+}
+
+/**
+ * @param {import('bookshelf').Model} model
+ * @param {APIConfig} _apiConfig
+ * @param {Frame} frame
+ *
+ * @returns {{members: SerializedMember[]}}
+ */
+function singleMember(model, _apiConfig, frame) {
+    return {
+        members: [serializeMember(model, frame.options)]
+    };
+}
+
+/**
+ * @template PageMeta
+ *
+ * @param {{data: import('bookshelf').Model[], meta: PageMeta}} page
+ * @param {APIConfig} _apiConfig
+ * @param {Frame} frame
+ *
+ * @returns {string} - A CSV string
+ */
+function exportCSV(page, _apiConfig, frame) {
+    debug('exportCSV');
+
+    const members = page.data.map(model => serializeMember(model, frame.options));
+
+    return unparse(members);
+}
+
+/**
+ * @param {import('bookshelf').Model} member
+ * @param {object} options
+ *
+ * @returns {SerializedMember}
+ */
+function serializeMember(member, options) {
+    const json = member.toJSON(options);
+
+    let comped = false;
+    if (json.stripe && json.stripe.subscriptions) {
+        const hasCompedSubscription = !!json.stripe.subscriptions.find(
+            /**
+             * @param {SerializedMemberStripeSubscription} sub
+             */
+            function (sub) {
+                return sub.plan.nickname === 'Complimentary';
+            }
+        );
+        if (hasCompedSubscription) {
+            comped = true;
+        }
+    }
+
+    return {
+        id: json.id,
+        uuid: json.uuid,
+        email: json.email,
+        name: json.name,
+        note: json.note,
+        geolocation: json.geolocation,
+        subscribed: json.subscribed,
+        created_at: json.created_at,
+        updated_at: json.updated_at,
+        labels: json.labels,
+        stripe: json.stripe,
+        avatar_image: json.avatar_image,
+        comped: comped
+    };
+}
+
+/**
+ * @template Data
+ * @param {Data} data
+ * @returns Data
+ */
+function passthrough(data) {
+    return data;
+}
+
+/**
+ * @template Data
+ * @template Response
+ * @param {string} debugString
+ * @param {(data: Data, apiConfig: APIConfig, frame: Frame) => Response} serialize - A function to serialize the data into an object suitable for API response
+ *
+ * @returns {(data: Data, apiConfig: APIConfig, frame: Frame) => void}
+ */
+function createSerializer(debugString, serialize) {
+    return function serializer(data, apiConfig, frame) {
+        debug(debugString);
+        const response = serialize(data, apiConfig, frame);
+        frame.response = response;
+    };
+}
+
+/**
+ * @typedef {Object} SerializedMember
+ * @prop {string} id
+ * @prop {string} uuid
+ * @prop {string} email
+ * @prop {string=} name
+ * @prop {string=} note
+ * @prop {null|string} geolocation
+ * @prop {boolean} subscribed
+ * @prop {string} created_at
+ * @prop {string} updated_at
+ * @prop {string[]} labels
+ * @prop {null|SerializedMemberStripeData} stripe
+ * @prop {string} avatar_image
+ * @prop {boolean} comped
+ */
+
+/**
+ * @typedef {Object} SerializedMemberStripeData
+ * @prop {SerializedMemberStripeSubscription[]} subscriptions
+ */
+
+/**
+ * @typedef {Object} SerializedMemberStripeSubscription
+ *
+ * @prop {string} id
+ * @prop {string} status
+ * @prop {string} start_date
+ * @prop {string} default_payment_card_last4
+ * @prop {string} current_period_end
+ * @prop {boolean} cancel_at_period_end
+ *
+ * @prop {Object} customer
+ * @prop {string} customer.id
+ * @prop {null|string} customer.name
+ * @prop {string} customer.email
+ *
+ * @prop {Object} plan
+ * @prop {string} plan.id
+ * @prop {string} plan.nickname
+ * @prop {number} plan.amount
+ * @prop {string} plan.currency
+ * @prop {string} plan.currency_symbol
+ */
+
+/**
+ * @typedef {Object} APIConfig
+ * @prop {string} docName
+ * @prop {string} method
+ */
+
+/**
+ * @typedef {Object<string, any>} Frame
+ * @prop {Object} options
+ */

core/server/api/canary/utils/serializers/output/utils/mapper.js

@@ -142,23 +142,6 @@ const mapAction = (model, frame) => {
     return attrs;
 };
 
-const mapMember = (model, frame) => {
-    const jsonModel = model.toJSON ? model.toJSON(frame.options) : model;
-
-    if (_.get(jsonModel, 'stripe.subscriptions')) {
-        let compedSubscriptions = _.get(jsonModel, 'stripe.subscriptions').filter(sub => (sub.plan.nickname === 'Complimentary'));
-        const hasCompedSubscription = !!(compedSubscriptions.length);
-
-        // NOTE: `frame.options.fields` has to be taken into account in the same way as for `stripe.subscriptions`
-        //       at the moment of implementation fields were not fully supported by members endpoints
-        Object.assign(jsonModel, {
-            comped: hasCompedSubscription
-        });
-    }
-
-    return jsonModel;
-};
-
 const mapLabel = (model, frame) => {
     const jsonModel = model.toJSON ? model.toJSON(frame.options) : model;
     return jsonModel;
@@ -189,5 +172,4 @@ module.exports.mapIntegration = mapIntegration;
 module.exports.mapSettings = mapSettings;
 module.exports.mapImage = mapImage;
 module.exports.mapAction = mapAction;
-module.exports.mapMember = mapMember;
 module.exports.mapEmail = mapEmail;