Skip to content

Commit

Permalink
fix: OAuth2 fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@anku255
anku255 committed Jul 29, 2024
1 parent 6974420 commit 1410df5
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 86 deletions.
4 changes: 3 additions & 1 deletion lib/build/querier.js
View file

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

11 changes: 9 additions & 2 deletions lib/build/recipe/oauth2client/recipeImplementation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ function getRecipeImplementation(_querier, config) {
return tokenResponse.jsonResponse;
},
getUserInfo: async function ({ providerConfig, oAuthTokens }) {
var _a, _b;
let jwks;
const accessToken = oAuthTokens["access_token"];
const idToken = oAuthTokens["id_token"];
Expand Down Expand Up @@ -146,9 +147,15 @@ function getRecipeImplementation(_querier, config) {
rawUserInfoFromProvider.fromUserInfoAPI = userInfoFromAccessToken.jsonResponse;
}
let userId = undefined;
if (rawUserInfoFromProvider.fromIdTokenPayload !== undefined) {
if (
((_a = rawUserInfoFromProvider.fromIdTokenPayload) === null || _a === void 0 ? void 0 : _a.sub) !==
undefined
) {
userId = rawUserInfoFromProvider.fromIdTokenPayload["sub"];
} else if (rawUserInfoFromProvider.fromUserInfoAPI !== undefined) {
} else if (
((_b = rawUserInfoFromProvider.fromUserInfoAPI) === null || _b === void 0 ? void 0 : _b.sub) !==
undefined
) {
userId = rawUserInfoFromProvider.fromUserInfoAPI["sub"];
}
if (userId === undefined) {
Expand Down
6 changes: 1 addition & 5 deletions lib/build/recipe/oauth2provider/api/implementation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,13 @@ function getAPIImplementation() {
};
},
userInfoGET: async ({ accessTokenPayload, user, scopes, tenantId, options, userContext }) => {
const userInfo = await options.recipeImplementation.buildUserInfo({
return options.recipeImplementation.buildUserInfo({
user,
accessTokenPayload,
scopes,
tenantId,
userContext,
});
return {
status: "OK",
info: userInfo,
};
},
};
}
Expand Down
19 changes: 10 additions & 9 deletions lib/build/recipe/oauth2provider/api/userInfo.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ async function userInfoGET(apiImplementation, tenantId, options, userContext) {
const accessToken = authHeader.replace(/^Bearer /, "").trim();
let accessTokenPayload;
try {
accessTokenPayload = await recipe_1.default
.getInstanceOrThrowError()
.recipeInterfaceImpl.validateOAuth2AccessToken({
token: accessToken,
// TODO: expectedAudience?
userContext,
});
const {
payload,
} = await recipe_1.default.getInstanceOrThrowError().recipeInterfaceImpl.validateOAuth2AccessToken({
token: accessToken,
// TODO: expectedAudience?
userContext,
});
accessTokenPayload = payload;
} catch (error) {
options.res.setHeader("WWW-Authenticate", 'Bearer error="invalid_token"', false);
utils_1.sendNon200ResponseWithMessage(options.res, "Invalid or expired OAuth2 access token", 400);
Expand All @@ -52,7 +53,7 @@ async function userInfoGET(apiImplementation, tenantId, options, userContext) {
accessTokenPayload === null ||
typeof accessTokenPayload !== "object" ||
typeof accessTokenPayload.sub !== "string" ||
typeof accessTokenPayload.scope !== "string"
!Array.isArray(accessTokenPayload.scp)
) {
options.res.setHeader("WWW-Authenticate", 'Bearer error="invalid_token"', false);
utils_1.sendNon200ResponseWithMessage(options.res, "Malformed access token payload", 400);
Expand All @@ -73,7 +74,7 @@ async function userInfoGET(apiImplementation, tenantId, options, userContext) {
accessTokenPayload,
user,
tenantId,
scopes: accessTokenPayload.scope.split(" "),
scopes: accessTokenPayload.scp,
options,
userContext,
});
Expand Down
63 changes: 36 additions & 27 deletions lib/build/recipe/oauth2provider/recipeImplementation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,15 @@ const utils_1 = require("../../utils");
const OAuth2Client_1 = require("./OAuth2Client");
const __1 = require("../..");
const combinedRemoteJWKSet_1 = require("../../combinedRemoteJWKSet");
// TODO: Remove this core changes are done
function getUpdatedRedirectTo(appInfo, redirectTo) {
return redirectTo
.replace(
querier_1.hydraPubDomain,
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous()
)
.replace("oauth2", "oauth2provider");
}
function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload, getDefaultUserInfoPayload) {
return {
getLoginRequest: async function (input) {
Expand Down Expand Up @@ -103,10 +112,7 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
);
return {
// TODO: FIXME!!!
redirectTo: resp.data.redirect_to.replace(
querier_1.hydraPubDomain,
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous()
),
redirectTo: getUpdatedRedirectTo(appInfo, resp.data.redirect_to),
};
},
rejectLoginRequest: async function (input) {
Expand All @@ -126,10 +132,7 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
);
return {
// TODO: FIXME!!!
redirectTo: resp.data.redirect_to.replace(
querier_1.hydraPubDomain,
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous()
),
redirectTo: getUpdatedRedirectTo(appInfo, resp.data.redirect_to),
};
},
getConsentRequest: async function (input) {
Expand Down Expand Up @@ -172,10 +175,7 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
);
return {
// TODO: FIXME!!!
redirectTo: resp.data.redirect_to.replace(
querier_1.hydraPubDomain,
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous()
),
redirectTo: getUpdatedRedirectTo(appInfo, resp.data.redirect_to),
};
},
rejectConsentRequest: async function (input) {
Expand All @@ -195,10 +195,7 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
);
return {
// TODO: FIXME!!!
redirectTo: resp.data.redirect_to.replace(
querier_1.hydraPubDomain,
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous()
),
redirectTo: getUpdatedRedirectTo(appInfo, resp.data.redirect_to),
};
},
authorization: async function (input) {
Expand Down Expand Up @@ -237,7 +234,7 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
scopes: consentRequest.requestedScope || [],
userContext: input.userContext,
});
const accessTokenPayload = this.buildAccessTokenPayload({
const accessTokenPayload = await this.buildAccessTokenPayload({
user,
session: input.session,
defaultPayload: Object.assign(
Expand All @@ -251,6 +248,7 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
: _b.join(" ")) !== null && _c !== void 0
? _c
: "",
aud: [consentRequest.client.clientId],
}
),
userContext: input.userContext,
Expand Down Expand Up @@ -405,36 +403,47 @@ function getRecipeInterface(querier, _config, appInfo, getDefaultIdTokenPayload,
return getDefaultUserInfoPayload(user, accessTokenPayload, scopes, tenantId, userContext);
},
validateOAuth2AccessToken: async function (input) {
var _a, _b, _c, _d;
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 &&
!(payload.aud instanceof Array && payload.aud.includes(input.expectedAudience))
) {
// TODO: Fix this
const aud =
(_b = (_a = payload.ext) === null || _a === void 0 ? void 0 : _a.aud) !== null && _b !== void 0
? _b
: payload.aud instanceof Array
? payload.aud
: (_d = (_c = payload.aud) === null || _c === void 0 ? void 0 : _c.split(" ")) !== null &&
_d !== void 0
? _d
: [];
if (input.expectedAudience !== undefined && !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
return { status: "OK", payload: payload };
},
validateOAuth2IdToken: async function (input) {
var _a, _b;
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 &&
!(payload.aud instanceof Array && payload.aud.includes(input.expectedAudience))
) {
const aud =
payload.aud instanceof Array
? payload.aud
: (_b = (_a = payload.aud) === null || _a === void 0 ? void 0 : _a.split(" ")) !== null &&
_b !== void 0
? _b
: [];
if (input.expectedAudience !== undefined && !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
8 changes: 1 addition & 7 deletions lib/build/recipe/oauth2provider/types.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,7 @@ export declare type APIInterface = {
tenantId: string;
options: APIOptions;
userContext: UserContext;
}) => Promise<
| {
status: "OK";
info: JSONObject;
}
| GeneralErrorResponse
>);
}) => Promise<JSONObject | GeneralErrorResponse>);
};
export declare type OAuth2ClientOptions = {
clientId: string;
Expand Down
4 changes: 3 additions & 1 deletion lib/ts/querier.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ export class Querier {
let headers: any = {
"cdi-version": apiVersion,
};
if (!isForm) {
if (isForm) {
headers["content-type"] = "application/x-www-form-urlencoded";
} else {
headers["content-type"] = "application/json; charset=utf-8";
}
if (Querier.apiKey !== undefined) {
Expand Down
4 changes: 2 additions & 2 deletions lib/ts/recipe/oauth2client/recipeImplementation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ export default function getRecipeImplementation(_querier: Querier, config: TypeN

let userId: string | undefined = undefined;

if (rawUserInfoFromProvider.fromIdTokenPayload !== undefined) {
if (rawUserInfoFromProvider.fromIdTokenPayload?.sub !== undefined) {
userId = rawUserInfoFromProvider.fromIdTokenPayload["sub"];
} else if (rawUserInfoFromProvider.fromUserInfoAPI !== undefined) {
} else if (rawUserInfoFromProvider.fromUserInfoAPI?.sub !== undefined) {
userId = rawUserInfoFromProvider.fromUserInfoAPI["sub"];
}

Expand Down
7 changes: 1 addition & 6 deletions lib/ts/recipe/oauth2provider/api/implementation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,13 @@ export default function getAPIImplementation(): APIInterface {
};
},
userInfoGET: async ({ accessTokenPayload, user, scopes, tenantId, options, userContext }) => {
const userInfo = await options.recipeImplementation.buildUserInfo({
return options.recipeImplementation.buildUserInfo({
user,
accessTokenPayload,
scopes,
tenantId,
userContext,
});

return {
status: "OK",
info: userInfo,
};
},
};
}
19 changes: 10 additions & 9 deletions lib/ts/recipe/oauth2provider/api/userInfo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,14 @@ export default async function userInfoGET(
let accessTokenPayload: JSONObject;

try {
accessTokenPayload = await OAuth2ProviderRecipe.getInstanceOrThrowError().recipeInterfaceImpl.validateOAuth2AccessToken(
{
token: accessToken,
// TODO: expectedAudience?
userContext,
}
);
const {
payload,
} = await OAuth2ProviderRecipe.getInstanceOrThrowError().recipeInterfaceImpl.validateOAuth2AccessToken({
token: accessToken,
// TODO: expectedAudience?
userContext,
});
accessTokenPayload = payload;
} catch (error) {
options.res.setHeader("WWW-Authenticate", 'Bearer error="invalid_token"', false);
sendNon200ResponseWithMessage(options.res, "Invalid or expired OAuth2 access token", 400);
Expand All @@ -60,7 +61,7 @@ export default async function userInfoGET(
accessTokenPayload === null ||
typeof accessTokenPayload !== "object" ||
typeof accessTokenPayload.sub !== "string" ||
typeof accessTokenPayload.scope !== "string"
!Array.isArray(accessTokenPayload.scp)
) {
options.res.setHeader("WWW-Authenticate", 'Bearer error="invalid_token"', false);
sendNon200ResponseWithMessage(options.res, "Malformed access token payload", 400);
Expand All @@ -81,7 +82,7 @@ export default async function userInfoGET(
accessTokenPayload,
user,
tenantId,
scopes: accessTokenPayload.scope.split(" "),
scopes: accessTokenPayload.scp as string[],
options,
userContext,
});
Expand Down
27 changes: 11 additions & 16 deletions lib/ts/recipe/oauth2provider/recipeImplementation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ import { getUser } from "../..";
import { getCombinedJWKS } from "../../combinedRemoteJWKSet";
import { JSONObject } from "@loopback/core";

// TODO: Remove this core changes are done
function getUpdatedRedirectTo(appInfo: NormalisedAppinfo, redirectTo: string) {
return redirectTo
.replace(hydraPubDomain, appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous())
.replace("oauth2", "oauth2provider");
}

export default function getRecipeInterface(
querier: Querier,
_config: TypeNormalisedInput,
Expand Down Expand Up @@ -80,10 +87,7 @@ export default function getRecipeInterface(

return {
// TODO: FIXME!!!
redirectTo: resp.data.redirect_to.replace(
hydraPubDomain,
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous()
),
redirectTo: getUpdatedRedirectTo(appInfo, resp.data.redirect_to),
};
},
rejectLoginRequest: async function (this: RecipeInterface, input): Promise<{ redirectTo: string }> {
Expand All @@ -104,10 +108,7 @@ export default function getRecipeInterface(

return {
// TODO: FIXME!!!
redirectTo: resp.data.redirect_to.replace(
hydraPubDomain,
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous()
),
redirectTo: getUpdatedRedirectTo(appInfo, resp.data.redirect_to),
};
},
getConsentRequest: async function (this: RecipeInterface, input): Promise<ConsentRequest> {
Expand Down Expand Up @@ -152,10 +153,7 @@ export default function getRecipeInterface(

return {
// TODO: FIXME!!!
redirectTo: resp.data.redirect_to.replace(
hydraPubDomain,
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous()
),
redirectTo: getUpdatedRedirectTo(appInfo, resp.data.redirect_to),
};
},

Expand All @@ -177,10 +175,7 @@ export default function getRecipeInterface(

return {
// TODO: FIXME!!!
redirectTo: resp.data.redirect_to.replace(
hydraPubDomain,
appInfo.apiDomain.getAsStringDangerous() + appInfo.apiBasePath.getAsStringDangerous()
),
redirectTo: getUpdatedRedirectTo(appInfo, resp.data.redirect_to),
};
},
authorization: async function (this: RecipeInterface, input) {
Expand Down
Loading

0 comments on commit 1410df5

Please sign in to comment.