Skip to content

Commit 79126ff

Browse files
coopernetescoopernetes
authored
fix: restore user configs being merged with defaults (finos#1129)
* fix: restore user configs being merged with defaults * fix: tests * fix: other tests
1 parent 80bc561 commit 79126ff

File tree

5 files changed

+58
-93
lines changed

5 files changed

+58
-93
lines changed

‎index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import yargs from 'yargs';
55
import { hideBin } from 'yargs/helpers';
66
import * as fs from 'fs';
77
import { configFile, setConfigFile, validate } from './src/config/file';
8+
import { initUserConfig } from './src/config';
89
import proxy from './src/proxy';
910
import service from './src/service';
1011

@@ -30,6 +31,7 @@ const argv = yargs(hideBin(process.argv))
3031
.parseSync();
3132

3233
setConfigFile(argv.c as string || "");
34+
initUserConfig();
3335

3436
if (argv.v) {
3537
if (!fs.existsSync(configFile)) {

‎src/config/index.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,15 @@ import {
1414
} from './types';
1515

1616
let _userSettings: UserSettings | null = null;
17-
if (existsSync(configFile)) {
18-
_userSettings = JSON.parse(readFileSync(configFile, 'utf-8'));
19-
}
17+
console.log(`_userSettings during import: ${_userSettings}`); // for debugging only
18+
19+
export const initUserConfig = () => {
20+
console.log(`Initializing user configuration from ${configFile}`); // for debugging only
21+
if (existsSync(configFile)) {
22+
_userSettings = JSON.parse(readFileSync(configFile, 'utf-8'));
23+
}
24+
};
25+
2026
let _authorisedList: AuthorisedRepo[] = defaultSettings.authorisedList;
2127
let _database: Database[] = defaultSettings.sink;
2228
let _authentication: Authentication[] = defaultSettings.authentication;

‎test/testActiveDirectoryAuth.test.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ describe('ActiveDirectory auth method', () => {
5050
fs: fsStub,
5151
});
5252

53+
// Initialize the user config after proxyquiring to load the stubbed config
54+
config.initUserConfig();
55+
5356
const { configure } = proxyquire('../src/service/passport/activeDirectory', {
5457
'./ldaphelper': ldapStub,
5558
'../../db': dbStub,

‎test/testAuthMethods.test.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ describe('auth methods', async () => {
3030
const config = proxyquire('../src/config', {
3131
fs: fsStub,
3232
});
33+
34+
// Initialize the user config after proxyquiring to load the stubbed config
35+
config.initUserConfig();
3336

3437
expect(() => config.getAuthMethods()).to.throw(Error, 'No authentication method enabled');
3538
});
@@ -52,6 +55,9 @@ describe('auth methods', async () => {
5255
fs: fsStub,
5356
});
5457

58+
// Initialize the user config after proxyquiring to load the stubbed config
59+
config.initUserConfig();
60+
5561
const authMethods = config.getAuthMethods();
5662
expect(authMethods).to.have.lengthOf(3);
5763
expect(authMethods[0].type).to.equal('local');

‎test/testConfig.test.js‎

Lines changed: 38 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ describe('default configuration', function () {
1111
it('should use default values if no user-settings.json file exists', function () {
1212
const config = require('../src/config');
1313
config.logConfiguration();
14-
const enabledMethods = defaultSettings.authentication.filter(method => method.enabled);
14+
const enabledMethods = defaultSettings.authentication.filter((method) => method.enabled);
1515

1616
expect(config.getAuthMethods()).to.deep.equal(enabledMethods);
1717
expect(config.getDatabase()).to.be.eql(defaultSettings.sink[0]);
@@ -41,26 +41,22 @@ describe('user configuration', function () {
4141

4242
beforeEach(function () {
4343
delete require.cache[require.resolve('../src/config/env')];
44+
delete require.cache[require.resolve('../src/config')];
4445
oldEnv = { ...process.env };
4546
tempDir = fs.mkdtempSync('gitproxy-test');
4647
tempUserFile = path.join(tempDir, 'test-settings.json');
47-
require('../src/config/file').configFile = tempUserFile;
48+
require('../src/config/file').setConfigFile(tempUserFile);
4849
});
4950

5051
it('should override default settings for authorisedList', function () {
5152
const user = {
52-
authorisedList: [
53-
{
54-
project: 'foo',
55-
name: 'bar',
56-
url: 'https://github.com/foo/bar.git',
57-
},
58-
],
53+
authorisedList: [{ project: 'foo', name: 'bar', url: 'https://github.com/foo/bar.git' }],
5954
};
6055
fs.writeFileSync(tempUserFile, JSON.stringify(user));
6156

6257
const config = require('../src/config');
63-
const enabledMethods = defaultSettings.authentication.filter(method => method.enabled);
58+
config.initUserConfig();
59+
const enabledMethods = defaultSettings.authentication.filter((method) => method.enabled);
6460

6561
expect(config.getAuthorisedList()).to.be.eql(user.authorisedList);
6662
expect(config.getAuthMethods()).to.deep.equal(enabledMethods);
@@ -69,19 +65,13 @@ describe('user configuration', function () {
6965
});
7066

7167
it('should override default settings for authentication', function () {
72-
const user = {
73-
authentication: [
74-
{
75-
type: 'google',
76-
enabled: true,
77-
},
78-
],
79-
};
68+
const user = { authentication: [{ type: 'google', enabled: true }] };
8069
fs.writeFileSync(tempUserFile, JSON.stringify(user));
8170

8271
const config = require('../src/config');
72+
config.initUserConfig();
8373
const authMethods = config.getAuthMethods();
84-
const googleAuth = authMethods.find(method => method.type === 'google');
74+
const googleAuth = authMethods.find((method) => method.type === 'google');
8575

8676
expect(googleAuth).to.not.be.undefined;
8777
expect(googleAuth.enabled).to.be.true;
@@ -92,18 +82,12 @@ describe('user configuration', function () {
9282
});
9383

9484
it('should override default settings for database', function () {
95-
const user = {
96-
sink: [
97-
{
98-
type: 'postgres',
99-
enabled: true,
100-
},
101-
],
102-
};
85+
const user = { sink: [{ type: 'postgres', enabled: true }] };
10386
fs.writeFileSync(tempUserFile, JSON.stringify(user));
10487

10588
const config = require('../src/config');
106-
const enabledMethods = defaultSettings.authentication.filter(method => method.enabled);
89+
config.initUserConfig();
90+
const enabledMethods = defaultSettings.authentication.filter((method) => method.enabled);
10791

10892
expect(config.getDatabase()).to.be.eql(user.sink[0]);
10993
expect(config.getDatabase()).to.not.be.eql(defaultSettings.sink[0]);
@@ -112,30 +96,22 @@ describe('user configuration', function () {
11296
});
11397

11498
it('should override default settings for SSL certificate', function () {
115-
const user = {
116-
tls: {
117-
key: 'my-key.pem',
118-
cert: 'my-cert.pem',
119-
},
120-
};
99+
const user = { tls: { key: 'my-key.pem', cert: 'my-cert.pem' } };
121100
fs.writeFileSync(tempUserFile, JSON.stringify(user));
122101

123102
const config = require('../src/config');
103+
config.initUserConfig();
124104

125105
expect(config.getTLSKeyPemPath()).to.be.eql(user.tls.key);
126106
expect(config.getTLSCertPemPath()).to.be.eql(user.tls.cert);
127107
});
128108

129109
it('should override default settings for rate limiting', function () {
130-
const limitConfig = {
131-
rateLimit: {
132-
windowMs: 60000,
133-
limit: 1500,
134-
},
135-
};
110+
const limitConfig = { rateLimit: { windowMs: 60000, limit: 1500 } };
136111
fs.writeFileSync(tempUserFile, JSON.stringify(limitConfig));
137112

138113
const config = require('../src/config');
114+
config.initUserConfig();
139115

140116
expect(config.getRateLimit().windowMs).to.be.eql(limitConfig.rateLimit.windowMs);
141117
expect(config.getRateLimit().limit).to.be.eql(limitConfig.rateLimit.limit);
@@ -145,68 +121,55 @@ describe('user configuration', function () {
145121
const user = {
146122
attestationConfig: {
147123
questions: [
148-
{
149-
label: 'Testing Label Change',
150-
tooltip: {
151-
text: 'Testing Tooltip Change',
152-
links: [],
153-
},
154-
},
124+
{ label: 'Testing Label Change', tooltip: { text: 'Testing Tooltip Change', links: [] } },
155125
],
156126
},
157127
};
158128
fs.writeFileSync(tempUserFile, JSON.stringify(user));
159129

160130
const config = require('../src/config');
131+
config.initUserConfig();
161132

162133
expect(config.getAttestationConfig()).to.be.eql(user.attestationConfig);
163134
});
164135

165136
it('should override default settings for url shortener', function () {
166-
const user = {
167-
urlShortener: 'https://url-shortener.com',
168-
};
137+
const user = { urlShortener: 'https://url-shortener.com' };
169138
fs.writeFileSync(tempUserFile, JSON.stringify(user));
170139

171140
const config = require('../src/config');
141+
config.initUserConfig();
172142

173143
expect(config.getURLShortener()).to.be.eql(user.urlShortener);
174144
});
175145

176146
it('should override default settings for contact email', function () {
177-
const user = {
178-
contactEmail: 'test@example.com',
179-
};
147+
const user = { contactEmail: 'test@example.com' };
180148
fs.writeFileSync(tempUserFile, JSON.stringify(user));
181149

182150
const config = require('../src/config');
151+
config.initUserConfig();
183152

184153
expect(config.getContactEmail()).to.be.eql(user.contactEmail);
185154
});
186155

187156
it('should override default settings for plugins', function () {
188-
const user = {
189-
plugins: ['plugin1', 'plugin2'],
190-
};
157+
const user = { plugins: ['plugin1', 'plugin2'] };
191158
fs.writeFileSync(tempUserFile, JSON.stringify(user));
192159

193160
const config = require('../src/config');
161+
config.initUserConfig();
194162

195163
expect(config.getPlugins()).to.be.eql(user.plugins);
196164
});
197165

198166
it('should override default settings for sslCertPemPath', function () {
199-
const user = {
200-
tls: {
201-
enabled: true,
202-
key: 'my-key.pem',
203-
cert: 'my-cert.pem',
204-
}
205-
};
167+
const user = { tls: { enabled: true, key: 'my-key.pem', cert: 'my-cert.pem' } };
206168

207169
fs.writeFileSync(tempUserFile, JSON.stringify(user));
208170

209171
const config = require('../src/config');
172+
config.initUserConfig();
210173

211174
expect(config.getTLSCertPemPath()).to.be.eql(user.tls.cert);
212175
expect(config.getTLSKeyPemPath()).to.be.eql(user.tls.key);
@@ -215,73 +178,58 @@ describe('user configuration', function () {
215178

216179
it('should prioritize tls.key and tls.cert over sslKeyPemPath and sslCertPemPath', function () {
217180
const user = {
218-
tls: {
219-
enabled: true,
220-
key: 'good-key.pem',
221-
cert: 'good-cert.pem',
222-
},
181+
tls: { enabled: true, key: 'good-key.pem', cert: 'good-cert.pem' },
223182
sslKeyPemPath: 'bad-key.pem',
224183
sslCertPemPath: 'bad-cert.pem',
225184
};
226185
fs.writeFileSync(tempUserFile, JSON.stringify(user));
227-
186+
228187
const config = require('../src/config');
188+
config.initUserConfig();
229189

230190
expect(config.getTLSCertPemPath()).to.be.eql(user.tls.cert);
231191
expect(config.getTLSKeyPemPath()).to.be.eql(user.tls.key);
232192
expect(config.getTLSEnabled()).to.be.eql(user.tls.enabled);
233193
});
234194

235195
it('should use sslKeyPemPath and sslCertPemPath if tls.key and tls.cert are not present', function () {
236-
const user = {
237-
sslKeyPemPath: 'good-key.pem',
238-
sslCertPemPath: 'good-cert.pem',
239-
};
196+
const user = { sslKeyPemPath: 'good-key.pem', sslCertPemPath: 'good-cert.pem' };
240197
fs.writeFileSync(tempUserFile, JSON.stringify(user));
241198

242199
const config = require('../src/config');
200+
config.initUserConfig();
243201

244202
expect(config.getTLSCertPemPath()).to.be.eql(user.sslCertPemPath);
245203
expect(config.getTLSKeyPemPath()).to.be.eql(user.sslKeyPemPath);
246204
expect(config.getTLSEnabled()).to.be.eql(false);
247205
});
248206

249207
it('should override default settings for api', function () {
250-
const user = {
251-
api: {
252-
gitlab: {
253-
baseUrl: 'https://gitlab.com',
254-
},
255-
},
256-
};
208+
const user = { api: { gitlab: { baseUrl: 'https://gitlab.com' } } };
257209
fs.writeFileSync(tempUserFile, JSON.stringify(user));
258-
210+
259211
const config = require('../src/config');
212+
config.initUserConfig();
260213

261214
expect(config.getAPIs()).to.be.eql(user.api);
262215
});
263216

264217
it('should override default settings for cookieSecret if env var is used', function () {
265218
fs.writeFileSync(tempUserFile, '{}');
266-
process.env.GIT_PROXY_COOKIE_SECRET = 'test-cookie-secret'
219+
process.env.GIT_PROXY_COOKIE_SECRET = 'test-cookie-secret';
267220

268221
const config = require('../src/config');
222+
config.initUserConfig();
269223
expect(config.getCookieSecret()).to.equal('test-cookie-secret');
270224
});
271225

272226
it('should override default settings for mongo connection string if env var is used', function () {
273-
const user = {
274-
sink: [
275-
{
276-
type: 'mongo',
277-
enabled: true,
278-
}
279-
]
280-
};
227+
const user = { sink: [{ type: 'mongo', enabled: true }] };
281228
fs.writeFileSync(tempUserFile, JSON.stringify(user));
282229
process.env.GIT_PROXY_MONGO_CONNECTION_STRING = 'mongodb://example.com:27017/test';
283230

284231
const config = require('../src/config');
232+
config.initUserConfig();
285233
expect(config.getDatabase().connectionString).to.equal('mongodb://example.com:27017/test');
286234
});
287235

0 commit comments

Comments
 (0)