Skip to content

Commit 632a56c

Browse files
GufCabAramAlsabti
andauthored
Feature/iot 1201 simplified passport saml (#202)
* Update passport-saml * feat: use info from public certificate * refactor: update logout flow with notes for testing * refactor: add logging to logout for test * Fix CVE–2022–33171 * build(deps): bump typeorm to v0.3.6 for nestjs. * deps(build): bump typeorm to v0.3.6 * fix: typeorm migrations * fix: typeorm errors * deps(build): bump @nestjs to v9.x * Added all autofixes from npm audit 31/10/22 * User can now only be seen and edited by user admins of their own organization * Awaiting users are now also sorted based on organization * Added error message if trying to use email already in use. * Simplified the public certificate off the Passport SAML * User admin can now change email for KOMBIT users * Updated @nestjs/cli to fix minimist dependency * Minor fixes and added SSL to ORM Config CLI * Fixed moment dependency issue * Updated passport-saml in package-lock.json Co-authored-by: Aram Al-Sabti <[email protected]>
1 parent 46dbf7d commit 632a56c

File tree

59 files changed

+47396
-2338
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

59 files changed

+47396
-2338
lines changed

ormconfig.js

Lines changed: 0 additions & 16 deletions
This file was deleted.

package-lock.json

Lines changed: 2191 additions & 1989 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package-lock_BACKUP_772.json

Lines changed: 11081 additions & 0 deletions
Large diffs are not rendered by default.

package-lock_BASE_772.json

Lines changed: 11242 additions & 0 deletions
Large diffs are not rendered by default.

package-lock_LOCAL_772.json

Lines changed: 11101 additions & 0 deletions
Large diffs are not rendered by default.

package-lock_REMOTE_772.json

Lines changed: 11216 additions & 0 deletions
Large diffs are not rendered by default.

package.json

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"scripts": {
99
"prebuild": "rimraf dist",
1010
"build": "nest build",
11-
"generate-migration": "npm run typeorm migration:generate -- -n",
11+
"generate-migration": "npm run typeorm migration:generate -n",
1212
"format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"",
1313
"prestart": "npm run run-migrations",
1414
"prestart:debug": "npm run run-migrations",
@@ -25,23 +25,24 @@
2525
"test:cov": "jest --coverage",
2626
"test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
2727
"test:e2e": "jest --config ./jest-e2e.js --detectOpenHandles --colors",
28-
"typeorm": "ts-node -r tsconfig-paths/register ./node_modules/typeorm/cli.js --config ./ormconfig.js",
28+
"typeorm": "ts-node -r tsconfig-paths/register ./node_modules/typeorm/cli.js -d \"src/repositories/os2iot.repository.ts\"",
2929
"typeorm-e2e": "ts-node -r tsconfig-paths/register ./node_modules/typeorm/cli.js --config ./ormconfig-e2e.json"
3030
},
3131
"dependencies": {
32-
"@nestjs/common": "^7.6.1",
33-
"@nestjs/config": "^0.6.1",
34-
"@nestjs/core": "^7.6.1",
35-
"@nestjs/jwt": "^7.2.0",
36-
"@nestjs/passport": "^7.1.5",
37-
"@nestjs/platform-express": "^7.6.1",
38-
"@nestjs/schedule": "^0.4.1",
39-
"@nestjs/swagger": "^4.8.2",
40-
"@nestjs/typeorm": "^7.1.5",
32+
"@nestjs/axios": "^0.1.0",
33+
"@nestjs/common": "^9.1.2",
34+
"@nestjs/config": "^2.2.0",
35+
"@nestjs/core": "^9.1.2",
36+
"@nestjs/jwt": "^9.0.0",
37+
"@nestjs/passport": "^9.0.0",
38+
"@nestjs/platform-express": "^9.1.2",
39+
"@nestjs/schedule": "^2.1.0",
40+
"@nestjs/swagger": "^6.1.2",
41+
"@nestjs/typeorm": "^9.0.1",
4142
"@types/bcryptjs": "^2.4.2",
4243
"@types/geojson": "^7946.0.7",
4344
"@types/kafkajs": "^1.9.0",
44-
"@types/passport-saml": "^1.1.2",
45+
"@types/passport-saml": "^1.1.3",
4546
"@types/uuid": "^8.3.0",
4647
"@types/ws": "^8.5.3",
4748
"@types/xml2js": "^0.4.7",
@@ -56,50 +57,55 @@
5657
"kafkajs": "^1.15.0",
5758
"lodash": "^4.17.20",
5859
"mqtt": "^4.3.7",
59-
"nestjs-pino": "^1.3.0",
6060
"njwt": "^1.0.0",
6161
"nodemailer": "^6.7.2",
62-
"passport": "^0.4.1",
62+
"passport": "^0.6.0",
6363
"passport-headerapikey": "^1.2.2",
6464
"passport-jwt": "^4.0.0",
6565
"passport-local": "^1.0.0",
66-
"passport-saml": "^1.3.5",
66+
"passport-saml": "^3.2.4",
6767
"pg": "^8.5.1",
6868
"protobufjs": "^6.11.3",
6969
"reflect-metadata": "^0.1.13",
7070
"rimraf": "^3.0.2",
71-
"rxjs": "^6.6.3",
71+
"rxjs": "^7.5.7",
7272
"swagger-ui-express": "^4.1.5",
73-
"typeorm": "^0.2.34",
73+
"typeorm": "^0.3.10",
7474
"uuid": "^8.3.2",
75-
"vm2": "^3.9.9",
75+
"vm2": "^3.9.11",
7676
"wait-for-expect": "^3.0.2"
7777
},
7878
"devDependencies": {
79-
"@nestjs/cli": "^7.5.4",
80-
"@nestjs/testing": "^7.6.1",
79+
"@nestjs/cli": "^9.1.4",
80+
"@nestjs/testing": "^9.1.2",
81+
"@types/bcryptjs": "^2.4.2",
8182
"@types/bluebird": "^3.5.33",
8283
"@types/compression": "^1.7.0",
8384
"@types/cookie-parser": "^1.4.2",
8485
"@types/cron": "^1.7.2",
8586
"@types/express": "^4.17.9",
87+
"@types/geojson": "^7946.0.7",
88+
"@types/kafkajs": "^1.9.0",
8689
"@types/lodash": "^4.14.165",
8790
"@types/node": "^14.14.14",
8891
"@types/nodemailer": "^6.4.4",
8992
"@types/passport-jwt": "^3.0.3",
9093
"@types/passport-local": "^1.0.33",
94+
"@types/passport-saml": "^1.1.2",
9195
"@types/supertest": "^2.0.10",
96+
"@types/uuid": "^8.3.0",
9297
"@types/validator": "^13.7.1",
93-
"@types/ws": "^8.5.2",
94-
"@typescript-eslint/eslint-plugin": "^4.10.0",
95-
"@typescript-eslint/parser": "^4.10.0",
96-
"eslint": "^7.15.0",
98+
"@types/ws": "^8.5.3",
99+
"@types/xml2js": "^0.4.7",
100+
"@typescript-eslint/eslint-plugin": "^5.38.1",
101+
"@typescript-eslint/parser": "^5.38.1",
102+
"eslint": "^8.24.0",
97103
"eslint-config-prettier": "^7.0.0",
98104
"jest": "26.6.3",
99105
"prettier": "^2.2.1",
100106
"supertest": "^6.0.1",
101107
"ts-jest": "^26.4.4",
102-
"ts-node": "^9.1.1",
103-
"typescript": "^3.9.7"
108+
"ts-node": "^10.9.1",
109+
"typescript": "^4.8.3"
104110
}
105111
}

