From 4a539e191d6aea9805dee6162da0bc2e3ff03e59 Mon Sep 17 00:00:00 2001 From: Stanislav Mishchyshyn Date: Wed, 8 Nov 2023 17:55:16 +0200 Subject: [PATCH] fix(registry): fix openid identifier match (case-insensitive) --- e2e/spec/news.spec.e2e.ts | 2 +- ilc/package-lock.json | 22 --------- registry/package-lock.json | 49 ------------------- registry/package.json | 2 +- registry/server/auth.ts | 12 +++-- .../templates/services/resources/Resource.ts | 5 +- .../templates/services/templatesRepository.ts | 11 +++-- registry/tests/auth.spec.ts | 47 +++++++++++++++++- start_all.js | 2 +- 9 files changed, 67 insertions(+), 85 deletions(-) diff --git a/e2e/spec/news.spec.e2e.ts b/e2e/spec/news.spec.e2e.ts index 31b03ab76..ef8807d0e 100644 --- a/e2e/spec/news.spec.e2e.ts +++ b/e2e/spec/news.spec.e2e.ts @@ -12,7 +12,7 @@ Scenario('should open a news page and show news sources', async ({ I, newsPage: I.see('Pick a news source', newsPage.bannerHeadline); }); -Scenario.skip('should open an article page from a direct link', async ({ I, newsPage: newsPage }) => { +Scenario.only('should open an article page from a direct link', async ({ I, newsPage: newsPage }) => { I.amOnPage(newsPage.url.main); I.waitInUrl(newsPage.url.main, 10); I.waitForElement(newsPage.newsSources, 10); diff --git a/ilc/package-lock.json b/ilc/package-lock.json index 94ecc7b12..6de5f1ef0 100644 --- a/ilc/package-lock.json +++ b/ilc/package-lock.json @@ -63,7 +63,6 @@ "karma-parallel": "^0.3.1", "karma-sinon": "^1.0.5", "karma-sourcemap-loader": "^0.4.0", - "karma-spec-reporter": "^0.0.36", "karma-webpack": "^5.0.0", "mocha": "^10.2.0", "nanohtml": "^1.10.0", @@ -4808,15 +4807,6 @@ "integrity": "sha512-IfEDxwoWIjkeXL1eXcDiow4UbKjhLdq6/EuSVR9GMN7KVH3r9gQ83e73hsz1Nd1T3ijd5xv1wcWRYO+D6kCI2w==", "dev": true }, - "node_modules/colors": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/colors/-/colors-1.4.0.tgz", - "integrity": "sha512-a+UqTh4kgZg/SlGvfbzDHpgRu7AAQOmmqRHJnxhRZICKFUT91brVhNNt58CMWU9PsBbv3PDCZUHbVxuDiH2mtA==", - "dev": true, - "engines": { - "node": ">=0.1.90" - } - }, "node_modules/combined-stream": { "version": "1.0.8", "resolved": "https://registry.npmjs.org/combined-stream/-/combined-stream-1.0.8.tgz", @@ -7516,18 +7506,6 @@ "graceful-fs": "^4.2.10" } }, - "node_modules/karma-spec-reporter": { - "version": "0.0.36", - "resolved": "https://registry.npmjs.org/karma-spec-reporter/-/karma-spec-reporter-0.0.36.tgz", - "integrity": "sha512-11bvOl1x6ryKZph7kmbmMpbi8vsngEGxGOoeTlIcDaH3ab3j8aPJnZ+r+K/SS0sBSGy5VGkGYO2+hLct7hw/6w==", - "dev": true, - "dependencies": { - "colors": "1.4.0" - }, - "peerDependencies": { - "karma": ">=0.9" - } - }, "node_modules/karma-webpack": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/karma-webpack/-/karma-webpack-5.0.0.tgz", diff --git a/registry/package-lock.json b/registry/package-lock.json index a7ae654cb..d308703fc 100644 --- a/registry/package-lock.json +++ b/registry/package-lock.json @@ -10,7 +10,6 @@ "license": "Apache-2.0", "dependencies": { "@namecheap/error-extender": "^1.2.0", - "@nc_ui/ilc-plugin-reporting-spaceship": "file:../../ilc-plugin-reporting-spaceship/nc_ui-ilc-plugin-reporting-spaceship-1.5.2.tgz", "@newrelic/native-metrics": "^10.0.1", "axios": "^0.26.1", "bcrypt": "^5.1.1", @@ -1582,35 +1581,6 @@ "resolved": "https://registry.npmjs.org/@namecheap/error-extender/-/error-extender-1.2.0.tgz", "integrity": "sha512-EQh8nDsV9LPTpa18DARIbIuIDf4d+MybGtDhixTld/5us7smcbGEpHvs8FxxP6bygQhntewDprSK52s4eQxVww==" }, - "node_modules/@nc_ui/ilc-plugin-reporting-spaceship": { - "version": "1.5.2", - "resolved": "file:../../ilc-plugin-reporting-spaceship/nc_ui-ilc-plugin-reporting-spaceship-1.5.2.tgz", - "integrity": "sha512-5RD4yfvnmGfmDY1/Ju8K9ZKQAPFvrKVcPERJBYVXKkIaSNgsaEQQQdxadAbiiVxIMyp/4MOHnBXA1OA/6F56AQ==", - "license": "UNLICENSED", - "dependencies": { - "@types/pino": "^7.0.4", - "@types/uniqid": "^5.3.2", - "http-headers": "^3.0.2", - "ilc-plugins-sdk": "file:../ilc-plugins-sdk/ilc-plugins-sdk-2.0.0.tgz", - "pino": "^8.16.0", - "uniqid": "^5.4.0" - }, - "engines": { - "node": ">=16" - } - }, - "node_modules/@nc_ui/ilc-plugin-reporting-spaceship/node_modules/ilc-plugins-sdk": { - "version": "2.0.0", - "resolved": "file:../../ilc-plugins-sdk/ilc-plugins-sdk-2.0.0.tgz", - "integrity": "sha512-ZsTq1Z8utX/vWfTTY6QjEMCcXhJxONtzfJW7leL/37tLz/jKmE1u+wVLlDQma297GZ6zLv9s9sNH9gJV1OlWgw==", - "license": "Apache-2.0", - "dependencies": { - "fast-glob": "^3.3.1", - "http-headers": "^3.0.2", - "hyperid": "^3.1.1", - "pino": "^8.16.0" - } - }, "node_modules/@newrelic/aws-sdk": { "version": "7.0.1", "resolved": "https://registry.npmjs.org/@newrelic/aws-sdk/-/aws-sdk-7.0.1.tgz", @@ -2743,15 +2713,6 @@ "@types/passport": "*" } }, - "node_modules/@types/pino": { - "version": "7.0.5", - "resolved": "https://registry.npmjs.org/@types/pino/-/pino-7.0.5.tgz", - "integrity": "sha512-wKoab31pknvILkxAF8ss+v9iNyhw5Iu/0jLtRkUD74cNfOOLJNnqfFKAv0r7wVaTQxRZtWrMpGfShwwBjOcgcg==", - "deprecated": "This is a stub types definition. pino provides its own type definitions, so you do not need this installed.", - "dependencies": { - "pino": "*" - } - }, "node_modules/@types/qs": { "version": "6.9.8", "resolved": "https://registry.npmjs.org/@types/qs/-/qs-6.9.8.tgz", @@ -2829,11 +2790,6 @@ "resolved": "https://registry.npmjs.org/@types/triple-beam/-/triple-beam-1.3.3.tgz", "integrity": "sha512-6tOUG+nVHn0cJbVp25JFayS5UE6+xlbcNF9Lo9mU7U0zk3zeUShZied4YEQZjy1JBF043FSkdXw8YkUJuVtB5g==" }, - "node_modules/@types/uniqid": { - "version": "5.3.2", - "resolved": "https://registry.npmjs.org/@types/uniqid/-/uniqid-5.3.2.tgz", - "integrity": "sha512-/NYoaZpWsnAJDsGYeMNDeG3p3fuUb4AiC7MfKxi5VSu18tXd08w6Ch0fKW94T4FeLXXZwZPoFgHA1O0rDYKyMQ==" - }, "node_modules/@types/url-join": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/@types/url-join/-/url-join-4.0.1.tgz", @@ -9559,11 +9515,6 @@ "string.fromcodepoint": "^0.2.1" } }, - "node_modules/uniqid": { - "version": "5.4.0", - "resolved": "https://registry.npmjs.org/uniqid/-/uniqid-5.4.0.tgz", - "integrity": "sha512-38JRbJ4Fj94VmnC7G/J/5n5SC7Ab46OM5iNtSstB/ko3l1b5g7ALt4qzHFgGciFkyiRNtDXtLNb+VsxtMSE77A==" - }, "node_modules/unique-filename": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/unique-filename/-/unique-filename-1.1.1.tgz", diff --git a/registry/package.json b/registry/package.json index c2e469e59..46a01cc30 100644 --- a/registry/package.json +++ b/registry/package.json @@ -8,7 +8,7 @@ "compile": "tsc --incremental", "dev": "nodemon -e ts,json5 --exec \"npx tsc --incremental && dotenv -- npm run start\" ", "build": "npm run compile && cd ./client && npm run build", - "start": "node -r source-map-support/register ./build/server/index.js", + "start": "dotenv -- node -r source-map-support/register ./build/server/index.js", "start-docker": "npm run migrate && npm start", "knex": "knex", "migrate": "dotenv -- knex --knexfile config/knexfile.ts migrate:latest", diff --git a/registry/server/auth.ts b/registry/server/auth.ts index 5f6cd1b54..97dcdbe61 100644 --- a/registry/server/auth.ts +++ b/registry/server/auth.ts @@ -318,10 +318,14 @@ export default async (app: Express, settingsService: SettingsService, config: an }; async function getEntityWithCreds(provider: string, identifier: string, secret: string | null): Promise { - const user = await db.select().from('auth_entities').first('identifier', 'id', 'role', 'secret').where({ - provider, - identifier, - }); + const user = await db + .select() + .from('auth_entities') + .first('identifier', 'id', 'role', 'secret') + .where({ + provider, + }) + .andWhereRaw('LOWER(identifier) = LOWER(?)', [identifier]); if (!user) { return null; } diff --git a/registry/server/templates/services/resources/Resource.ts b/registry/server/templates/services/resources/Resource.ts index b2b83dd8f..fee2f6404 100644 --- a/registry/server/templates/services/resources/Resource.ts +++ b/registry/server/templates/services/resources/Resource.ts @@ -18,7 +18,10 @@ export abstract class Resource { ...Attributes.crossorigin, }; - constructor(public uri: string, params?: Params) { + constructor( + public uri: string, + params?: Params, + ) { this.params = params || {}; } diff --git a/registry/server/templates/services/templatesRepository.ts b/registry/server/templates/services/templatesRepository.ts index 49dc5238d..1aca45bf8 100644 --- a/registry/server/templates/services/templatesRepository.ts +++ b/registry/server/templates/services/templatesRepository.ts @@ -14,10 +14,13 @@ export async function readTemplateWithAllVersions(templateName: string) { .select() .from(tables.templatesLocalized) .where('templateName', templateName); - template.localizedVersions = localizedTemplates.reduce((acc, item) => { - acc[item.locale] = { content: item.content }; - return acc; - }, {} as Record); + template.localizedVersions = localizedTemplates.reduce( + (acc, item) => { + acc[item.locale] = { content: item.content }; + return acc; + }, + {} as Record, + ); return template; } diff --git a/registry/tests/auth.spec.ts b/registry/tests/auth.spec.ts index 0c24913ec..d7f5d9087 100644 --- a/registry/tests/auth.spec.ts +++ b/registry/tests/auth.spec.ts @@ -15,6 +15,8 @@ import nock from 'nock'; import * as bcrypt from 'bcrypt'; import { muteConsole, unmuteConsole } from './utils/console'; import { loadPlugins } from '../server/util/pluginManager'; +import { isSqlite } from '../server/util/db'; +import knex from 'knex'; const generateResp403 = (username: string) => ({ message: `Access denied. "${username}" has "readonly" access.`, @@ -76,11 +78,11 @@ describe('Authentication / Authorization', () => { Buffer.from('token_secret', 'utf8').toString('base64'); }); - it('should not authenticate with invalid creds', async () => { + it('should not authenticate with invalid credentails', async () => { await request.get('/protected').set('Authorization', `Bearer invalid`).expect(401); }); - it('should authenticate with correct creds', async () => { + it('should authenticate with correct credentails', async () => { await request.get('/protected').set('Authorization', `Bearer ${authToken}`).expect(200, 'ok'); }); @@ -363,6 +365,47 @@ describe('Authentication / Authorization', () => { await agent.get('/auth/logout').expect(302); await agent.get('/protected').expect(401); }); + it('should authenticate against OpenID server (case-insensitive identifier)', async () => { + await db('auth_entities').where('identifier', userIdentifier.toUpperCase()).delete(); + await db('auth_entities').insert({ + identifier: userIdentifier.toUpperCase(), + provider: 'openid', + role: 'admin', + }); + const res = await agent + .get('/auth/openid') + .expect(302) + .expect( + 'Location', + new RegExp( + 'https://ad\\.example\\.doesnotmatter\\.com/adfs/oauth2/authorize/\\?client_id=ba05c345-e144-4688-b0be-3e1097ddd32d&scope=openid&response_type=code&redirect_uri=http%3A%2F%2Flocalhost%3A4000%2Fauth%2Fopenid%2Freturn&state=.+?$', + ), + ); + + await agent + .get(`/auth/openid/return?${getQueryOfCodeAndSessionState(res.header['location'])}`) + .expect(302) + .expect('Location', '/') + .expect((res) => { + let setCookie = res.header['set-cookie']; + assert.ok(Array.isArray(setCookie)); + assert.ok(setCookie[0]); + + const parts: any = querystring.parse(setCookie[0].replace(/\s?;\s?/, '&')); + const userInfo = JSON.parse(parts['ilc:userInfo']); + + assert.strictEqual(userInfo.identifier, userIdentifier.toUpperCase()); + assert.strictEqual(userInfo.role, 'admin'); + }); + + // respect session cookie + await agent.get('/protected').expect(200, 'ok'); + + // correctly logout + await agent.get('/auth/logout').expect(302); + await agent.get('/protected').expect(401); + await db('auth_entities').where('identifier', userIdentifier.toUpperCase()).delete(); + }); it('should authenticate against OpenID server & perform impersonation', async () => { getStub.withArgs(SettingKeys.AuthOpenIdUniqueIdentifierClaimName).returns(Promise.resolve('upn')); diff --git a/start_all.js b/start_all.js index 956b45e69..83bc8cd38 100644 --- a/start_all.js +++ b/start_all.js @@ -19,7 +19,7 @@ const commands = [ }, { cwd: 'registry', - command: `npm run migrate && npm run seed && npm run ${noWatch ? 'start' : 'dev'}`, + command: `npm run migrate && npm run ${noWatch ? 'start' : 'dev'}`, name: 'registry', }, ];