Skip to content

Commit

Permalink
fix(registry): fix openid identifier match (case-insensitive)
Browse files Browse the repository at this point in the history
  • Loading branch information
stas-nc committed Nov 8, 2023
1 parent 58738ae commit 4a539e1
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 85 deletions.
2 changes: 1 addition & 1 deletion e2e/spec/news.spec.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {

This comment has been minimized.

Copy link
@b1ff

b1ff Nov 8, 2023

Contributor

I wouldn't leave only or skip in master, at least without explaining comment above test

I.amOnPage(newsPage.url.main);
I.waitInUrl(newsPage.url.main, 10);
I.waitForElement(newsPage.newsSources, 10);
Expand Down
22 changes: 0 additions & 22 deletions ilc/package-lock.json

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

49 changes: 0 additions & 49 deletions registry/package-lock.json

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

2 changes: 1 addition & 1 deletion registry/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 8 additions & 4 deletions registry/server/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,10 +318,14 @@ export default async (app: Express, settingsService: SettingsService, config: an
};

async function getEntityWithCreds(provider: string, identifier: string, secret: string | null): Promise<User | null> {
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;
}
Expand Down
5 changes: 4 additions & 1 deletion registry/server/templates/services/resources/Resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 || {};
}

Expand Down
11 changes: 7 additions & 4 deletions registry/server/templates/services/templatesRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@ export async function readTemplateWithAllVersions(templateName: string) {
.select()
.from<LocalizedTemplate>(tables.templatesLocalized)
.where('templateName', templateName);
template.localizedVersions = localizedTemplates.reduce((acc, item) => {
acc[item.locale] = { content: item.content };
return acc;
}, {} as Record<string, object>);
template.localizedVersions = localizedTemplates.reduce(
(acc, item) => {
acc[item.locale] = { content: item.content };
return acc;
},
{} as Record<string, object>,
);

return template;
}
Expand Down
47 changes: 45 additions & 2 deletions registry/tests/auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

This comment has been minimized.

Copy link
@b1ff

b1ff Nov 8, 2023

Contributor

seems unused

import knex from 'knex';

const generateResp403 = (username: string) => ({
message: `Access denied. "${username}" has "readonly" access.`,
Expand Down Expand Up @@ -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');
});

Expand Down Expand Up @@ -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'));
Expand Down
2 changes: 1 addition & 1 deletion start_all.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
];
Expand Down

0 comments on commit 4a539e1

Please sign in to comment.