Skip to content

Commit

Permalink
refactor: Use config.get() for accessing config values
Browse files Browse the repository at this point in the history
* added @types/config to enforce use of config.get()
  • Loading branch information
jrassa committed Mar 29, 2024
1 parent 5a28ee9 commit 0535d89
Show file tree
Hide file tree
Showing 46 changed files with 617 additions and 448 deletions.
18 changes: 16 additions & 2 deletions config/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,14 @@ module.exports = {
publishProvider: './src/app/common/event/event-publish.provider',
socketProvider: './src/app/common/sockets/event-socket.provider',

notifications: {
enabled: false,
topic: 'notification.posted'
},

messages: {
topic: 'message.posted'
topic: 'message.posted',
expireSeconds: 2592000 // default to 30 days
},

pages: {
Expand Down Expand Up @@ -153,6 +159,10 @@ module.exports = {
}
},

orgLevelConfig: {
required: false
},

/**
* Session settings are required regardless of auth strategy
*/
Expand Down Expand Up @@ -335,6 +345,8 @@ module.exports = {

teams: {
implicitMembers: {
enabled: false,

/**
* 'roles' strategy matches user.externalRoles against team.requiresExternalRoles to determine implicit
* membership in team. User must have all of the specified roles to be granted access to team.
Expand Down Expand Up @@ -452,5 +464,7 @@ module.exports = {
{ level: 'LEVEL-2', prefix: '(L2)' },
{ level: 'LEVEL-3', prefix: '(L3)' }
]
}
},

userPreferences: {}
};
2 changes: 1 addition & 1 deletion config/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = {

// Configuration for outgoing mail server / service
mailer: {
provider: './src/app/core/email/providers/log-email.provider'
provider: './src/app/core/email/providers/noop-email.provider'
},

/**
Expand Down
11 changes: 11 additions & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
]
},
"dependencies": {
"@types/config": "^3.3.4",
"agenda": "^4.3.0",
"async": "2.6",
"chalk": "2",
Expand Down
40 changes: 24 additions & 16 deletions src/app/core/access-checker/access-checker.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,14 @@ describe('Access Checker Service:', () => {
*/
describe('Broken Access Checker', () => {
beforeEach(() => {
sandbox.stub(config.auth.accessChecker, 'provider').value({
file: 'src/app/core/access-checker/providers/failure-provider.service',
config: {}
});
const configGetStub = sandbox.stub(config, 'get');
configGetStub
.withArgs('auth.accessChecker.provider.file')
.returns(
'src/app/core/access-checker/providers/failure-provider.service'
);
configGetStub.withArgs('auth.accessChecker.provider.config').returns({});
configGetStub.callThrough();

// Need to clear cached provider from service to ensure proper test run.
accessChecker.provider = null;
Expand Down Expand Up @@ -169,10 +173,14 @@ describe('Access Checker Service:', () => {
*/
describe('Working Access Checker', () => {
beforeEach(() => {
sandbox.stub(config.auth.accessChecker, 'provider').value({
file: 'src/app/core/access-checker/providers/example.provider',
config: provider
});
const configGetStub = sandbox.stub(config, 'get');
configGetStub
.withArgs('auth.accessChecker.provider.file')
.returns('src/app/core/access-checker/providers/example.provider');
configGetStub
.withArgs('auth.accessChecker.provider.config')
.returns(provider);
configGetStub.callThrough();

// Need to clear cached provider from service to ensure proper test run.
accessChecker.provider = null;
Expand Down Expand Up @@ -245,8 +253,6 @@ describe('Access Checker Service:', () => {
*/
describe('Missing Access Checker Config', () => {
beforeEach(() => {
sandbox.stub(config.auth.accessChecker, 'provider').value({});

// Need to clear cached provider from service to ensure proper test run.
accessChecker.provider = null;
});
Expand All @@ -257,7 +263,7 @@ describe('Access Checker Service:', () => {
.get('notincache')
.should.be.rejectedWith(
new Error(
'Error retrieving entry from the access checker provider: Invalid accessChecker provider configuration.'
'Error retrieving entry from the access checker provider: Configuration property "auth.accessChecker.provider.file" is not defined'
)
);
});
Expand All @@ -266,12 +272,14 @@ describe('Access Checker Service:', () => {
/**
* Test functionality with missing access checker provider file
*/
describe('Missing Access Checker Config', () => {
describe('Invalid Access Checker Config', () => {
beforeEach(() => {
sandbox.stub(config.auth.accessChecker, 'provider').value({
file: 'invalid/path/to/provider',
config: {}
});
const configGetStub = sandbox.stub(config, 'get');
configGetStub
.withArgs('auth.accessChecker.provider.file')
.returns('invalid/path/to/provider');
configGetStub.withArgs('auth.accessChecker.provider.config').returns({});
configGetStub.callThrough();

// Need to clear cached provider from service to ensure proper test run.
accessChecker.provider = null;
Expand Down
8 changes: 0 additions & 8 deletions src/app/core/access-checker/access-checker.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,6 @@ class AccessCheckerService {
*/
async getProvider(): Promise<AccessCheckerProvider> {
if (!this.provider) {
if (!config.has('auth.accessChecker.provider.file')) {
logger.error(
'Invalid accessChecker provider configuration. No `provider.file` specified'
);
return Promise.reject(
new Error('Invalid accessChecker provider configuration.')
);
}
const providerFile = config.get<string>(
'auth.accessChecker.provider.file'
);
Expand Down
2 changes: 1 addition & 1 deletion src/app/core/audit/audit.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export const getCSV = (req, res) => {
req.exportQuery
) as FilterQuery<AuditDocument>;

const fileName = `${config.app.instanceName}-${exportConfig.type}.csv`;
const fileName = `${config.get('app.instanceName')}-${exportConfig.type}.csv`;

const columns = exportConfig.config.cols;

Expand Down
2 changes: 1 addition & 1 deletion src/app/core/config/config.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('Config Server Controller', () => {
it('should include apiDocs', () => {
const systemConfig = configController.getSystemConfig();
systemConfig.should.have.property('apiDocs');
systemConfig.apiDocs.enabled.should.be.a.Boolean();
(systemConfig.apiDocs as any).enabled.should.be.a.Boolean();
});
});
});
27 changes: 14 additions & 13 deletions src/app/core/config/config.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,28 @@ import pkg from '../../../../package.json';

export const getSystemConfig = () => {
const toReturn = {
auth: config.auth.strategy,
apiDocs: config.apiDocs,
app: config.app,
requiredRoles: config.auth.requiredRoles,
auth: config.get('auth.strategy'),
apiDocs: config.get('apiDocs'),
app: config.get('app'),
requiredRoles: config.get('auth.requiredRoles'),

version: pkg.version,
banner: config.banner,
copyright: config.copyright,
banner: config.get('banner'),
copyright: config.get('copyright'),

contactEmail: config.app.contactEmail,
contactEmail: config.get('app.contactEmail'),

feedback: config.feedback,
teams: config.teams,
feedback: config.get('feedback'),
teams: config.get('teams'),

userPreferences: config.userPreferences,
userPreferences: config.get('userPreferences'),

masqueradeEnabled:
config.auth.strategy === 'proxy-pki' && config.auth.masquerade === true,
masqueradeUserHeader: config.masqueradeUserHeader,
config.get('auth.strategy') === 'proxy-pki' &&
config.get('auth.masquerade') === true,
masqueradeUserHeader: config.get('masqueradeUserHeader'),

allowDeleteUser: config.allowDeleteUser
allowDeleteUser: config.get('allowDeleteUser')
};

return toReturn;
Expand Down
32 changes: 18 additions & 14 deletions src/app/core/email/email.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,23 +135,27 @@ describe('Email Service:', () => {
};

beforeEach(() => {
sandbox.stub(config, 'coreEmails').value({
default: {
header,
footer
}
const configGetStub = sandbox.stub(config, 'get');
configGetStub.withArgs('coreEmails.default').returns({
header,
footer
});
configGetStub.callThrough();
});

it('should build email content', async () => {
const expectedResult = `${header}
<p>Welcome to ${config.app.title}, ${user.name}!</p>
<p>Have a question? Take a look at our <a href="${config.app.helpUrl}">Help documentation</a>.</p>
<p>If you need to contact a member of our team, you can reach us at ${config.app.contactEmail}.</p>
<p>Welcome to ${config.get('app.title')}, ${user.name}!</p>
<p>Have a question? Take a look at our <a href="${config.get(
'app.helpUrl'
)}">Help documentation</a>.</p>
<p>If you need to contact a member of our team, you can reach us at ${config.get(
'app.contactEmail'
)}.</p>
<br/>
<br/>
<p>Thanks,</p>
<p>The ${config.app.title} Support Team</p><p></p>
<p>The ${config.get('app.title')} Support Team</p><p></p>
${footer}
`;

Expand Down Expand Up @@ -207,12 +211,12 @@ ${footer}
};

beforeEach(() => {
sandbox.stub(config, 'coreEmails').value({
default: {
header,
footer
}
const configGetStub = sandbox.stub(config, 'get');
configGetStub.withArgs('coreEmails.default').returns({
header,
footer
});
configGetStub.callThrough();
});

it('should return merged mail options', async () => {
Expand Down
37 changes: 22 additions & 15 deletions src/app/core/email/email.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ class EmailService {
// Set email header/footer
const data = _.merge(
{},
config.coreEmails.default,
config.get('coreEmails.default'),
{
app: config.app,
app: config.get('app'),
user: user
},
overrides
Expand All @@ -92,9 +92,9 @@ class EmailService {
buildEmailSubject(template, user, overrides = {}): string {
const data = _.merge(
{},
config.coreEmails.default,
config.get('coreEmails.default'),
{
app: config.app,
app: config.get('app'),
user: user
},
overrides
Expand All @@ -105,23 +105,24 @@ class EmailService {
async generateMailOptions(
user,
req,
emailConfig,
emailTemplateConfig,
emailContentData = {},
emailSubjectData = {},
mailOpts = {}
): Promise<MailOptions> {
if (user.toObject) {
user = user.toObject();
}
let emailContent, emailSubject;
let emailContent: string;
let emailSubject: string;
try {
emailContent = await this.buildEmailContent(
path.posix.resolve(emailConfig.templatePath),
path.posix.resolve(emailTemplateConfig.templatePath),
user,
emailContentData
);
emailSubject = this.buildEmailSubject(
emailConfig.subject,
emailTemplateConfig.subject,
user,
emailSubjectData
);
Expand All @@ -130,22 +131,28 @@ class EmailService {
return Promise.reject(error);
}

return _.merge({}, config.coreEmails.default, emailConfig, mailOpts, {
subject: emailSubject,
html: emailContent
});
return _.merge(
{},
config.get('coreEmails.default'),
emailTemplateConfig,
mailOpts,
{
subject: emailSubject,
html: emailContent
}
);
}

/**
* Initializes the provider only once. Use the getProvider() method
* to create and/or retrieve this singleton
*/
async getProvider(): Promise<EmailProvider> {
if (!this.provider && config.mailer?.provider) {
if (!this.provider && config.has('mailer.provider')) {
const { default: Provider } = await import(
path.posix.resolve(config.mailer.provider)
path.posix.resolve(config.get('mailer.provider'))
);
this.provider = new Provider(config.mailer.options);
this.provider = new Provider(config.get('mailer.options'));
}
return this.provider;
}
Expand Down
2 changes: 1 addition & 1 deletion src/app/core/email/providers/file-email.provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export default class implements EmailProvider {
* Mocks sending an email with the input mail options by logging each email to individual files
*/
async sendMail(mailOptions: MailOptions): Promise<void> {
const fileDirectory = config.mailer.options['outputDir'] ?? 'emails';
const fileDirectory = config.get<string>('mailer.options.outputDir');

// ensure directory exists
await fsPromises.mkdir(fileDirectory, { recursive: true });
Expand Down
Loading

0 comments on commit 0535d89

Please sign in to comment.