src/auth/kombit.strategy.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1+
import configuration from "@config/configuration";
2+
import { UserResponseDto } from "@dto/user-response.dto";
3+
import { ErrorCodes } from "@enum/error-codes.enum";
14
import { Injectable, Logger } from "@nestjs/common";
25
import { PassportStrategy } from "@nestjs/passport";
36
import { AuthService } from "@services/user-management/auth.service";
47
import { Profile, Strategy } from "passport-saml";
5-
import { UserResponseDto } from "@dto/user-response.dto";
6-
import configuration from "@config/configuration";
7-
import { ErrorCodes } from "@enum/error-codes.enum";
88

99
@Injectable()
1010
export class KombitStrategy extends PassportStrategy(Strategy, "kombit") {
@@ -26,6 +26,7 @@ export class KombitStrategy extends PassportStrategy(Strategy, "kombit") {
2626
logoutUrl: configuration()["kombit"]["entryPoint"],
2727
entryPoint: configuration()["kombit"]["entryPoint"],
2828
identifierFormat: "",
29+
cert: configuration()["kombit"]["certificatePublicKey"],
2930
privateCert: configuration()["kombit"]["certificatePrivateKey"],
3031
decryptionPvk: configuration()["kombit"]["certificatePrivateKey"],
3132
signatureAlgorithm: "sha256",

src/config/configuration.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ export default (): any => {
2323
kombit: {
2424
entryPoint:
2525
process.env.KOMBIT_ENTRYPOINT ||
26-
"https://adgangsstyring.eksterntest-stoettesystemerne.dk/runtime/saml2/issue.idp",
26+
"https://adgangsstyring.eksterntest-stoettesystemerne.dk/runtime/saml2/issue.idp",
27+
certificatePublicKey: process.env.KOMBIT_CERTIFICATEPUBLICKEY || "INSERT_KOMBIT_CERT", // Public certificate from Kombit Test server
2728
certificatePrivateKey: process.env.KOMBIT_CERTIFICATEPRIVATEKEY || null,
2829
roleUri:
2930
process.env.KOMBIT_ROLE_NAME ||

src/controllers/user-management/auth.controller.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import { Request as expressRequest, Response } from "express";
4040
import { KombitStrategy } from "@auth/kombit.strategy";
4141
import { ErrorCodes } from "@enum/error-codes.enum";
4242
import { CustomExceptionFilter } from "@auth/custom-exception-filter";
43+
import { RequestWithUser, Profile } from "passport-saml/lib/passport-saml/types";
4344
import { isOrganizationPermission } from "@helpers/security-helper";
4445

4546
@UseFilters(new CustomExceptionFilter())
@@ -99,8 +100,21 @@ export class AuthController {
99100
@UseGuards(JwtAuthGuard)
100101
@ApiOperation({ summary: "Initiates the SAML logout flow" })
101102
public async logout(@Req() req: expressRequest, @Res() res: Response): Promise<any> {
102-
this.logger.debug("Get logout Logging out ...");
103-
this.strategy.logout(req, (err: Error, url: string): void => {
103+
this.logger.debug("Logging out ...");
104+
const reqConverted: RequestWithUser = req as RequestWithUser;
105+
// TODO: Not tested as KOMBIT isn't set up locally. Test on test environment
106+
// Inspecting the source code (v3.2.1), we gather that
107+
// - ID is unknown. Might be unused or required for @InResponseTo in saml.js
108+
// - nameID is used. Corresponds to user.nameId in DB
109+
// - nameIDFormat is used. Correspond to <NameIDFormat> in the public certificate
110+
reqConverted.samlLogoutRequest = null; // Property must be set, but it is unused in the source code
111+
// reqConverted.user.nameID = reqConverted.user.nameID;
112+
reqConverted.user.nameIDFormat = "urn:oasis:names:tc:SAML:1.1:nameid-format:X509SubjectName";
113+
// reqConverted.user = { reqCo };
114+
// TODO: Remove after test
115+
this.logger.debug(`KOMBIT logout request: ${JSON.stringify(req)}`)
116+
117+
this.strategy.logout(reqConverted, (err: Error, url: string): void => {
104118
req.logout();
105119
this.logger.debug("Inside callback");
106120
if (!err) {

0 commit comments

Comments
 (0)