[CORL 133] API Review (#2197)

* refactor: removed unused subscription code

* refactor: removed management api's

* refactor: cleanup of connections

* refactor: refactored comments edge

* refactor: simplified connection resolving

* feat: added story connection edge

* fix: added story index

* feat: added user pagination and user edge

* fix: added filter to comment query

* fix: removed unused resolvers

* fix: creating a comment reply should require auth

* refactor: cleanup of graph files

* feat: removed display name, made username non-unique

* fix: fixed tests

* fix: fixed tests

* fix: added more api docs

* fix: fixed bug with installer

* refactor: fixes and updates

* fix: added linting for graphql, fixed schema

* feat: added docker build tests

* fix: upped output timeout

* fix: fixed stacktraces in production builds

* fix: removed `git add`

- `git add` was causing issues with
    partial staged changs on files

* feat: improved error messaging for auth

* refactor: cleaned up queue names

* fix: merge error

src/core/server/app/middleware/context/tenant.ts

@@ -1,63 +0,0 @@
-import { RequestHandler } from "express-jwt";
-import { Db } from "mongodb";
-
-import { Config } from "talk-server/config";
-import TenantContext from "talk-server/graph/tenant/context";
-import { TaskQueue } from "talk-server/queue";
-import { I18n } from "talk-server/services/i18n";
-import { JWTSigningConfig } from "talk-server/services/jwt";
-import { AugmentedRedis } from "talk-server/services/redis";
-import { Request } from "talk-server/types/express";
-
-export interface TenantContextMiddlewareOptions {
-  mongo: Db;
-  redis: AugmentedRedis;
-  queue: TaskQueue;
-  config: Config;
-  signingConfig: JWTSigningConfig;
-  i18n: I18n;
-}
-
-export const tenantContext = ({
-  mongo,
-  redis,
-  queue,
-  config,
-  signingConfig,
-  i18n,
-}: TenantContextMiddlewareOptions): RequestHandler => (
-  req: Request,
-  res,
-  next
-) => {
-  if (!req.talk) {
-    return next(new Error("talk was not set"));
-  }
-
-  const { tenant, cache } = req.talk;
-
-  if (!cache) {
-    return next(new Error("cache was not set"));
-  }
-
-  if (!tenant) {
-    return next(new Error("tenant was not set"));
-  }
-
-  req.talk.context = {
-    tenant: new TenantContext({
-      req,
-      config,
-      mongo,
-      redis,
-      tenant,
-      user: req.user,
-      tenantCache: cache.tenant,
-      queue,
-      signingConfig,
-      i18n,
-    }),
-  };
-
-  next();
-};

src/core/server/app/middleware/graphql/index.ts

@@ -0,0 +1,72 @@
+import { GraphQLOptions } from "apollo-server-express";
+import { Handler } from "express";
+import { FieldDefinitionNode, GraphQLError, ValidationContext } from "graphql";
+
+// TODO: when https://github.com/apollographql/apollo-server/pull/1907 is merged, update this import path
+import {
+  ExpressGraphQLOptionsFunction,
+  graphqlExpress,
+} from "apollo-server-express/dist/expressApollo";
+
+import { Omit } from "talk-common/types";
+import { Config } from "talk-server/config";
+import {
+  ErrorWrappingExtension,
+  LoggerExtension,
+} from "talk-server/graph/common/extensions";
+
+export * from "./batch";
+
+// Sourced from: https://github.com/apollographql/apollo-server/blob/958846887598491fadea57b3f9373d129300f250/packages/apollo-server-core/src/ApolloServer.ts#L46-L57
+const NoIntrospection = (context: ValidationContext) => ({
+  Field(node: FieldDefinitionNode) {
+    if (node.name.value === "__schema" || node.name.value === "__type") {
+      context.reportError(
+        new GraphQLError(
+          "GraphQL introspection is not allowed in production, but the query contained __schema or __type.",
+          [node]
+        )
+      );
+    }
+  },
+});
+
+/**
+ * graphqlMiddleware wraps the GraphQL middleware server with some custom
+ * extension management.
+ *
+ * @param config application configuration
+ * @param requestOptions options to pass to the graphql server
+ */
+export const graphqlMiddleware = (
+  config: Config,
+  requestOptions: ExpressGraphQLOptionsFunction
+): Handler => {
+  // Create a new baseOptions that will be merged into the new options.
+  const baseOptions: Omit<GraphQLOptions, "schema"> = {
+    // Disable the debug mode, as we already add in our logging function.
+    debug: false,
+    // Include extensions.
+    extensions: [
+      () => new ErrorWrappingExtension(),
+      () => new LoggerExtension(),
+    ],
+  };
+
+  if (config.get("env") === "production" && !config.get("enable_graphiql")) {
+    // Disable introspection in production.
+    baseOptions.validationRules = [NoIntrospection];
+  }
+
+  // Generate the actual middleware.
+  return graphqlExpress(async (req, res) => {
+    // Resolve the options for the GraphQL middleware.
+    const options = await requestOptions(req, res);
+
+    // Provide the options.
+    return {
+      ...options,
+      ...baseOptions,
+    };
+  });
+};

src/core/server/app/middleware/installed.ts

@@ -1,20 +1,35 @@
-import { RequestHandler } from "express";
-
 import { isInstalled } from "talk-server/services/tenant";
-import TenantCache from "talk-server/services/tenant/cache";
+import { RequestHandler } from "talk-server/types/express";
 
 export interface InstalledMiddlewareOptions {
-  tenantCache: TenantCache;
   redirectURL?: string;
   redirectIfInstalled?: boolean;
 }
 
+const DefaultInstalledMiddlewareOptions: Required<
+  InstalledMiddlewareOptions
+> = {
+  redirectIfInstalled: false,
+  redirectURL: "/install",
+};
+
 export const installedMiddleware = ({
-  tenantCache,
-  redirectIfInstalled = false,
-  redirectURL = "/install",
-}: InstalledMiddlewareOptions): RequestHandler => async (req, res, next) => {
-  const installed = await isInstalled(tenantCache);
+  redirectIfInstalled = DefaultInstalledMiddlewareOptions.redirectIfInstalled,
+  redirectURL = DefaultInstalledMiddlewareOptions.redirectURL,
+}: InstalledMiddlewareOptions = DefaultInstalledMiddlewareOptions): RequestHandler => async (
+  req,
+  res,
+  next
+) => {
+  if (!req.talk) {
+    return next(new Error("talk was not set"));
+  }
+
+  if (!req.talk.cache) {
+    return next(new Error("cache was not set"));
+  }
+
+  const installed = await isInstalled(req.talk.cache.tenant);
 
   // If Talk is installed, and redirectIfInstall is true, then it will redirect.
   // If Talk is not installed, and redirectIfInstall is false, then it will also

src/core/server/app/middleware/passport/index.ts

@@ -2,17 +2,16 @@ import { NextFunction, RequestHandler, Response } from "express";
 import { Redis } from "ioredis";
 import Joi from "joi";
 import jwt from "jsonwebtoken";
-import { Db } from "mongodb";
 import passport, { Authenticator } from "passport";
 import now from "performance-now";
 
+import { AppOptions } from "talk-server/app";
 import FacebookStrategy from "talk-server/app/middleware/passport/strategies/facebook";
 import GoogleStrategy from "talk-server/app/middleware/passport/strategies/google";
 import { JWTStrategy } from "talk-server/app/middleware/passport/strategies/jwt";
 import { createLocalStrategy } from "talk-server/app/middleware/passport/strategies/local";
 import OIDCStrategy from "talk-server/app/middleware/passport/strategies/oidc";
 import { validate } from "talk-server/app/request/body";
-import { Config } from "talk-server/config";
 import logger from "talk-server/logger";
 import { User } from "talk-server/models/user";
 import {
@@ -22,7 +21,6 @@ import {
   SigningTokenOptions,
   signTokenString,
 } from "talk-server/services/jwt";
-import TenantCache from "talk-server/services/tenant/cache";
 import { Request } from "talk-server/types/express";
 
 export type VerifyCallback = (
@@ -31,13 +29,10 @@ export type VerifyCallback = (
   info?: { message: string }
 ) => void;
 
-export interface PassportOptions {
-  config: Config;
-  mongo: Db;
-  redis: Redis;
-  signingConfig: JWTSigningConfig;
-  tenantCache: TenantCache;
-}
+export type PassportOptions = Pick<
+  AppOptions,
+  "mongo" | "redis" | "config" | "tenantCache" | "signingConfig"
+>;
 
 export function createPassport(
   options: PassportOptions
@@ -142,8 +137,7 @@ export async function handleOAuth2Callback(
   user: User | null,
   signingConfig: JWTSigningConfig,
   req: Request,
-  res: Response,
-  next: NextFunction
+  res: Response
 ) {
   const path = "/embed/auth/callback";
   if (!user) {
@@ -195,7 +189,7 @@ export const wrapOAuth2Authn = (
     name,
     { ...options, session: false },
     (err: Error | null, user: User | null) => {
-      handleOAuth2Callback(err, user, signingConfig, req, res, next);
+      handleOAuth2Callback(err, user, signingConfig, req, res);
     }
   )(req, res, next);
 

src/core/server/app/middleware/passport/strategies/facebook.ts

@@ -1,9 +1,9 @@
-import { Db } from "mongodb";
 import { Profile, Strategy } from "passport-facebook";
 
-import OAuth2Strategy from "talk-server/app/middleware/passport/strategies/oauth2";
+import OAuth2Strategy, {
+  OAuth2StrategyOptions,
+} from "talk-server/app/middleware/passport/strategies/oauth2";
 import { constructTenantURL } from "talk-server/app/url";
-import { Config } from "talk-server/config";
 import {
   GQLAuthIntegrations,
   GQLFacebookAuthIntegration,
@@ -14,14 +14,9 @@ import {
   FacebookProfile,
   retrieveUserWithProfile,
 } from "talk-server/models/user";
-import TenantCache from "talk-server/services/tenant/cache";
 import { upsert } from "talk-server/services/users";
 
-export interface FacebookStrategyOptions {
-  config: Config;
-  mongo: Db;
-  tenantCache: TenantCache;
-}
+export type FacebookStrategyOptions = OAuth2StrategyOptions;
 
 export default class FacebookStrategy extends OAuth2Strategy<
   GQLFacebookAuthIntegration,
@@ -77,7 +72,7 @@ export default class FacebookStrategy extends OAuth2Strategy<
       }
 
       user = await upsert(this.mongo, tenant, {
-        displayName,
+        username: displayName,
         role: GQLUSER_ROLE.COMMENTER,
         email,
         emailVerified,
@@ -102,7 +97,7 @@ export default class FacebookStrategy extends OAuth2Strategy<
         callbackURL: constructTenantURL(
           this.config,
           tenant,
-          "/api/tenant/auth/facebook/callback"
+          "/api/auth/facebook/callback"
         ),
         profileFields: ["id", "displayName", "photos", "email"],
         enableProof: true,

src/core/server/app/middleware/passport/strategies/google.ts

@@ -1,9 +1,9 @@
-import { Db } from "mongodb";
 import { Profile, Strategy } from "passport-google-oauth2";
 
-import OAuth2Strategy from "talk-server/app/middleware/passport/strategies/oauth2";
+import OAuth2Strategy, {
+  OAuth2StrategyOptions,
+} from "talk-server/app/middleware/passport/strategies/oauth2";
 import { constructTenantURL } from "talk-server/app/url";
-import { Config } from "talk-server/config";
 import {
   GQLAuthIntegrations,
   GQLGoogleAuthIntegration,
@@ -14,20 +14,9 @@ import {
   GoogleProfile,
   retrieveUserWithProfile,
 } from "talk-server/models/user";
-import TenantCache from "talk-server/services/tenant/cache";
 import { upsert } from "talk-server/services/users";
 
-export interface GoogleStrategyOptions {
-  config: Config;
-  mongo: Db;
-  tenantCache: TenantCache;
-}
-
-export interface GoogleStrategyOptions {
-  config: Config;
-  mongo: Db;
-  tenantCache: TenantCache;
-}
+export type GoogleStrategyOptions = OAuth2StrategyOptions;
 
 export default class GoogleStrategy extends OAuth2Strategy<
   GQLGoogleAuthIntegration,
@@ -82,7 +71,7 @@ export default class GoogleStrategy extends OAuth2Strategy<
       }
 
       user = await upsert(this.mongo, tenant, {
-        displayName,
+        username: displayName,
         role: GQLUSER_ROLE.COMMENTER,
         email,
         emailVerified,
@@ -107,7 +96,7 @@ export default class GoogleStrategy extends OAuth2Strategy<
         callbackURL: constructTenantURL(
           this.config,
           tenant,
-          "/api/tenant/auth/google/callback"
+          "/api/auth/google/callback"
         ),
         passReqToCallback: true,
       },

src/core/server/app/middleware/passport/strategies/jwt.ts

@@ -1,8 +1,7 @@
-import { Redis } from "ioredis";
 import jwt from "jsonwebtoken";
-import { Db } from "mongodb";
 import { Strategy } from "passport-strategy";
 
+import { AppOptions } from "talk-server/app";
 import {
   JWTToken,
   JWTVerifier,
@@ -14,17 +13,13 @@ import {
 import { TenantNotFoundError, TokenInvalidError } from "talk-server/errors";
 import { Tenant } from "talk-server/models/tenant";
 import { User } from "talk-server/models/user";
-import {
-  extractJWTFromRequest,
-  JWTSigningConfig,
-} from "talk-server/services/jwt";
+import { extractJWTFromRequest } from "talk-server/services/jwt";
 import { Request } from "talk-server/types/express";
 
-export interface JWTStrategyOptions {
-  signingConfig: JWTSigningConfig;
-  mongo: Db;
-  redis: Redis;
-}
+export type JWTStrategyOptions = Pick<
+  AppOptions,
+  "signingConfig" | "mongo" | "redis"
+>;
 
 /**
  * Token is the various forms of the Token that can be verified.

src/core/server/app/middleware/passport/strategies/oidc/index.ts

@@ -37,6 +37,7 @@ export interface OIDCIDToken {
   picture?: string;
   name?: string;
   nickname?: string;
+  preferred_username?: string;
 }
 
 export interface StrategyItem {
@@ -121,11 +122,18 @@ export const OIDCIDTokenSchema = Joi.object()
     picture: Joi.string().default(undefined),
     name: Joi.string().default(undefined),
     nickname: Joi.string().default(undefined),
+    preferred_username: Joi.string().default(undefined),
   })
-  .optionalKeys(["picture", "email_verified", "name", "nickname"]);
+  .optionalKeys([
+    "picture",
+    "email_verified",
+    "name",
+    "nickname",
+    "preferred_username",
+  ]);
 
 export async function findOrCreateOIDCUser(
-  db: Db,
+  mongo: Db,
   tenant: Tenant,
   integration: GQLOIDCAuthIntegration,
   token: OIDCIDToken
@@ -140,6 +148,7 @@ export async function findOrCreateOIDCUser(
     picture,
     name,
     nickname,
+    preferred_username,
   }: OIDCIDToken = validate(OIDCIDTokenSchema, token);
 
   // Construct the profile that will be used to query for the user.
@@ -151,7 +160,7 @@ export async function findOrCreateOIDCUser(
   };
 
   // Try to lookup user given their id provided in the `sub` claim.
-  let user = await retrieveUserWithProfile(db, tenant.id, {
+  let user = await retrieveUserWithProfile(mongo, tenant.id, {
     // NOTE: (wyattjoh) as the current requirements do not allow multiple OIDC integrations, we are only getting the profile based on the OIDC provider.
     type: "oidc",
     id: sub,
@@ -164,11 +173,12 @@ export async function findOrCreateOIDCUser(
 
     // FIXME: implement rules.
 
-    const displayName = nickname || name || undefined;
+    // Try to extract the username from the following chain:
+    const username = preferred_username || nickname || name;
 
     // Create the new user, as one didn't exist before!
-    user = await upsert(db, tenant, {
-      displayName,
+    user = await upsert(mongo, tenant, {
+      username,
       role: GQLUSER_ROLE.COMMENTER,
       email,
       emailVerified: email_verified,
@@ -310,7 +320,7 @@ export default class OIDCStrategy extends Strategy {
     const { clientID, clientSecret, authorizationURL, tokenURL } = integration;
 
     // Construct the callbackURL from the request.
-    const callbackURL = reconstructURL(req, `/api/tenant/auth/oidc/callback`);
+    const callbackURL = reconstructURL(req, `/api/auth/oidc/callback`);
 
     // Create a new OAuth2Strategy, where we pass the verify callback bound to
     // this OIDCStrategy instance.

src/core/server/app/middleware/passport/strategies/verifiers/sso.ts

@@ -18,9 +18,8 @@ export interface SSOStrategyOptions {
 export interface SSOUserProfile {
   id: string;
   email: string;
-  username?: string;
+  username: string;
   avatar?: string;
-  displayName?: string;
 }
 
 export interface SSOToken {
@@ -38,7 +37,7 @@ export const SSOUserProfileSchema = Joi.object()
   .optionalKeys(["avatar", "displayName"]);
 
 export async function findOrCreateSSOUser(
-  db: Db,
+  mongo: Db,
   tenant: Tenant,
   integration: GQLSSOAuthIntegration,
   token: SSOToken
@@ -49,7 +48,7 @@ export async function findOrCreateSSOUser(
   }
 
   // Unpack/validate the token content.
-  const { id, email, username, displayName, avatar }: SSOUserProfile = validate(
+  const { id, email, username, avatar }: SSOUserProfile = validate(
     SSOUserProfileSchema,
     token.user
   );
@@ -60,7 +59,7 @@ export async function findOrCreateSSOUser(
   };
 
   // Try to lookup user given their id provided in the `sub` claim.
-  let user = await retrieveUserWithProfile(db, tenant.id, profile);
+  let user = await retrieveUserWithProfile(mongo, tenant.id, profile);
   if (!user) {
     if (!integration.allowRegistration) {
       // Registration is disabled, so we can't create the user user here.
@@ -70,11 +69,8 @@ export async function findOrCreateSSOUser(
     // FIXME: (wyattjoh) implement rules! Not all users should be able to create an account via this method.
 
     // Create the new user, as one didn't exist before!
-    user = await upsert(db, tenant, {
+    user = await upsert(mongo, tenant, {
       username,
-      // When the displayName is disabled on the tenant, the displayName will
-      // never be set (or even stored in the database).
-      displayName,
       role: GQLUSER_ROLE.COMMENTER,
       email,
       avatar,

src/core/server/app/middleware/tenant.ts

@@ -13,12 +13,17 @@ export const tenantMiddleware = ({
 }: MiddlewareOptions): RequestHandler => async (req, res, next) => {
   try {
     // Set Talk on the request.
-    req.talk = {
-      cache: {
+    if (!req.talk) {
+      req.talk = {};
+    }
+
+    // Set the Talk Tenant Cache on the request.
+    if (!req.talk.cache) {
+      req.talk.cache = {
         // Attach the tenant cache to the request.
         tenant: cache,
-      },
-    };
+      };
+    }
 
     // Attach the tenant to the request.
     const tenant = await cache.retrieveByDomain(req.hostname);