diff --git a/common/authorization.js b/common/authorization.js index 94a53b8..cdf4a26 100644 --- a/common/authorization.js +++ b/common/authorization.js @@ -1,5 +1,8 @@ export function isSecurityReviewer(username) { - // TODO: Externalize to configuration - const securityReviewers = ['octocat', 'monalisa']; - return securityReviewers.includes(username); + if (!process.env.SECURITY_REVIEWERS) { + throw new Error('SECURITY_REVIEWERS environment variable is not set'); + } + + const securityReviewers = process.env.SECURITY_REVIEWERS.split(',').map(r => r.trim().toLowerCase()); + return securityReviewers.includes(username.toLowerCase()); } diff --git a/test/common/authorization.test.js b/test/common/authorization.test.js new file mode 100644 index 0000000..0815e57 --- /dev/null +++ b/test/common/authorization.test.js @@ -0,0 +1,33 @@ +import assert from 'node:assert/strict'; +import { describe, test, beforeEach, afterEach } from 'node:test'; +import { isSecurityReviewer } from '../../common/authorization.js'; + +describe('isSecurityReviewer', () => { + let envBackup; + + beforeEach(() => { + envBackup = process.env.SECURITY_REVIEWERS; + }); + + afterEach(() => { + process.env.SECURITY_REVIEWERS = envBackup; + }); + + test('throws error when SECURITY_REVIEWERS is not set', () => { + delete process.env.SECURITY_REVIEWERS; + assert.throws(() => isSecurityReviewer('user1'), { + message: 'SECURITY_REVIEWERS environment variable is not set' + }); + }); + + test('returns true for a security reviewer', () => { + process.env.SECURITY_REVIEWERS = ' USER1, user2, user3 '; + assert.equal(isSecurityReviewer('user1'), true); + assert.equal(isSecurityReviewer('USER2'), true); + }); + + test('returns false for a non-security reviewer', () => { + process.env.SECURITY_REVIEWERS = ' USER1, user2, user3 '; + assert.equal(isSecurityReviewer('user4'), false); + }); +}); \ No newline at end of file