Skip to content

Commit

Permalink
fix: consent and logout integration logic
Browse files Browse the repository at this point in the history
  • Loading branch information
Benehiko committed Sep 18, 2023
1 parent 768fafd commit 775f111
Show file tree
Hide file tree
Showing 10 changed files with 241 additions and 120 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
47 changes: 42 additions & 5 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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"
Expand All @@ -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(
Expand All @@ -53,8 +86,6 @@ app.engine(

registerHealthRoute(router)
registerLoginRoute(router)
registerConsentRoute(app)
registerConsentPostRoute(app)
registerRecoveryRoute(router)
registerRegistrationRoute(router)
registerSettingsRoute(router)
Expand All @@ -67,10 +98,16 @@ router.get("/", (req: Request, res: Response) => {
res.redirect(303, "welcome")
})

// all routes registered after this point are protected by CSRF
app.use(doubleCsrfProtection)
app.use(csrfErrorHandler(invalidCsrfTokenError))

registerConsentRoute(app)
registerLogoutRoute(app)

registerStaticRoutes(router)
register404Route(router)
register500Route(router)

app.use(baseUrl, router)

const port = Number(process.env.PORT) || 3000
Expand Down
12 changes: 12 additions & 0 deletions src/pkg/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down
11 changes: 10 additions & 1 deletion src/pkg/route.ts
Original file line number Diff line number Diff line change
@@ -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"

Expand All @@ -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
Expand Down
Loading

0 comments on commit 775f111

Please sign in to comment.