From 0e7b58cdceeadd6a38d8e09ce0c960fe2cce5967 Mon Sep 17 00:00:00 2001 From: Stanislav Mishchyshyn Date: Wed, 8 Nov 2023 18:05:35 +0200 Subject: [PATCH] fix(registry): fix openid identifier match (case-insensitive) --- registry/server/auth.ts | 12 +++-- .../templates/services/resources/Resource.ts | 5 +- .../templates/services/templatesRepository.ts | 11 +++-- registry/tests/auth.spec.ts | 47 ++++++++++++++++++- 4 files changed, 64 insertions(+), 11 deletions(-) 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'));