review: changes for review

config/watcher.ts

@@ -9,8 +9,8 @@ const config: Config = {
   rootDir: path.resolve(__dirname, "../src"),
   watchers: {
     compileGraphQLTypes: {
-      paths: ["core/server/graph/**/*.graphql"],
-      executor: new CommandExecutor("npm run compile:graphql", {
+      paths: ["core/server/**/*.graphql"],
+      executor: new CommandExecutor("npm run compile:schema", {
         runOnInit: true,
       }),
     },

package.json

@@ -7,7 +7,7 @@
     "build:server": "tsc -p ./src/tsconfig.json",
     "build": "npm-run-all compile --parallel build:*",
     "compile:css-types": "tcm src/core/client/",
-    "compile:graphql": "node ./scripts/types.js",
+    "compile:schema": "node ./scripts/generateSchemaTypes.js",
     "compile:relay-stream": "relay-compiler --src ./src/core/client/stream --schema $(ts-node ./scripts/schemaPath.ts tenant) --language typescript --artifactDirectory ./src/core/client/stream/__generated__ --no-watchman",
     "compile": "npm-run-all --parallel compile:*",
     "docz:watch": "docz dev",
@@ -175,4 +175,4 @@
     "webpack-hot-client": "^4.0.3",
     "webpack-manifest-plugin": "^2.0.3"
   }
-}
+}

scripts/generateSchemaTypes.js

@@ -17,15 +17,9 @@ function lintAndWrite(files) {
 function getFileName(name) {
   return path.join(
     __dirname,
-    "..",
-    "src",
-    "core",
-    "server",
-    "graph",
+    "../src/core/server/graph",
     name,
-    "schema",
-    "__generated__",
-    "types.ts"
+    "schema/__generated__/types.ts"
   );
 }
 

src/core/client/tsconfig.json

@@ -12,8 +12,7 @@
       "talk-stream/*": ["./stream/*"],
       "talk-framework/*": ["./framework/*"],
       "talk-ui/*": ["./ui/*"],
-      "talk-common/*": ["../common/*"],
-      "talk-locales/*": ["../../locales/*"]
+      "talk-common/*": ["../common/*"]
     }
   },
   "include": ["./**/*", "../../types/**/*.d.ts"],

src/core/server/app/handlers/auth/local.ts

@@ -22,8 +22,7 @@ const SignupBodySchema = Joi.object().keys({
   email: Joi.string().trim(),
 });
 
-// Extends the default signup body schema with the displayName to allow it to be
-// sent.
+// Extends the default signup body schema to allow the displayName to be set.
 const SignupDisplayNameBodySchema = SignupBodySchema.keys({
   displayName: Joi.string().trim(),
 });

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

@@ -51,6 +51,50 @@ export function isOIDCToken(token: OIDCIDToken | object): token is OIDCIDToken {
   return false;
 }
 
+/**
+ * keyFunc will provide the secret based on the given jwkw client.
+ *
+ * @param client the jwks client for the specific request being made
+ */
+const signingKeyFactory = (client: jwks.JwksClient): jwt.KeyFunction => (
+  { kid },
+  callback
+) => {
+  if (!kid) {
+    // TODO: return better error.
+    return callback(new Error("no kid in id_token"));
+  }
+
+  // Get the signing key from the jwks provider.
+  client.getSigningKey(kid, (err, key) => {
+    if (err) {
+      // TODO: wrap error?
+      return callback(err);
+    }
+
+    // Grab the signingKey out of the provided key.
+    const signingKey = key.publicKey || key.rsaPublicKey;
+
+    callback(null, signingKey);
+  });
+};
+
+function getEnabledIntegration(tenant: Tenant) {
+  const integration = tenant.auth.integrations.oidc;
+  if (!integration) {
+    // TODO: return a better error.
+    throw new Error("integration not found");
+  }
+
+  // Handle when the integration is enabled/disabled.
+  if (!integration.enabled) {
+    // TODO: return a better error.
+    throw new Error("integration not enabled");
+  }
+
+  return integration;
+}
+
 export async function findOrCreateOIDCUser(
   db: Db,
   tenant: Tenant,
@@ -104,19 +148,6 @@ export default class OIDCStrategy extends Strategy {
     this.db = db;
   }
 
-  private async verify(
-    tenant: Tenant,
-    token: OIDCIDToken,
-    done: VerifyCallback
-  ) {
-    try {
-      const user = await findOrCreateOIDCUser(this.db, tenant, token);
-      return done(null, user);
-    } catch (err) {
-      return done(err);
-    }
-  }
-
   private lookupJWKSClient(
     req: Request,
     tenantID: string,
@@ -151,7 +182,7 @@ export default class OIDCStrategy extends Strategy {
     return entry.jwksClient;
   }
 
-  private verifyCallback = (
+  private userAuthenticatedCallback = (
     req: Request,
     accessToken: string, // ignore the access token, we don't use it.
     refreshToken: string, // ignore the refresh token, we don't use it.
@@ -190,50 +221,30 @@ export default class OIDCStrategy extends Strategy {
     // Verify that the id_token is valid or not.
     jwt.verify(
       id_token,
-      this.keyFunc(client),
+      signingKeyFactory(client),
       {
         issuer: integration.issuer,
       },
-      (err, decoded) => {
+      async (err, decoded) => {
         if (err) {
           // TODO: wrap error?
           return done(err);
         }
 
-        // Delegate the verify method off to the passed in verify method.
-        this.verify(tenant, decoded as OIDCIDToken, done);
+        try {
+          const user = await findOrCreateOIDCUser(
+            this.db,
+            tenant,
+            decoded as OIDCIDToken
+          );
+          return done(null, user);
+        } catch (err) {
+          return done(err);
+        }
       }
     );
   };
 
-  /**
-   * keyFunc will provide the secret based on the given jwkw client.
-   *
-   * @param client the jwks client for the specific request being made
-   */
-  private keyFunc = (client: jwks.JwksClient): jwt.KeyFunction => (
-    { kid },
-    callback
-  ) => {
-    if (!kid) {
-      // TODO: return better error.
-      return callback(new Error("no kid in id_token"));
-    }
-
-    // Get the signing key from the jwks provider.
-    client.getSigningKey(kid, (err, key) => {
-      if (err) {
-        // TODO: wrap error?
-        return callback(err);
-      }
-
-      // Grab the signingKey out of the provided key.
-      const signingKey = key.publicKey || key.rsaPublicKey;
-
-      callback(null, signingKey);
-    });
-  };
-
   private createStrategy(
     req: Request,
     integration: OIDCAuthIntegration
@@ -254,7 +265,7 @@ export default class OIDCStrategy extends Strategy {
         tokenURL,
         callbackURL,
       },
-      this.verifyCallback
+      this.userAuthenticatedCallback
     );
   }
 
@@ -315,22 +326,6 @@ export default class OIDCStrategy extends Strategy {
   }
 }
 
-function getEnabledIntegration(tenant: Tenant) {
-  const integration = tenant.auth.integrations.oidc;
-  if (!integration) {
-    // TODO: return a better error.
-    throw new Error("integration not found");
-  }
-
-  // Handle when the integration is enabled/disabled.
-  if (!integration.enabled) {
-    // TODO: return a better error.
-    throw new Error("integration not enabled");
-  }
-
-  return integration;
-}
-
 export function createOIDCStrategy({ db }: OIDCStrategyOptions) {
   return new OIDCStrategy({ db });
 }