Skip to content

Commit

Permalink
fix: add workaround to validate access/idtokens
Browse files Browse the repository at this point in the history
  • Loading branch information
@porcellus
porcellus committed Jul 29, 2024
1 parent 6950da7 commit 6974420
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 38 deletions.
16 changes: 16 additions & 0 deletions lib/build/combinedRemoteJWKSet.js
View file

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

58 changes: 39 additions & 19 deletions lib/build/recipe/oauth2provider/recipeImplementation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
};
},
authorization: async function (input) {
var _a, _b, _c;
var _a, _b, _c, _d, _e;
const resp = await querier.sendGetRequestWithResponseHeaders(
new normalisedURLPath_1.default(`/recipe/oauth2/pub/auth`),
input.params,
Expand Down Expand Up @@ -240,7 +240,19 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
const accessTokenPayload = this.buildAccessTokenPayload({
user,
session: input.session,
defaultPayload: input.session.getAccessTokenPayload(input.userContext),
defaultPayload: Object.assign(
Object.assign({}, input.session.getAccessTokenPayload(input.userContext)),
{
iss: appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous(),
scope:
(_c =
(_b = consentRequest.requestedScope) === null || _b === void 0
? void 0
: _b.join(" ")) !== null && _c !== void 0
? _c
: "",
}
),
userContext: input.userContext,
scopes: consentRequest.requestedScope || [],
});
Expand All @@ -257,12 +269,12 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
);
return {
redirectTo: consentRes.redirectTo,
setCookie: (_b = resp.headers.get("set-cookie")) !== null && _b !== void 0 ? _b : undefined,
setCookie: (_d = resp.headers.get("set-cookie")) !== null && _d !== void 0 ? _d : undefined,
};
}
return {
redirectTo,
setCookie: (_c = resp.headers.get("set-cookie")) !== null && _c !== void 0 ? _c : undefined,
setCookie: (_e = resp.headers.get("set-cookie")) !== null && _e !== void 0 ? _e : undefined,
};
},
token: async function (input) {
Expand Down Expand Up @@ -318,9 +330,9 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
new normalisedURLPath_1.default(`/recipe/oauth2/admin/clients`),
Object.assign(Object.assign({}, utils_1.transformObjectKeys(input, "snake-case")), {
// TODO: these defaults should be set/enforced on the core side
accessTokenStrategy: "jwt",
skipConsent: true,
subjectType: "public",
access_token_strategy: "jwt",
skip_consent: true,
subject_type: "public",
}),
userContext
);
Expand Down Expand Up @@ -395,12 +407,16 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
validateOAuth2AccessToken: async function (input) {
const payload = (await jose.jwtVerify(input.token, combinedRemoteJWKSet_1.getCombinedJWKS())).payload;
// TODO: make this configurable?
const expectedIssuer =
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous();
if (payload.iss !== expectedIssuer) {
throw new Error("Issuer mismatch: this token was likely issued by another application or spoofed");
}
if (input.expectedAudience !== undefined && payload.aud !== input.expectedAudience) {
// const expectedIssuer =
// appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous();
// if (payload.iss !== expectedIssuer) {
// throw new Error("Issuer mismatch: this token was likely issued by another application or spoofed");
// }
if (
input.expectedAudience !== undefined &&
payload.aud !== input.expectedAudience &&
!(payload.aud instanceof Array && payload.aud.includes(input.expectedAudience))
) {
throw new Error("Audience mismatch: this token doesn't belong to the specified client");
}
// TODO: add a check to make sure this is the right token type as they can be signed with the same key
Expand All @@ -409,12 +425,16 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
validateOAuth2IdToken: async function (input) {
const payload = (await jose.jwtVerify(input.token, combinedRemoteJWKSet_1.getCombinedJWKS())).payload;
// TODO: make this configurable?
const expectedIssuer =
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous();
if (input.expectedAudience !== undefined && payload.iss !== expectedIssuer) {
throw new Error("Issuer mismatch: this token was likely issued by another application or spoofed");
}
if (input.expectedAudience !== undefined && payload.aud !== input.expectedAudience) {
// const expectedIssuer =
// appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous();
// if (input.expectedAudience !== undefined && payload.iss !== expectedIssuer) {
// throw new Error("Issuer mismatch: this token was likely issued by another application or spoofed");
// }
if (
input.expectedAudience !== undefined &&
payload.aud !== input.expectedAudience &&
!(payload.aud instanceof Array && payload.aud.includes(input.expectedAudience))
) {
throw new Error("Audience mismatch: this token doesn't belong to the specified client");
}
// TODO: add a check to make sure this is the right token type as they can be signed with the same key
Expand Down
10 changes: 10 additions & 0 deletions lib/ts/combinedRemoteJWKSet.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ export function resetCombinedJWKS() {
combinedJWKS = undefined;
}

// TODO: remove this after proper core support
const hydraJWKS = createRemoteJWKSet(new URL("http://localhost:4444/.well-known/jwks.json"), {
cooldownDuration: JWKCacheCooldownInMs,
cacheMaxAge: JWKCacheMaxAgeInMs,
});
/**
The function returned by this getter fetches all JWKs from the first available core instance.
This combines the other JWKS functions to become error resistant.
Expand All @@ -34,6 +39,11 @@ export function getCombinedJWKS() {

combinedJWKS = async (...args) => {
let lastError = undefined;

if (!args[0]?.kid?.startsWith("s-") && !args[0]?.kid?.startsWith("d-")) {
return hydraJWKS(...args);
}

if (JWKS.length === 0) {
throw Error(
"No SuperTokens core available to query. Please pass supertokens > connectionURI to the init function, or override all the functions of the recipe you are using."
Expand Down
47 changes: 28 additions & 19 deletions lib/ts/recipe/oauth2provider/recipeImplementation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,10 +221,15 @@ export default function getRecipeInterface(
userContext: input.userContext,
});

const accessTokenPayload = this.buildAccessTokenPayload({
const accessTokenPayload = await this.buildAccessTokenPayload({
user,
session: input.session,
defaultPayload: input.session.getAccessTokenPayload(input.userContext), // TODO: validate
defaultPayload: {
...input.session.getAccessTokenPayload(input.userContext), // TODO: validate based on access token structure rfc
iss: appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous(),
scope: consentRequest.requestedScope?.join(" ") ?? "",
aud: [consentRequest.client!.clientId],
},
userContext: input.userContext,
scopes: consentRequest.requestedScope || [],
});
Expand Down Expand Up @@ -308,9 +313,9 @@ export default function getRecipeInterface(
{
...transformObjectKeys(input, "snake-case"),
// TODO: these defaults should be set/enforced on the core side
accessTokenStrategy: "jwt",
skipConsent: true,
subjectType: "public",
access_token_strategy: "jwt",
skip_consent: true,
subject_type: "public",
},
userContext
);
Expand Down Expand Up @@ -392,13 +397,17 @@ export default function getRecipeInterface(
const payload = (await jose.jwtVerify(input.token, getCombinedJWKS())).payload;

// TODO: make this configurable?
const expectedIssuer =
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous();
if (payload.iss !== expectedIssuer) {
throw new Error("Issuer mismatch: this token was likely issued by another application or spoofed");
}

if (input.expectedAudience !== undefined && payload.aud !== input.expectedAudience) {
// const expectedIssuer =
// appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous();
// if (payload.iss !== expectedIssuer) {
// throw new Error("Issuer mismatch: this token was likely issued by another application or spoofed");
// }

// TODO: Fix this
const aud =
(payload.ext as any)?.aud ??
(payload.aud instanceof Array ? payload.aud : payload.aud?.split(" ") ?? []);
if (input.expectedAudience !== undefined && !aud.includes(input.expectedAudience)) {
throw new Error("Audience mismatch: this token doesn't belong to the specified client");
}

Expand All @@ -410,13 +419,13 @@ export default function getRecipeInterface(
const payload = (await jose.jwtVerify(input.token, getCombinedJWKS())).payload;

// TODO: make this configurable?
const expectedIssuer =
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous();
if (input.expectedAudience !== undefined && payload.iss !== expectedIssuer) {
throw new Error("Issuer mismatch: this token was likely issued by another application or spoofed");
}

if (input.expectedAudience !== undefined && payload.aud !== input.expectedAudience) {
// const expectedIssuer =
// appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous();
// if (input.expectedAudience !== undefined && payload.iss !== expectedIssuer) {
// throw new Error("Issuer mismatch: this token was likely issued by another application or spoofed");
// }
const aud = payload.aud instanceof Array ? payload.aud : payload.aud?.split(" ") ?? [];
if (input.expectedAudience !== undefined && !aud.includes(input.expectedAudience)) {
throw new Error("Audience mismatch: this token doesn't belong to the specified client");
}

Expand Down
26 changes: 26 additions & 0 deletions test/test-server/src/oauth2provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,32 @@ const router = Router()
} catch (e) {
next(e);
}
})
.post("/validateoauth2accesstoken", async (req, res, next) => {
try {
logDebugMessage("OAuth2Provider:validateOAuth2AccessToken %j", req.body);
const response = await OAuth2Provider.validateOAuth2AccessToken(
req.body.token,
req.body.expectedAudience,
req.body.userContext
);
res.json(response);
} catch (e) {
next(e);
}
})
.post("/validateoauth2idtoken", async (req, res, next) => {
try {
logDebugMessage("OAuth2Provider:validateOAuth2IdToken %j", req.body);
const response = await OAuth2Provider.validateOAuth2IdToken(
req.body.token,
req.body.expectedAudience,
req.body.userContext
);
res.json(response);
} catch (e) {
next(e);
}
});

export default router;

0 comments on commit 6974420

Please sign in to comment.