fix: consent and logout integration logic

ory · Sep 18, 2023 · b7709c3 · b7709c3
1 parent 768fafd
commit b7709c3
Show file tree

Hide file tree

Showing 10 changed files with 209 additions and 109 deletions.
diff --git a/README.md b/README.md
@@ -52,6 +52,8 @@ Ory OAuth2 requires more setup to get CSRF cookies on the `/consent` endpoint.
   setup. This sets the CSRF cookies to `secure: false`, required for running
   locally. When using this setting, you must also set `CSRF_COOKIE_NAME` to a
   name without the `__Host-` prefix.
+- `TRUSTED_CLIENT_IDS` (optional): A list of trusted client ids. They can be set
+  to skip the consent screen.
 
 Getting TLS working:
 

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -34,7 +34,7 @@
     "axios": "1.2.6",
     "body-parser": "1.20.2",
     "cookie-parser": "1.4.6",
-    "csrf-csrf": "3.0.0",
+    "csrf-csrf": "3.0.1",
     "express": "4.18.2",
     "express-handlebars": "6.0.7",
     "express-jwt": "8.4.1",

diff --git a/src/index.ts b/src/index.ts
@@ -6,11 +6,10 @@ import {
   detectLanguage,
   handlebarsHelpers,
 } from "./pkg"
-import { middleware as middlewareLogger } from "./pkg/logger"
+import { logger, middleware as middlewareLogger } from "./pkg/logger"
 import {
   register404Route,
   register500Route,
-  registerConsentPostRoute,
   registerConsentRoute,
   registerErrorRoute,
   registerHealthRoute,
@@ -22,8 +21,12 @@ import {
   registerStaticRoutes,
   registerVerificationRoute,
   registerWelcomeRoute,
+  registerLogoutRoute,
 } from "./routes"
+import { csrfErrorHandler } from "./routes/csrfError"
+import bodyParser from "body-parser"
 import cookieParser from "cookie-parser"
+import { DoubleCsrfCookieOptions, doubleCsrf } from "csrf-csrf"
 import express, { Request, Response } from "express"
 import { engine } from "express-handlebars"
 import * as fs from "fs"
@@ -34,10 +37,40 @@ const baseUrl = process.env.BASE_PATH || "/"
 const app = express()
 const router = express.Router()
 
+const cookieOptions: DoubleCsrfCookieOptions = {
+  sameSite: "lax",
+  signed: true,
+  // set secure cookies by default (recommended in production)
+  // can be disabled through DANGEROUSLY_DISABLE_SECURE_COOKIES=true env var
+  secure: true,
+  ...(process.env.DANGEROUSLY_DISABLE_SECURE_CSRF_COOKIES && {
+    secure: false,
+  }),
+}
+
+const cookieName = process.env.CSRF_COOKIE_NAME || "__Host-ax-x-csrf-token"
+const cookieSecret = process.env.CSRF_COOKIE_SECRET
+
+// Sets up csrf protection
+const {
+  invalidCsrfTokenError,
+  doubleCsrfProtection, // This is the default CSRF protection middleware.
+} = doubleCsrf({
+  getSecret: () => cookieSecret || "", // A function that optionally takes the request and returns a secret
+  cookieName: cookieName, // The name of the cookie to be used, recommend using Host prefix.
+  cookieOptions,
+  ignoredMethods: ["GET", "HEAD", "OPTIONS", "PUT", "DELETE"], // A list of request methods that will not be protected.
+  getTokenFromRequest: (req: Request) => {
+    logger.debug("Getting CSRF token from request", { body: req.body })
+    return req.body._csrf
+  }, // A function that returns the token from the request
+})
+
 app.use(middlewareLogger)
 app.use(cookieParser(process.env.COOKIE_SECRET || ""))
 app.use(addFavicon(defaultConfig))
 app.use(detectLanguage)
+app.use(bodyParser.urlencoded({ extended: false }))
 app.set("view engine", "hbs")
 
 app.engine(
@@ -53,8 +86,6 @@ app.engine(
 
 registerHealthRoute(router)
 registerLoginRoute(router)
-registerConsentRoute(app)
-registerConsentPostRoute(app)
 registerRecoveryRoute(router)
 registerRegistrationRoute(router)
 registerSettingsRoute(router)
@@ -71,6 +102,12 @@ registerStaticRoutes(router)
 register404Route(router)
 register500Route(router)
 
+// all routes registered after this point are protected by CSRF
+app.use(doubleCsrfProtection)
+app.use(csrfErrorHandler(invalidCsrfTokenError))
+
+registerConsentRoute(app)
+registerLogoutRoute(app)
 app.use(baseUrl, router)
 
 const port = Number(process.env.PORT) || 3000

diff --git a/src/pkg/index.ts b/src/pkg/index.ts
@@ -38,6 +38,18 @@ export const defaultConfig: RouteOptionsCreator = () => {
       process.env.CSRF_COOKIE_NAME
         ? true
         : false,
+    shouldSkipConsent: (challenge) => {
+      let trustedClients: string[] = []
+      if (process.env.TRUSTED_CLIENT_IDS) {
+        trustedClients = String(process.env.TRUSTED_CLIENT_IDS).split(",")
+      }
+      return challenge.skip ||
+        challenge.client?.skip_consent ||
+        (challenge.client?.client_id &&
+          trustedClients.indexOf(challenge.client?.client_id) > -1)
+        ? true
+        : false
+    },
     ...sdk,
   }
 }

diff --git a/src/pkg/route.ts b/src/pkg/route.ts
@@ -1,6 +1,11 @@
 // Copyright © 2022 Ory Corp
 // SPDX-License-Identifier: Apache-2.0
-import { FrontendApi, IdentityApi, OAuth2Api } from "@ory/client"
+import {
+  FrontendApi,
+  IdentityApi,
+  OAuth2Api,
+  OAuth2ConsentRequest,
+} from "@ory/client"
 import { Theme } from "@ory/elements-markup"
 import { NextFunction, Request, Response, Router } from "express"
 
@@ -15,6 +20,10 @@ export interface RouteOptions {
   // This is used to determine if the consent route should be registered
   // We need to check if the required environment variables are set
   isOAuthConsentRouteEnabled: () => boolean
+  // Checks if the OAuth2 consent request should be skipped
+  // In some cases Hydra will let us skip the consent request
+  // Setting `TRUSTED_CLIENT_IDS` will skip the consent request for the given client ids
+  shouldSkipConsent: (challenge: OAuth2ConsentRequest) => boolean
 
   logoUrl?: string
   faviconUrl?: string

diff --git a/src/routes/consent.ts b/src/routes/consent.ts
@@ -1,63 +1,17 @@
 // Copyright © 2023 Ory Corp
 // SPDX-License-Identifier: Apache-2.0
-import { defaultConfig, logger, RouteCreator, RouteRegistrator } from "../pkg"
+import {
+  defaultConfig,
+  RouteCreator,
+  RouteRegistrator,
+  setSession,
+} from "../pkg"
 import { oidcConformityMaybeFakeSession } from "./stub/oidc-cert"
 import { AcceptOAuth2ConsentRequestSession } from "@ory/client"
 import { UserConsentCard } from "@ory/elements-markup"
 import bodyParser from "body-parser"
-import { doubleCsrf, DoubleCsrfCookieOptions } from "csrf-csrf"
 import { Request, Response, NextFunction } from "express"
 
-const cookieOptions: DoubleCsrfCookieOptions = {
-  sameSite: "lax",
-  signed: true,
-  // set secure cookies by default (recommended in production)
-  // can be disabled through DANGEROUSLY_DISABLE_SECURE_COOKIES=true env var
-  secure: true,
-  ...(process.env.DANGEROUSLY_DISABLE_SECURE_CSRF_COOKIES && {
-    secure: false,
-  }),
-}
-
-const cookieName = process.env.CSRF_COOKIE_NAME || "__Host-ax-x-csrf-token"
-const cookieSecret = process.env.CSRF_COOKIE_SECRET
-
-// Sets up csrf protection
-const {
-  invalidCsrfTokenError,
-  doubleCsrfProtection, // This is the default CSRF protection middleware.
-} = doubleCsrf({
-  getSecret: () => cookieSecret || "", // A function that optionally takes the request and returns a secret
-  cookieName: cookieName, // The name of the cookie to be used, recommend using Host prefix.
-  cookieOptions,
-  ignoredMethods: ["GET", "HEAD", "OPTIONS", "PUT", "DELETE"], // A list of request methods that will not be protected.
-  getTokenFromRequest: (req: Request) => {
-    logger.debug("Getting CSRF token from request", { body: req.body })
-    return req.body._csrf
-  }, // A function that returns the token from the request
-})
-
-// Error handling, validation error interception
-const csrfErrorHandler = (
-  error: unknown,
-  req: Request,
-  res: Response,
-  next: NextFunction,
-) => {
-  if (error == invalidCsrfTokenError) {
-    logger.debug("The CSRF token is invalid or could not be found.", {
-      req: req,
-    })
-    next(
-      new Error(
-        "A security violation was detected, please fill out the form again.",
-      ),
-    )
-  } else {
-    next()
-  }
-}
-
 const extractSession = (
   req: Request,
   grantScope: string[],
@@ -115,7 +69,12 @@ export const createConsentRoute: RouteCreator =
   (createHelpers) => (req: Request, res: Response, next: NextFunction) => {
     res.locals.projectName = "An application requests access to your data!"
 
-    const { oauth2, isOAuthConsentRouteEnabled } = createHelpers(req, res)
+    const {
+      oauth2,
+      isOAuthConsentRouteEnabled,
+      shouldSkipConsent: canSkipConsent,
+      logoUrl,
+    } = createHelpers(req, res)
 
     if (!isOAuthConsentRouteEnabled()) {
       res.redirect("404")
@@ -133,11 +92,6 @@ export const createConsentRoute: RouteCreator =
       return
     }
 
-    let trustedClients: string[] = []
-    if (process.env.TRUSTED_CLIENT_IDS) {
-      trustedClients = String(process.env.TRUSTED_CLIENT_IDS).split(",")
-    }
-
     // This section processes consent requests and either shows the consent UI or
     // accepts the consent request right away if the user has given consent to this
     // app before
@@ -146,15 +100,7 @@ export const createConsentRoute: RouteCreator =
       // This will be called if the HTTP request was successful
       .then(async ({ data: body }) => {
         // If a user has granted this application the requested scope, hydra will tell us to not show the UI.
-        if (
-          body.skip ||
-          body.client?.skip_consent ||
-          (body.client?.client_id &&
-            trustedClients.indexOf(body.client?.client_id) > -1)
-        ) {
-          // You can apply logic here, for example grant another scope, or do whatever...
-          // ...
-
+        if (canSkipConsent(body)) {
           let grantScope = req.body.grant_scope
           if (!Array.isArray(grantScope)) {
             grantScope = [grantScope]
@@ -199,11 +145,14 @@ export const createConsentRoute: RouteCreator =
           card: UserConsentCard({
             consent: body,
             csrfToken: req.csrfToken(true),
-            cardImage: body.client?.logo_uri || "/ory-logo.svg",
-            client_name: body.client?.client_name || "unknown client",
-            requested_scope: body.requested_scope,
+            cardImage: body.client?.logo_uri || logoUrl,
+            client_name:
+              body.client?.client_name ||
+              body.client?.client_id ||
+              "Unknown Client",
+            requested_scope: body.requested_scope || [],
             client: body.client,
-            action: (process.env.BASE_URL || "") + "/consent",
+            action: "consent",
           }),
         })
       })
@@ -222,19 +171,17 @@ export const createConsentPostRoute: RouteCreator =
       return
     }
 
-    const { consent_challenge: challenge, consent_action, remember } = req.body
+    const {
+      consent_challenge: challenge,
+      consent_action,
+      remember,
+      grant_scope,
+    } = req.body
 
-    // Here is also the place to add data to the ID or access token. For example,
-    // if the scope 'profile' is added, add the family and given name to the ID Token claims:
-    // if (grantScope.indexOf('profile')) {
-    //   session.id_token.family_name = 'Doe'
-    //   session.id_token.given_name = 'John'
-    // }
-    let grantScope = req.body.grant_scope
+    let grantScope = grant_scope
     if (!Array.isArray(grantScope)) {
       grantScope = [grantScope]
     }
-
     // extractSession only gets the sesseion data from the request
     // You can extract more data from the Ory Identities admin API
     const session = extractSession(req, grantScope)
@@ -244,12 +191,12 @@ export const createConsentPostRoute: RouteCreator =
     if (consent_action === "accept") {
       await oauth2
         .getOAuth2ConsentRequest({
-          consentChallenge: challenge,
+          consentChallenge: String(challenge),
         })
         .then(async ({ data: body }) => {
           return oauth2
             .acceptOAuth2ConsentRequest({
-              consentChallenge: challenge,
+              consentChallenge: String(challenge),
               acceptOAuth2ConsentRequest: {
                 // We can grant all scopes that have been requested - hydra already checked for us that no additional scopes
                 // are requested accidentally.
@@ -288,7 +235,7 @@ export const createConsentPostRoute: RouteCreator =
     // Looks like the consent request was denied by the user
     await oauth2
       .rejectOAuth2ConsentRequest({
-        consentChallenge: challenge,
+        consentChallenge: String(challenge),
         rejectOAuth2Request: {
           error: "access_denied",
           error_description: "The resource owner denied the request",
@@ -302,28 +249,18 @@ export const createConsentPostRoute: RouteCreator =
       .catch(next)
   }
 
-var parseForm = bodyParser.urlencoded({ extended: false })
-
-export const registerConsentRoute: RouteRegistrator = function (
+export const registerConsentRoute: RouteRegistrator = (
   app,
   createHelpers = defaultConfig,
-) {
-  return app.get(
+) => {
+  app.get(
     "/consent",
-    doubleCsrfProtection,
+    setSession(createHelpers),
     createConsentRoute(createHelpers),
   )
-}
-
-export const registerConsentPostRoute: RouteRegistrator = function (
-  app,
-  createHelpers = defaultConfig,
-) {
-  return app.post(
+  app.post(
     "/consent",
-    parseForm,
-    doubleCsrfProtection,
-    csrfErrorHandler,
+    setSession(createHelpers),
     createConsentPostRoute(createHelpers),
   )
 }