From b3cfe326315a64127618bdb41f34029d2d5fb3ac Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 1 Nov 2018 20:34:48 +0000 Subject: [PATCH] [next] Story Improvements (#2054) * feat: improved logs * feat: improved scraper - added scraper debug graph call * feat: improved story closing * fix: fixed tests --- .../components/HistoryComment.spec.tsx | 4 +- .../profile/components/HistoryComment.tsx | 9 ++- .../containers/HistoryCommentContainer.tsx | 4 +- src/core/client/stream/test/fixtures.ts | 3 + src/core/server/app/middleware/csp/tenant.ts | 17 +++- src/core/server/app/url.ts | 27 ++++--- .../server/graph/tenant/loaders/stories.ts | 5 ++ .../server/graph/tenant/resolvers/query.ts | 2 + .../server/graph/tenant/resolvers/story.ts | 16 +++- .../server/graph/tenant/schema/schema.graphql | 66 ++++++++++++++-- src/core/server/logger.ts | 3 + src/core/server/models/story.ts | 62 ++++++++------- .../moderation/phases/storyClosed.spec.ts | 44 ++++++++--- .../comments/moderation/phases/storyClosed.ts | 12 ++- .../services/queue/tasks/scraper/index.ts | 77 +++++++++++++------ .../queue/tasks/scraper/rules/modified.ts | 1 + .../queue/tasks/scraper/rules/section.ts | 1 + .../server/services/stories/index.spec.ts | 61 +++++++++++++++ src/core/server/services/stories/index.ts | 55 ++++++++++++- 19 files changed, 377 insertions(+), 92 deletions(-) create mode 100644 src/core/server/services/stories/index.spec.ts diff --git a/src/core/client/stream/tabs/profile/components/HistoryComment.spec.tsx b/src/core/client/stream/tabs/profile/components/HistoryComment.spec.tsx index 2fe5ba906..4bf0c7b9d 100644 --- a/src/core/client/stream/tabs/profile/components/HistoryComment.spec.tsx +++ b/src/core/client/stream/tabs/profile/components/HistoryComment.spec.tsx @@ -15,7 +15,9 @@ it("renders correctly", () => { createdAt: "2018-07-06T18:24:00.000Z", replyCount: 4, story: { - title: "Story Title", + metadata: { + title: "Story Title", + }, }, conversationURL: "http://localhost/conversation", onGotoConversation: noop, diff --git a/src/core/client/stream/tabs/profile/components/HistoryComment.tsx b/src/core/client/stream/tabs/profile/components/HistoryComment.tsx index 827709ac3..0262474c4 100644 --- a/src/core/client/stream/tabs/profile/components/HistoryComment.tsx +++ b/src/core/client/stream/tabs/profile/components/HistoryComment.tsx @@ -18,7 +18,9 @@ export interface HistoryCommentProps { createdAt: string; replyCount: number | null; story: { - title: string | null; + metadata: { + title: string | null; + } | null; }; conversationURL: string; onGotoConversation: (e: React.MouseEvent) => void; @@ -27,7 +29,10 @@ export interface HistoryCommentProps { const HistoryComment: StatelessComponent = props => { return ( - + {"Story: {$title}"} {props.createdAt} diff --git a/src/core/client/stream/tabs/profile/containers/HistoryCommentContainer.tsx b/src/core/client/stream/tabs/profile/containers/HistoryCommentContainer.tsx index 6a9baa482..dae02c774 100644 --- a/src/core/client/stream/tabs/profile/containers/HistoryCommentContainer.tsx +++ b/src/core/client/stream/tabs/profile/containers/HistoryCommentContainer.tsx @@ -55,8 +55,10 @@ const enhanced = withSetCommentIDMutation( replyCount story { id - title url + metadata { + title + } } } `, diff --git a/src/core/client/stream/test/fixtures.ts b/src/core/client/stream/test/fixtures.ts index ba1910a22..2fd35fdfe 100644 --- a/src/core/client/stream/test/fixtures.ts +++ b/src/core/client/stream/test/fixtures.ts @@ -215,6 +215,9 @@ export const commentWithDeepestReplies = denormalizeComment({ }); export const baseStory = { + metadata: { + title: "title", + }, isClosed: false, comments: { edges: [], diff --git a/src/core/server/app/middleware/csp/tenant.ts b/src/core/server/app/middleware/csp/tenant.ts index 2db349699..840955fae 100644 --- a/src/core/server/app/middleware/csp/tenant.ts +++ b/src/core/server/app/middleware/csp/tenant.ts @@ -1,12 +1,13 @@ import builder from "content-security-policy-builder"; import { doesRequireSchemePrefixing, - extractParentsOrigin, + extractParentsURL, getOrigin, isURLSecure, prefixSchemeIfRequired, } from "talk-server/app/url"; import { Tenant } from "talk-server/models/tenant"; +import { isURLPermitted } from "talk-server/services/stories"; import { Request, RequestHandler } from "talk-server/types/express"; /** @@ -65,8 +66,13 @@ export function generateFrameOptions( return `allow-from ${getOrigin(tenant.domains[0])}`; } - // Grab the parent's hostname. - const parentsOrigin = extractParentsOrigin(req); + const parentsURL = extractParentsURL(req); + if (!parentsURL) { + return "deny"; + } + + // Grab the parent's origin. + const parentsOrigin = getOrigin(parentsURL); if (!parentsOrigin) { return "deny"; } @@ -88,6 +94,11 @@ export function generateFrameOptions( )}`; } + // Determine if this origin is allowed. + if (!isURLPermitted(tenant, parentsURL)) { + return "deny"; + } + // As we can only return a single domain in the `allow-from` directive as per // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options // We need to find the domain that is asking so we can respond with the right diff --git a/src/core/server/app/url.ts b/src/core/server/app/url.ts index de4686e24..37609fb19 100644 --- a/src/core/server/app/url.ts +++ b/src/core/server/app/url.ts @@ -70,12 +70,7 @@ export function prefixSchemeIfRequired(secure: boolean, url: string) { return url; } -/** - * extractParentsOrigin will pull the parent's origin out. - * - * @param req the request where we want to extract the parent's hostname from. - */ -export function extractParentsOrigin(req: Request) { +export function extractParentsURL(req: Request) { // The only two places this could be is in the referer header or the parentUrl // query parameter (injected by pym.js). If both of these are empty, then we // can't find anything. @@ -87,16 +82,30 @@ export function extractParentsOrigin(req: Request) { if (req.headers.referer && req.headers.referer.length > 0) { // If the header contains multiple values, return the first one. if (Array.isArray(req.headers.referer)) { - return getOrigin(req.headers.referer[0]); + return req.headers.referer[0]; } - return getOrigin(req.headers.referer); + return req.headers.referer; } // If the parentUrl query parameter is provided, then try to parse it. if (req.query.parentUrl) { - return getOrigin(req.query.parentUrl); + return req.query.parentUrl; } return null; } + +/** + * extractParentsOrigin will pull the parent's origin out. + * + * @param req the request where we want to extract the parent's hostname from. + */ +export function extractParentsOrigin(req: Request) { + const url = extractParentsURL(req); + if (!url) { + return null; + } + + return getOrigin(url); +} diff --git a/src/core/server/graph/tenant/loaders/stories.ts b/src/core/server/graph/tenant/loaders/stories.ts index 057eee7d6..0a9efb9e7 100644 --- a/src/core/server/graph/tenant/loaders/stories.ts +++ b/src/core/server/graph/tenant/loaders/stories.ts @@ -1,11 +1,13 @@ import DataLoader from "dataloader"; import TenantContext from "talk-server/graph/tenant/context"; +import { GQLStoryMetadata } from "talk-server/graph/tenant/schema/__generated__/types"; import { FindOrCreateStoryInput, retrieveManyStories, Story, } from "talk-server/models/story"; +import { scraper } from "talk-server/services/queue/tasks/scraper"; import { findOrCreate } from "talk-server/services/stories"; export default (ctx: TenantContext) => ({ @@ -14,4 +16,7 @@ export default (ctx: TenantContext) => ({ story: new DataLoader(ids => retrieveManyStories(ctx.mongo, ctx.tenant.id, ids) ), + debugScrapeMetadata: new DataLoader(urls => + Promise.all(urls.map(url => scraper.scrape(url))) + ), }); diff --git a/src/core/server/graph/tenant/resolvers/query.ts b/src/core/server/graph/tenant/resolvers/query.ts index 732e4db6f..c217e9388 100644 --- a/src/core/server/graph/tenant/resolvers/query.ts +++ b/src/core/server/graph/tenant/resolvers/query.ts @@ -8,6 +8,8 @@ const Query: GQLQueryTypeResolver = { me: (source, args, ctx) => ctx.user, discoverOIDCConfiguration: (source, { issuer }, ctx) => ctx.loaders.Auth.discoverOIDCConfiguration.load(issuer), + debugScrapeStoryMetadata: (source, { url }, ctx) => + ctx.loaders.Stories.debugScrapeMetadata.load(url), }; export default Query; diff --git a/src/core/server/graph/tenant/resolvers/story.ts b/src/core/server/graph/tenant/resolvers/story.ts index aeb5d2304..85a38dec3 100644 --- a/src/core/server/graph/tenant/resolvers/story.ts +++ b/src/core/server/graph/tenant/resolvers/story.ts @@ -1,3 +1,5 @@ +import { DateTime } from "luxon"; + import { GQLStoryTypeResolver } from "talk-server/graph/tenant/schema/__generated__/types"; import { decodeActionCounts } from "talk-server/models/action"; import { Story } from "talk-server/models/story"; @@ -5,8 +7,20 @@ import { Story } from "talk-server/models/story"; const Story: GQLStoryTypeResolver = { comments: (story, input, ctx) => ctx.loaders.Comments.forStory(story.id, input), - // TODO: implement this. isClosed: () => false, + closedAt: (story, input, ctx) => { + if (story.closedAt) { + return story.closedAt; + } + + if (ctx.tenant.autoCloseStream && ctx.tenant.closedTimeout) { + return DateTime.fromJSDate(story.created_at) + .plus(ctx.tenant.closedTimeout) + .toJSDate(); + } + + return null; + }, actionCounts: story => decodeActionCounts(story.action_counts), commentCounts: story => story.comment_counts, }; diff --git a/src/core/server/graph/tenant/schema/schema.graphql b/src/core/server/graph/tenant/schema/schema.graphql index c63c887d7..241a9d4f8 100644 --- a/src/core/server/graph/tenant/schema/schema.graphql +++ b/src/core/server/graph/tenant/schema/schema.graphql @@ -1211,6 +1211,49 @@ enum COMMENT_SORT { RESPECT_DESC } +""" +StoryMetadata stores all the metadata that is scraped using the scraping tools +inside Talk. Talk utilizes [metascraper](https://metascraper.js.org/) which uses +a variety of filters derived from [The Open Graph Protocol](https://ogp.me/) to +scan for metadata on the page. +""" +type StoryMetadata { + """ + title stores the scraped title from the Story page. + """ + title: String + + """ + author stores the scraped author from the Story page. + """ + author: String + + """ + description stores the scraped description from the Story page. + """ + description: String + + """ + image stores the scraped image from the Story page. + """ + image: String + + """ + publishedAt stores the scraped publication date from the Story page. + """ + publishedAt: Time + + """ + modifiedAt stores the scraped modified date from the Story page. + """ + modifiedAt: Time + + """ + section stores the scraped section from the Story page. + """ + section: String +} + """ Story is an Article or Page where Comments are written on by Users. """ @@ -1226,9 +1269,14 @@ type Story { url: String! """ - title is the title of the scraped Story. + metadata stores the scraped metadata from the Story page. """ - title: String + metadata: StoryMetadata + + """ + scrapedAt is the Time that the Story had it's metadata scraped at. + """ + scrapedAt: Time """ comments are the comments on the Story. @@ -1245,11 +1293,6 @@ type Story { """ actionCounts: ActionCounts! @auth(roles: [ADMIN, MODERATOR]) - """ - author is the authors listed in the meta tags for the Story. - """ - author: String - """ closedAt is the Time that the Story is closed for commenting. """ @@ -1338,6 +1381,13 @@ type Query { """ discoverOIDCConfiguration(issuer: String!): DiscoveredOIDCConfiguration @auth(roles: [ADMIN]) + + """ + debugScrapeStoryMetadata will return the information that Talk was able to + scrape from the given URL. No data will be saved related to the Story based + on this scrape. + """ + debugScrapeStoryMetadata(url: String!): StoryMetadata @auth(roles: [ADMIN]) } ################################################################################ @@ -1679,7 +1729,7 @@ SettingsInput is the partial type of the Settings type for performing mutations. """ input SettingsInput { """ - domains will return a given list of whitelisted domains. + domains will return a given list of domains that hosts Stories. """ domains: [String!] diff --git a/src/core/server/logger.ts b/src/core/server/logger.ts index 51b67be3e..7a2128124 100644 --- a/src/core/server/logger.ts +++ b/src/core/server/logger.ts @@ -18,6 +18,9 @@ function getStreams() { const logger = bunyan.createLogger({ name: "talk", + + // Include file references in log entries. + src: true, serializers, streams: getStreams(), level: config.get("logging_level") as LogLevelString, diff --git a/src/core/server/models/story.ts b/src/core/server/models/story.ts index f3a59552d..b6ac62d25 100644 --- a/src/core/server/models/story.ts +++ b/src/core/server/models/story.ts @@ -3,7 +3,10 @@ import uuid from "uuid"; import { Omit } from "talk-common/types"; import { dotize } from "talk-common/utils/dotize"; -import { GQLCOMMENT_STATUS } from "talk-server/graph/tenant/schema/__generated__/types"; +import { + GQLCOMMENT_STATUS, + GQLStoryMetadata, +} from "talk-server/graph/tenant/schema/__generated__/types"; import { EncodedActionCounts } from "talk-server/models/action"; import { ModerationSettings } from "talk-server/models/settings"; import { TenantResource } from "talk-server/models/tenant"; @@ -23,20 +26,21 @@ export interface CommentStatusCounts { export interface Story extends TenantResource { readonly id: string; - url: string; - scraped?: Date; - closedAt?: Date; - closedMessage?: string; - created_at: Date; - modified_date?: Date; - title?: string; - description?: string; - image?: string; - section?: string; - subsection?: string; - author?: string; - publication_date?: Date; + /** + * url is the URL to the Story page. + */ + url: string; + + /** + * metadata stores the scraped metadata from the Story page. + */ + metadata?: GQLStoryMetadata; + + /** + * scrapedAt is the Time that the Story had it's metadata scraped at. + */ + scrapedAt?: Date; /** * action_counts stores all the action counts for all Comment's on this Story. @@ -54,6 +58,17 @@ export interface Story extends TenantResource { * specific Story. */ settings?: Partial; + + /** + * closedAt is the date that the Story was forced closed at, or false to + * indicate that the story was re-opened. + */ + closedAt?: Date | false; + + /** + * created_at is the date that the Story was added to the Talk database. + */ + created_at: Date; } export interface UpsertStoryInput { @@ -68,8 +83,6 @@ export async function upsertStory( ) { const now = new Date(); - // TODO: verify that the url for the given Story is whitelisted by the tenant. - // Create the story, optionally sourcing the id from the input, additionally // porting in the tenant_id. const update: { $setOnInsert: Story } = { @@ -85,7 +98,7 @@ export async function upsertStory( // Perform the find and update operation to try and find and or create the // story. - const { value: story } = await collection(db).findOneAndUpdate( + const result = await collection(db).findOneAndUpdate( { url, tenant_id: tenantID, @@ -100,15 +113,8 @@ export async function upsertStory( returnOriginal: false, } ); - if (!story) { - return null; - } - if (!story.scraped) { - // TODO: create scrape job to collect story metadata - } - - return story; + return result.value || null; } /** @@ -127,7 +133,7 @@ export async function updateCommentStatusCount( id: string, commentStatusCounts: Partial ) { - const { value: story } = await collection(mongo).findOneAndUpdate( + const result = await collection(mongo).findOneAndUpdate( { id, tenant_id: tenantID, @@ -140,7 +146,7 @@ export async function updateCommentStatusCount( { returnOriginal: false } ); - return story; + return result.value || null; } function createEmptyCommentCounts(): CommentStatusCounts { @@ -283,5 +289,5 @@ export async function updateStoryActionCounts( { returnOriginal: false } ); - return result.value; + return result.value || null; } diff --git a/src/core/server/services/comments/moderation/phases/storyClosed.spec.ts b/src/core/server/services/comments/moderation/phases/storyClosed.spec.ts index c562c0318..b1410aacd 100644 --- a/src/core/server/services/comments/moderation/phases/storyClosed.spec.ts +++ b/src/core/server/services/comments/moderation/phases/storyClosed.spec.ts @@ -1,3 +1,5 @@ +import { DateTime } from "luxon"; + import { Comment } from "talk-server/models/comment"; import { Story } from "talk-server/models/story"; import { Tenant } from "talk-server/models/tenant"; @@ -6,14 +8,28 @@ import { storyClosed } from "talk-server/services/comments/moderation/phases/sto describe("storyClosed", () => { it("throws an error when the story is closed", () => { - const story = { closedAt: new Date() }; + expect(() => + storyClosed({ + story: { closedAt: new Date() } as Story, + tenant: {} as Tenant, + comment: {} as Comment, + author: {} as User, + }) + ).toThrow(); + + storyClosed({ + story: {} as Story, + tenant: { autoCloseStream: true } as Tenant, + comment: {} as Comment, + author: {} as User, + }); expect(() => storyClosed({ - story: story as Story, - tenant: (null as any) as Tenant, - comment: (null as any) as Comment, - author: (null as any) as User, + story: { created_at: new Date() } as Story, + tenant: { autoCloseStream: true, closedTimeout: -6000 } as Tenant, + comment: {} as Comment, + author: {} as User, }) ).toThrow(); }); @@ -23,19 +39,23 @@ describe("storyClosed", () => { expect( storyClosed({ - story: { closedAt: new Date(now.getTime() + 60000) } as Story, - tenant: (null as any) as Tenant, - comment: (null as any) as Comment, - author: (null as any) as User, + story: { + closedAt: DateTime.fromJSDate(now) + .plus(60000) + .toJSDate(), + } as Story, + tenant: {} as Tenant, + comment: {} as Comment, + author: {} as User, }) ).toBeUndefined(); expect( storyClosed({ story: {} as Story, - tenant: (null as any) as Tenant, - comment: (null as any) as Comment, - author: (null as any) as User, + tenant: {} as Tenant, + comment: {} as Comment, + author: {} as User, }) ).toBeUndefined(); }); diff --git a/src/core/server/services/comments/moderation/phases/storyClosed.ts b/src/core/server/services/comments/moderation/phases/storyClosed.ts index 1b07c8413..d08bdf72b 100644 --- a/src/core/server/services/comments/moderation/phases/storyClosed.ts +++ b/src/core/server/services/comments/moderation/phases/storyClosed.ts @@ -6,10 +6,20 @@ import { // This phase checks to see if the story being processed is closed or not. export const storyClosed: IntermediateModerationPhase = ({ story, + tenant, }): IntermediatePhaseResult | void => { - // Check to see if the story has closed commenting... if (story.closedAt && story.closedAt.valueOf() <= Date.now()) { // TODO: (wyattjoh) return better error. throw new Error("story is currently closed for commenting"); } + + if ( + story.closedAt !== false && + tenant.autoCloseStream && + tenant.closedTimeout && + story.created_at.valueOf() + tenant.closedTimeout <= Date.now() + ) { + // TODO: (wyattjoh) return better error. + throw new Error("story is currently closed for commenting"); + } }; diff --git a/src/core/server/services/queue/tasks/scraper/index.ts b/src/core/server/services/queue/tasks/scraper/index.ts index 2a12bb86a..48d99a9f8 100644 --- a/src/core/server/services/queue/tasks/scraper/index.ts +++ b/src/core/server/services/queue/tasks/scraper/index.ts @@ -1,4 +1,5 @@ import Queue, { Job } from "bull"; +import Logger from "bunyan"; import cheerio from "cheerio"; import authorScraper from "metascraper-author"; import dateScraper from "metascraper-date"; @@ -7,6 +8,7 @@ import imageScraper from "metascraper-image"; import titleScraper from "metascraper-title"; import { Db } from "mongodb"; +import { GQLStoryMetadata } from "talk-server/graph/tenant/schema/__generated__/types"; import logger from "talk-server/logger"; import { updateStory } from "talk-server/models/story"; import Task from "talk-server/services/queue/Task"; @@ -25,10 +27,9 @@ export interface ScraperData { tenantID: string; } -const createJobProcessor = ( - options: ScrapeProcessorOptions, - scraper: Scraper -) => async (job: Job) => { +const createJobProcessor = (options: ScrapeProcessorOptions) => async ( + job: Job +) => { // Pull out the job data. const { storyID: id, storyURL: url, tenantID } = job.data; @@ -44,8 +45,8 @@ const createJobProcessor = ( ); // Get the metadata from the scraped html. - const meta = await scraper.scrape(url); - if (!meta) { + const metadata = await scraper.scrape(url); + if (!metadata) { logger.error( { job_id: job.id, @@ -61,14 +62,8 @@ const createJobProcessor = ( // Update the Story with the scraped details. const story = await updateStory(options.mongo, tenantID, id, { - title: meta.title || undefined, - description: meta.description || undefined, - image: meta.image ? meta.image : undefined, - author: meta.author || undefined, - publication_date: meta.date ? new Date(meta.date) : undefined, - modified_date: meta.modified ? new Date(meta.modified) : undefined, - section: meta.section || undefined, - scraped: new Date(), + metadata, + scrapedAt: new Date(), }); if (!story) { logger.error( @@ -105,25 +100,40 @@ export type Rule = Record< class Scraper { private rules: Rule[]; + private log: Logger; constructor(rules: Rule[]) { this.rules = rules; + this.log = logger.child({ taskName: "scraper" }); } - public async scrape(url: string) { + public async scrape(url: string): Promise { // Grab the page HTML. - // TODO: investigate adding scraping proxy support. + const log = this.log.child({ storyURL: url }); + + const start = Date.now(); + log.debug("starting scrape of Story"); + + // TODO: investigate adding scraping proxy support based on the Tenant. const res = await fetch(url, {}); if (res.status !== 200) { - return; + log.warn( + { statusCode: res.status }, + "scrape failed with non-200 status code" + ); + return null; } const html = await res.text(); + log.debug({ timeElapsed: Date.now() - start }, "scrape complete"); + // Load the DOM. const htmlDom = cheerio.load(html); + log.debug("parsed html"); + // Gather the results by evaluating each of the rules. const metadata: Record = {}; @@ -146,16 +156,26 @@ class Scraper { } } - return metadata; + log.debug("extracted metadata"); + + return { + title: metadata.title || undefined, + description: metadata.description || undefined, + image: metadata.image ? metadata.image : undefined, + author: metadata.author || undefined, + publishedAt: metadata.date ? new Date(metadata.date) : undefined, + modifiedAt: metadata.modified ? new Date(metadata.modified) : undefined, + section: metadata.section || undefined, + }; } } -export function createScraperTask( - queue: Queue.QueueOptions, - options: ScrapeProcessorOptions -) { - // Create the scraper object. - const scraper = new Scraper([ +/** + * createScraper will create a scraper that will utilize the rules defined to + * scrape metadata from the target page. + */ +function createScraper() { + return new Scraper([ authorScraper(), dateScraper(), descriptionScraper(), @@ -164,10 +184,17 @@ export function createScraperTask( modifiedScraper(), sectionScraper(), ]); +} +export const scraper = createScraper(); + +export function createScraperTask( + queue: Queue.QueueOptions, + options: ScrapeProcessorOptions +) { return new Task({ jobName: JOB_NAME, - jobProcessor: createJobProcessor(options, scraper), + jobProcessor: createJobProcessor(options), queue, }); } diff --git a/src/core/server/services/queue/tasks/scraper/rules/modified.ts b/src/core/server/services/queue/tasks/scraper/rules/modified.ts index 8b7ce6232..732a7c68e 100644 --- a/src/core/server/services/queue/tasks/scraper/rules/modified.ts +++ b/src/core/server/services/queue/tasks/scraper/rules/modified.ts @@ -2,6 +2,7 @@ import { Rules } from "metascraper"; export const modifiedScraper = (): Rules => ({ modified: [ + // From: http://ogp.me/#type_article ({ htmlDom: $ }) => $('meta[property="article:modified"]').attr("content"), ], }); diff --git a/src/core/server/services/queue/tasks/scraper/rules/section.ts b/src/core/server/services/queue/tasks/scraper/rules/section.ts index 957560920..717d6b51a 100644 --- a/src/core/server/services/queue/tasks/scraper/rules/section.ts +++ b/src/core/server/services/queue/tasks/scraper/rules/section.ts @@ -2,6 +2,7 @@ import { Rules } from "metascraper"; export const sectionScraper = (): Rules => ({ section: [ + // From: http://ogp.me/#type_article ({ htmlDom: $ }) => $('meta[property="article:section"]').attr("content"), ], }); diff --git a/src/core/server/services/stories/index.spec.ts b/src/core/server/services/stories/index.spec.ts new file mode 100644 index 000000000..2c045d7e8 --- /dev/null +++ b/src/core/server/services/stories/index.spec.ts @@ -0,0 +1,61 @@ +import { isURLPermitted } from "talk-server/services/stories"; + +it("denies when the tenant has no specified domains", () => { + const tenant = { domains: [] }; + + expect(isURLPermitted(tenant, "")).toEqual(false); +}); + +it("denies when tenant has a domain but not a valid url", () => { + const tenant = { domains: ["https://coralproject.net"] }; + + expect(isURLPermitted(tenant, "")).toEqual(false); +}); + +it("denies when there are multiple tenants domains and not a valid url", () => { + const tenant = { + domains: ["https://coralproject.net", "https://news.coralproject.net"], + }; + + expect(isURLPermitted(tenant, "")).toEqual(false); +}); + +it("denies when there are multiple tenants domains and a invalid url", () => { + const tenant = { + domains: ["https://coralproject.net", "https://news.coralproject.net"], + }; + + expect( + isURLPermitted(tenant, "https://blog.coralproject.net/a/page") + ).toEqual(false); +}); + +it("allows when there are multiple tenants domains and a valid url", () => { + const tenant = { + domains: ["https://coralproject.net", "https://news.coralproject.net"], + }; + + expect( + isURLPermitted(tenant, "https://news.coralproject.net/a/page") + ).toEqual(true); +}); + +it("allows when there are multiple prefix domains and a valid url", () => { + const tenant = { + domains: ["coralproject.net", "news.coralproject.net"], + }; + + expect(isURLPermitted(tenant, "http://news.coralproject.net/a/page")).toEqual( + true + ); +}); + +it("allows when there are some prefix domains and a valid url", () => { + const tenant = { + domains: ["http://coralproject.net", "news.coralproject.net"], + }; + + expect( + isURLPermitted(tenant, "https://news.coralproject.net/a/page") + ).toEqual(true); +}); diff --git a/src/core/server/services/stories/index.ts b/src/core/server/services/stories/index.ts index beec64153..79602b204 100644 --- a/src/core/server/services/stories/index.ts +++ b/src/core/server/services/stories/index.ts @@ -1,5 +1,12 @@ import { Db } from "mongodb"; +import { + doesRequireSchemePrefixing, + getOrigin, + isURLSecure, + prefixSchemeIfRequired, +} from "talk-server/app/url"; +import logger from "talk-server/logger"; import { findOrCreateStory, FindOrCreateStoryInput, @@ -16,6 +23,16 @@ export async function findOrCreate( input: FindOrCreateStory, scraper: Task ) { + // If the URL is provided, and the url is not on a allowed domain, then refuse + // to create the Asset. + if (input.url && !isURLPermitted(tenant, input.url)) { + logger.warn( + { storyURL: input.url, tenantDomains: tenant.domains }, + "provided story url was not in the list of permitted tenant domains" + ); + return null; + } + // TODO: check to see if the tenant has enabled lazy story creation. const story = await findOrCreateStory(db, tenant.id, input); @@ -23,7 +40,8 @@ export async function findOrCreate( return null; } - if (!story.scraped) { + if (!story.scrapedAt) { + // If the scraper has not scraped this story, we need to scrape it now! await scraper.add({ storyID: story.id, storyURL: story.url, @@ -33,3 +51,38 @@ export async function findOrCreate( return story; } + +/** + * isURLInsideAllowedDomains will validate if the given origin is allowed given + * the Tenant's domain configuration. + */ +export function isURLPermitted( + tenant: Pick, + targetURL: string +) { + // If there aren't any domains, then we reject it, because no url we have can + // satisfy those requirements. + if (tenant.domains.length === 0) { + return false; + } + + // If the scheme can not be inferred, then we can't determine the + // admissability of the url. + if (doesRequireSchemePrefixing(targetURL)) { + return false; + } + + // Determine the scheme of the targetOrigin. We know that the targetURL does + // not need prefixing, so it can only be true/false here. + const originSecure = isURLSecure(targetURL) as boolean; + + // Extract the origin from the URL. + const targetOrigin = getOrigin(targetURL); + + // Loop over all the Tenant domains provided. Prefix the domain of each if it + // is required with the target url scheme. Return if at least one match is + // found within the Tenant domains. + return tenant.domains + .map(domain => getOrigin(prefixSchemeIfRequired(originSecure, domain))) + .some(origin => origin === targetOrigin); +}