Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add cookie secret env var #755

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .env.development
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
VITE_API_URI=http://localhost:8080
VITE_SERVER_URI=http://localhost:8000
VITE_SERVER_URI=http://localhost:8000

COOKIE_SECRET="cookie secret"
4 changes: 3 additions & 1 deletion .env.production
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
VITE_API_URI=
VITE_API_URI=

COOKIE_SECRET=
1 change: 0 additions & 1 deletion config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"type": "object",
"properties": {
"proxyUrl": { "type": "string" },
"cookieSecret": { "type": "string" },
"sessionMaxAgeHours": { "type": "number" },
"api": {
"description": "Third party APIs",
Expand Down
1 change: 0 additions & 1 deletion proxy.config.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"proxyUrl": "https://github.com",
"cookieSecret": "cookie secret",
"sessionMaxAgeHours": 12,
"tempPassword": {
"sendEmail": false,
Expand Down
6 changes: 4 additions & 2 deletions scripts/doc-schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ try {
]).toString('utf-8');
console.log(genDocOutput);

const schemaDoc = readFileSync(`${tempdir}${sep}schema.md`, 'utf-8')
.replace(/# GitProxy configuration file/g, '# Schema Reference'); // https://github.com/finos/git-proxy/pull/327#discussion_r1377343213
const schemaDoc = readFileSync(`${tempdir}${sep}schema.md`, 'utf-8').replace(
/# GitProxy configuration file/g,
'# Schema Reference',
); // https://github.com/finos/git-proxy/pull/327#discussion_r1377343213
const docString = `---
title: Schema Reference
description: JSON schema reference documentation for GitProxy
Expand Down
6 changes: 5 additions & 1 deletion src/config/env.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
const { GIT_PROXY_SERVER_PORT = 8000, GIT_PROXY_HTTPS_SERVER_PORT = 8443, GIT_PROXY_UI_PORT = 8080 } = process.env;
const {
GIT_PROXY_SERVER_PORT = 8000,
GIT_PROXY_HTTPS_SERVER_PORT = 8443,
GIT_PROXY_UI_PORT = 8080,
} = process.env;

exports.Vars = { GIT_PROXY_SERVER_PORT, GIT_PROXY_HTTPS_SERVER_PORT, GIT_PROXY_UI_PORT };
8 changes: 4 additions & 4 deletions src/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ let _authentication = defaultSettings.authentication;
let _tempPassword = defaultSettings.tempPassword;
let _proxyUrl = defaultSettings.proxyUrl;
let _api = defaultSettings.api;
let _cookieSecret = defaultSettings.cookieSecret;
const _cookieSecret = process.env.COOKIE_SECRET;
let _sessionMaxAgeHours = defaultSettings.sessionMaxAgeHours;
let _sslKeyPath = defaultSettings.sslKeyPemPath;
let _sslCertPath = defaultSettings.sslCertPemPath;
Expand Down Expand Up @@ -100,8 +100,8 @@ const getAPIs = () => {
};

const getCookieSecret = () => {
if (_userSettings && _userSettings.cookieSecret) {
_cookieSecret = _userSettings.cookieSecret;
if (!_cookieSecret) {
throw new Error('COOKIE_SECRET is not defined in the environment variables.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this never trigger because of the fallback to 'cookie secret'?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sorry about that, I need to correct it.

}
return _cookieSecret;
};
Expand Down Expand Up @@ -167,7 +167,7 @@ const getPlugins = () => {
_plugins = _userSettings.plugins;
}
return _plugins;
}
};

const getSSLKeyPath = () => {
if (_userSettings && _userSettings.sslKeyPemPath) {
Expand Down
74 changes: 39 additions & 35 deletions src/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ function isCompatiblePlugin(obj, propertyName = 'isGitProxyPlugin') {
// valid plugin objects will have the appropriate property set to true
// if the prototype chain is exhausted, return false
while (obj != null) {
if (Object.prototype.hasOwnProperty.call(obj, propertyName) &&
if (
Object.prototype.hasOwnProperty.call(obj, propertyName) &&
obj.isGitProxyPlugin &&
Object.keys(obj).includes('exec')) {
Object.keys(obj).includes('exec')
) {
return true;
}
obj = Object.getPrototypeOf(obj);
Expand All @@ -34,7 +36,7 @@ function isCompatiblePlugin(obj, propertyName = 'isGitProxyPlugin') {
class PluginLoader {
constructor(targets) {
/**
* List of Node module specifiers to load as plugins. It can be a relative path, an
* List of Node module specifiers to load as plugins. It can be a relative path, an
* absolute path, or a module name (which can include scoped packages like '@bar/baz').
* @type {string[]}
* @public
Expand Down Expand Up @@ -64,42 +66,42 @@ class PluginLoader {
*/
async load() {
try {
const modulePromises = this.targets.map(target =>
this._loadPluginModule(target).catch(error => {
const modulePromises = this.targets.map((target) =>
this._loadPluginModule(target).catch((error) => {
console.error(`Failed to load plugin: ${error}`); // TODO: log.error()
return Promise.reject(error); // Or return an error object to handle it later
})
}),
);

const moduleResults = await Promise.allSettled(modulePromises);
const loadedModules = moduleResults
.filter(result => result.status === 'fulfilled' && result.value !== null)
.map(result => result.value);
.filter((result) => result.status === 'fulfilled' && result.value !== null)
.map((result) => result.value);

console.log(`Found ${loadedModules.length} plugin modules`); // TODO: log.debug()

const pluginTypeResultPromises = loadedModules.map(mod =>
this._getPluginObjects(mod).catch(error => {
const pluginTypeResultPromises = loadedModules.map((mod) =>
this._getPluginObjects(mod).catch((error) => {
console.error(`Failed to cast plugin objects: ${error}`); // TODO: log.error()
return Promise.reject(error); // Or return an error object to handle it later
})
}),
);

const settledPluginTypeResults = await Promise.allSettled(pluginTypeResultPromises);
/**
* @type {PluginTypeResult[]} List of resolved PluginTypeResult objects
*/
const pluginTypeResults = settledPluginTypeResults
.filter(result => result.status === 'fulfilled' && result.value !== null)
.map(result => result.value);
.filter((result) => result.status === 'fulfilled' && result.value !== null)
.map((result) => result.value);

for (const result of pluginTypeResults) {
this.pushPlugins.push(...result.pushAction)
this.pullPlugins.push(...result.pullAction)
this.pushPlugins.push(...result.pushAction);
this.pullPlugins.push(...result.pullAction);
}

const combinedPlugins = [...this.pushPlugins, ...this.pullPlugins];
combinedPlugins.forEach(plugin => {
combinedPlugins.forEach((plugin) => {
console.log(`Loaded plugin: ${plugin.constructor.name}`);
});
} catch (error) {
Expand Down Expand Up @@ -137,15 +139,17 @@ class PluginLoader {
console.log('found pull plugin', potentialModule.constructor.name);
plugins.pullAction.push(potentialModule);
} else {
console.error(`Error: Object ${potentialModule.constructor.name} does not seem to be a compatible plugin type`);
console.error(
`Error: Object ${potentialModule.constructor.name} does not seem to be a compatible plugin type`,
);
}
}

// handles the default export case
// `module.exports = new ProxyPlugin()` in CJS or `exports default new ProxyPlugin()` in ESM
// the "module" is a single object that could be a plugin
if (isCompatiblePlugin(pluginModule)) {
handlePlugin(pluginModule)
handlePlugin(pluginModule);
} else {
// handle the typical case of a module which exports multiple objects
// module.exports = { x, y } (CJS) or multiple `export ...` statements (ESM)
Expand Down Expand Up @@ -173,20 +177,20 @@ class ProxyPlugin {
* A plugin which executes a function when receiving a git push request.
*/
class PushActionPlugin extends ProxyPlugin {
/**
* Wrapper class which contains at least one function executed as part of the action chain for git push operations.
* The function must be called `exec` and take in two parameters: an Express Request (req) and the current Action
* executed in the chain (action). This function should return a Promise that resolves to an Action.
*
* Optionally, child classes which extend this can simply define the `exec` function as their own property.
* This is the preferred implementation when a custom plugin (subclass) has its own state or additional methods
* that are required.
*
* @param {function} exec - A function that:
* - Takes in an Express Request object as the first parameter (`req`).
* - Takes in an Action object as the second parameter (`action`).
* - Returns a Promise that resolves to an Action.
*/
/**
* Wrapper class which contains at least one function executed as part of the action chain for git push operations.
* The function must be called `exec` and take in two parameters: an Express Request (req) and the current Action
* executed in the chain (action). This function should return a Promise that resolves to an Action.
*
* Optionally, child classes which extend this can simply define the `exec` function as their own property.
* This is the preferred implementation when a custom plugin (subclass) has its own state or additional methods
* that are required.
*
* @param {function} exec - A function that:
* - Takes in an Express Request object as the first parameter (`req`).
* - Takes in an Action object as the second parameter (`action`).
* - Returns a Promise that resolves to an Action.
*/
constructor(exec) {
super();
this.isGitProxyPushActionPlugin = true;
Expand All @@ -202,11 +206,11 @@ class PullActionPlugin extends ProxyPlugin {
* Wrapper class which contains at least one function executed as part of the action chain for git pull operations.
* The function must be called `exec` and take in two parameters: an Express Request (req) and the current Action
* executed in the chain (action). This function should return a Promise that resolves to an Action.
*
*
* Optionally, child classes which extend this can simply define the `exec` function as their own property.
* This is the preferred implementation when a custom plugin (subclass) has its own state or additional methods
* that are required.
*
*
* @param {function} exec - A function that:
* - Takes in an Express Request object as the first parameter (`req`).
* - Takes in an Action object as the second parameter (`action`).
Expand All @@ -224,4 +228,4 @@ module.exports = {
PushActionPlugin,
PullActionPlugin,
isCompatiblePlugin,
}
};
2 changes: 1 addition & 1 deletion src/proxy/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ const { Step } = require('./Step');
module.exports = {
Action,
Step,
}
};
16 changes: 8 additions & 8 deletions src/proxy/index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const proxyApp = require('express')();
const bodyParser = require('body-parser');
const http = require("http");
const https = require("https");
const http = require('http');
const https = require('https');
const fs = require('fs');
const path = require("path");
const path = require('path');
const router = require('./routes').router;
const config = require('../config');
const db = require('../db');
Expand All @@ -17,18 +17,18 @@ const options = {
limit: '100000kb',
type: '*/*',
key: fs.readFileSync(path.join(__dirname, config.getSSLKeyPath())),
cert: fs.readFileSync(path.join(__dirname, config.getSSLCertPath()))
cert: fs.readFileSync(path.join(__dirname, config.getSSLCertPath())),
};

// Setup the proxy middleware
proxyApp.use(bodyParser.raw(options));
proxyApp.use('/', router);

const start = async () => {
const plugins = config.getPlugins();
const pluginLoader = new PluginLoader(plugins);
await pluginLoader.load();
chain.chainPluginLoader = pluginLoader;
const plugins = config.getPlugins();
const pluginLoader = new PluginLoader(plugins);
await pluginLoader.load();
chain.chainPluginLoader = pluginLoader;
// Check to see if the default repos are in the repo list
const defaultAuthorisedRepoList = config.getAuthorisedList();
const allowedList = await db.getRepos();
Expand Down
14 changes: 10 additions & 4 deletions src/ui/views/Login/Login.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export default function UserProfile() {
width={'150px'}
src={logo}
alt='logo'
data-test ="git-proxy-logo"
data-test='git-proxy-logo'
/>
</div>
</CardHeader>
Expand All @@ -119,7 +119,7 @@ export default function UserProfile() {
value={username}
onChange={(e) => setUsername(e.target.value)}
autoFocus={true}
data-test ='username'
data-test='username'
/>
</FormControl>
</GridItem>
Expand All @@ -133,15 +133,21 @@ export default function UserProfile() {
type='password'
value={password}
onChange={(e) => setPassword(e.target.value)}
data-test ='password'
data-test='password'
/>
</FormControl>
</GridItem>
</GridContainer>
</CardBody>
<CardFooter>
{!isLoading ? (
<Button color='success' block disabled={!validateForm()} type='submit' data-test="login">
<Button
color='success'
block
disabled={!validateForm()}
type='submit'
data-test='login'
>
Login
</Button>
) : (
Expand Down
16 changes: 12 additions & 4 deletions test/chain.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('proxy chain', function () {
// Re-require the chain module after stubbing processors
chain = require('../src/proxy/chain');

chain.chainPluginLoader = new PluginLoader([])
chain.chainPluginLoader = new PluginLoader([]);
});

afterEach(() => {
Expand Down Expand Up @@ -108,7 +108,11 @@ describe('proxy chain', function () {
mockPushProcessors.checkUserPushPermission.resolves(continuingAction);

// this stops the chain from further execution
mockPushProcessors.checkIfWaitingAuth.resolves({ type: 'push', continue: () => false, allowPush: false });
mockPushProcessors.checkIfWaitingAuth.resolves({
type: 'push',
continue: () => false,
allowPush: false,
});
const result = await chain.executeChain(req);

expect(mockPreProcessors.parseAction.called).to.be.true;
Expand Down Expand Up @@ -136,7 +140,11 @@ describe('proxy chain', function () {
mockPushProcessors.checkAuthorEmails.resolves(continuingAction);
mockPushProcessors.checkUserPushPermission.resolves(continuingAction);
// this stops the chain from further execution
mockPushProcessors.checkIfWaitingAuth.resolves({ type: 'push', continue: () => true, allowPush: true });
mockPushProcessors.checkIfWaitingAuth.resolves({
type: 'push',
continue: () => true,
allowPush: true,
});
const result = await chain.executeChain(req);

expect(mockPreProcessors.parseAction.called).to.be.true;
Expand Down Expand Up @@ -232,5 +240,5 @@ describe('proxy chain', function () {
expect(mockPushProcessors.checkRepoInAuthorisedList.called).to.be.false;
expect(mockPushProcessors.parsePush.called).to.be.false;
expect(result).to.deep.equal(action);
})
});
});
2 changes: 1 addition & 1 deletion test/fixtures/baz.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module.exports = {
foo: 'bar',
baz: {},
}
};
2 changes: 1 addition & 1 deletion test/fixtures/proxy.config.invalid-1.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
"url": "https://www.github.com/finos/git-proxy.git"
}
]
}
}
2 changes: 1 addition & 1 deletion test/fixtures/proxy.config.invalid-2.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
"link": "https://www.github.com/finos/git-proxy"
}
]
}
}
2 changes: 1 addition & 1 deletion test/fixtures/proxy.config.valid-1.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{}
{}
2 changes: 1 addition & 1 deletion test/fixtures/proxy.config.valid-2.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
"url": "[email protected]:finos/git-proxy-test.git"
}
]
}
}
Loading
Loading