From 8ad0d71319c7ddbb052387382c28663eb88db0b4 Mon Sep 17 00:00:00 2001 From: djbgeodan <138100818+djbgeodan@users.noreply.github.com> Date: Wed, 4 Oct 2023 10:59:58 +0200 Subject: [PATCH 1/4] remove session_state claim condition for end provide session --- src/plugin/oauth/index.js | 49 +++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/plugin/oauth/index.js b/src/plugin/oauth/index.js index 00aafae..35e4c17 100644 --- a/src/plugin/oauth/index.js +++ b/src/plugin/oauth/index.js @@ -1511,33 +1511,32 @@ class BaseOauthPlugin extends BasePlugin { sessionPayload.tokenSet.id_token ) { let idToken = jwt.decode(sessionPayload.tokenSet.id_token); - // TODO: this check may not be entirely needed/wanted - if (idToken.session_state) { - const payload = { - redirect_uri: redirect_uri, - aud: configAudMD5, - req: { - headers: { - referer: req.headers.referer, - }, + + const payload = { + redirect_uri: redirect_uri, + aud: configAudMD5, + req: { + headers: { + referer: req.headers.referer, }, - request_is_xhr, - }; - const stateToken = jwt.sign(payload, issuer_sign_secret); - const state = plugin.server.utils.encrypt( - issuer_encrypt_secret, - stateToken, - "hex" - ); + }, + request_is_xhr, + }; + const stateToken = jwt.sign(payload, issuer_sign_secret); + const state = plugin.server.utils.encrypt( + issuer_encrypt_secret, + stateToken, + "hex" + ); - redirect_uri = await client.endSessionUrl({ - id_token_hint: sessionPayload.tokenSet.id_token, - post_logout_redirect_uri: - plugin.config.features.logout.end_provider_session - .post_logout_redirect_uri, - state, - }); - } + redirect_uri = await client.endSessionUrl({ + id_token_hint: sessionPayload.tokenSet.id_token, + post_logout_redirect_uri: + plugin.config.features.logout.end_provider_session + .post_logout_redirect_uri, + state, + }); + } plugin.server.logger.info("deleting session: %s", session_id); From 81be3a528b6eeb08156a31e74e0b6dd251b78977 Mon Sep 17 00:00:00 2001 From: Travis Glenn Hansen Date: Thu, 5 Oct 2023 10:13:57 -0400 Subject: [PATCH 2/4] various improvements to auth data and logout handling Signed-off-by: Travis Glenn Hansen --- src/plugin/oauth/index.js | 87 ++++++++++++++++++++++++++++++++++----- 1 file changed, 77 insertions(+), 10 deletions(-) diff --git a/src/plugin/oauth/index.js b/src/plugin/oauth/index.js index 00aafae..6f5744f 100644 --- a/src/plugin/oauth/index.js +++ b/src/plugin/oauth/index.js @@ -11,6 +11,7 @@ const request = require("request"); const URI = require("uri-js"); const utils = require("../../utils"); const { v4: uuidv4 } = require("uuid"); +const YAML = require("yaml"); custom.setHttpOptionsDefaults({ followRedirect: false, @@ -975,7 +976,7 @@ class BaseOauthPlugin extends BasePlugin { res.json({}); }); - server.WebServer.get("/oauth/end-session-redirect", (req, res) => { + server.WebServer.get("/oauth/end-session-redirect", async (req, res) => { server.logger.silly("%j", { headers: req.headers, body: req.body, @@ -989,6 +990,22 @@ class BaseOauthPlugin extends BasePlugin { "hex" ); state = jwt.verify(state, issuer_sign_secret); + const state_id = state.state_id; + state = await STORE_HELPER.get( + "state", + STATE_CACHE_PREFIX + state_id + ); + + if (!state) { + throw new Error(`invalid state`); + } + + STORE_HELPER.delete("state", STATE_CACHE_PREFIX + state_id).catch( + () => { + // do nothing + } + ); + const redirect_uri = state.redirect_uri; server.logger.verbose("state redirect uri: %j", redirect_uri); @@ -1149,7 +1166,7 @@ class BaseOauthPlugin extends BasePlugin { let ttl; let bc_config; if (process.env.EAS_BACKCHANNEL_LOGOUT_CONFIG) { - bc_config = JSON.parse(process.env.EAS_BACKCHANNEL_LOGOUT_CONFIG); + bc_config = YAML.parse(process.env.EAS_BACKCHANNEL_LOGOUT_CONFIG); } // checked forced values @@ -1513,7 +1530,19 @@ class BaseOauthPlugin extends BasePlugin { let idToken = jwt.decode(sessionPayload.tokenSet.id_token); // TODO: this check may not be entirely needed/wanted if (idToken.session_state) { - const payload = { + const state_id = uuidv4(); + const statePointerPayload = { + state_id, + }; + + /** + * This process will have a registered redirect_uri with the OP that should point to + * /oauth/end-session-redirect + * + * upon arrival at that endpoint the state is retrieved to get the real redirect_uri + * otherwise state is unused + */ + const statePayload = { redirect_uri: redirect_uri, aud: configAudMD5, req: { @@ -1523,7 +1552,14 @@ class BaseOauthPlugin extends BasePlugin { }, request_is_xhr, }; - const stateToken = jwt.sign(payload, issuer_sign_secret); + + // persist state + await plugin.save_state(state_id, statePayload); + + const stateToken = jwt.sign( + statePointerPayload, + issuer_sign_secret + ); const state = plugin.server.utils.encrypt( issuer_encrypt_secret, stateToken, @@ -1579,6 +1615,12 @@ class BaseOauthPlugin extends BasePlugin { plugin.server.logger.verbose("decoded state: %j", state); + if (!state) { + plugin.server.logger.verbose("invalid state"); + res.statusCode = 503; + return res; + } + const configAudMD5 = configToken.audMD5; plugin.server.logger.verbose("audMD5: %s", configAudMD5); @@ -2073,7 +2115,7 @@ class BaseOauthPlugin extends BasePlugin { Date.now() / 1000 > sessionPayload.exp ) { plugin.server.logger.verbose("session has expired"); - store.del(SESSION_CACHE_PREFIX + session_id); + plugin.delete_session(session_id); return respond_to_failed_authorization(); } @@ -2216,6 +2258,11 @@ class BaseOauthPlugin extends BasePlugin { } } + await plugin.prepare_token_headers(res, sessionPayload); + await plugin.prepare_authentication_data(res, sessionPayload); + + // TODO: run js assertions here with all authdata etc + let now = Date.now() / 1000; if ( plugin.config.features.session_expiry !== true && @@ -2228,9 +2275,6 @@ class BaseOauthPlugin extends BasePlugin { ) { await plugin.update_session(session_id, sessionPayload); } - - await plugin.prepare_token_headers(res, sessionPayload); - await plugin.prepare_authentication_data(res, sessionPayload); res.statusCode = 200; return res; } else { @@ -2255,7 +2299,7 @@ class BaseOauthPlugin extends BasePlugin { let bc_config; if (process.env.EAS_BACKCHANNEL_LOGOUT_CONFIG) { - bc_config = JSON.parse(process.env.EAS_BACKCHANNEL_LOGOUT_CONFIG); + bc_config = YAML.parse(process.env.EAS_BACKCHANNEL_LOGOUT_CONFIG); } // checked forced values @@ -2565,6 +2609,22 @@ class BaseOauthPlugin extends BasePlugin { } async prepare_authentication_data(res, sessionData) { + let id_token_decoded; + let access_token_decoded; + let refresh_token_decoded; + + try { + id_token_decoded = jwt.decode(sessionData.tokenSet.id_token); + } catch {} + + try { + access_token_decoded = jwt.decode(sessionData.tokenSet.access_token); + } catch {} + + try { + refresh_token_decoded = jwt.decode(sessionData.tokenSet.refresh_token); + } catch {} + res.setAuthenticationData({ userinfo: sessionData.userinfo && sessionData.userinfo.data @@ -2574,6 +2634,9 @@ class BaseOauthPlugin extends BasePlugin { access_token: sessionData.tokenSet.access_token, refresh_token: sessionData.tokenSet.refresh_token, token_set: sessionData.tokenSet, + id_token_decoded, + access_token_decoded, + refresh_token_decoded, }); } @@ -2789,7 +2852,11 @@ class BaseOauthPlugin extends BasePlugin { ) { plugin.server.logger.debug("verifying id_token signature"); - let secret = _.get(plugin.config, "assertions.sig.secret", issuer.jwks_uri); + let secret = _.get( + plugin.config, + "assertions.sig.secret", + issuer.jwks_uri + ); try { // resolves to decoded token, if fails will go into catch From e01e686b05ea586779bb81b7bb00cdd5ed844566 Mon Sep 17 00:00:00 2001 From: Travis Glenn Hansen Date: Thu, 5 Oct 2023 10:15:26 -0400 Subject: [PATCH 3/4] better assertion handling, support uri encoding headers Signed-off-by: Travis Glenn Hansen --- src/assertion/index.js | 10 +++++++++- src/header/index.js | 3 +++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/assertion/index.js b/src/assertion/index.js index 3254472..f1f0b11 100644 --- a/src/assertion/index.js +++ b/src/assertion/index.js @@ -78,6 +78,11 @@ class Assertion { let value = await this.query(); let test; + // prevent stupid during tests below + if (value === null || typeof value === "undefined") { + value = ""; + } + logger.debug("asserting: %j against value: %j", this.config, value); if (rule.case_insensitive) { @@ -149,6 +154,9 @@ class Assertion { test = rule.value.includes(value); break; + case "empty": + test = value.length < 1 ? true : false; + break; case "regex": /** * this splits the simple "/pattern/[flags]" syntaxt into something the @@ -181,5 +189,5 @@ class Assertion { } module.exports = { - Assertion + Assertion, }; diff --git a/src/header/index.js b/src/header/index.js index ebfbc67..e4f5763 100644 --- a/src/header/index.js +++ b/src/header/index.js @@ -33,6 +33,9 @@ class HeaderInjector { case "base64": value = base64_encode(value); break; + case "uri": + value = encodeURIComponent(value); + break; default: case "plain": // noop From 6cdf7e0d708cb243fc577237c09e55eb8525f64f Mon Sep 17 00:00:00 2001 From: Travis Glenn Hansen Date: Wed, 25 Oct 2023 12:29:52 -0400 Subject: [PATCH 4/4] prep 0.13.3 Signed-off-by: Travis Glenn Hansen --- ASSERTIONS.md | 1 + CHANGELOG.md | 11 +++++++++++ HEADERS.md | 2 +- src/header/index.js | 1 + 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/ASSERTIONS.md b/ASSERTIONS.md index 6b1e3a1..42de95c 100644 --- a/ASSERTIONS.md +++ b/ASSERTIONS.md @@ -44,6 +44,7 @@ Valid options for method are: - `contains-all` - Similar to `contains` but allows the `value` to be a list of items. If **all** of the items in `value` are found in the `query` result then the assertion passes. This assumes the `query` is returning a list of values. +- `empty` - The value is a truthy false, empty, or empty string. ## examples diff --git a/CHANGELOG.md b/CHANGELOG.md index 52d5aea..37fb1c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ +# 0.13.3 + +Released 2023-10-25 + +- support `empty` assertion strategy +- support encoding headers with `uri` encoding +- migrate `state` to be stored server-side in more scenarios +- support assertions/headers based on additional authentication data using + decoded values of various tokens `id_token_decoded`, `access_token_decoded` and + `refresh_token_decoded` + # 0.13.2 Released 2023-06-27 diff --git a/HEADERS.md b/HEADERS.md index c2ec435..e40fcf4 100644 --- a/HEADERS.md +++ b/HEADERS.md @@ -41,7 +41,7 @@ as tokens etc may not exist. source: "userinfo",// userinfo, id_token, access_token, refresh_token, static, config_token, plugin_config, req, parentRequestInfo query_engine: "jp", query: "$.emails[*].email", // if left blank the data will be passed unaltered (ie: jwt encoded data) - encoding: "plain", // may be set to base64 + encoding: "plain", // may be set to base64 or uri query_engine: "jp", query: "$.login", diff --git a/src/header/index.js b/src/header/index.js index e4f5763..f8412dd 100644 --- a/src/header/index.js +++ b/src/header/index.js @@ -34,6 +34,7 @@ class HeaderInjector { value = base64_encode(value); break; case "uri": + case "url": value = encodeURIComponent(value); break; default: