Skip to content

Commit

Permalink
Implement authentication support for Unleash UI.
Browse files Browse the repository at this point in the history
Closes: #261, #233, #232, #231
  • Loading branch information
ivaosthu committed Jan 16, 2018
1 parent 8eb0fdc commit 323320b
Show file tree
Hide file tree
Showing 20 changed files with 671 additions and 197 deletions.
3 changes: 3 additions & 0 deletions docs/securing-unleash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Securing Unleash

TODO: write about how to secure `/api/client` and `/api/admin`
5 changes: 5 additions & 0 deletions lib/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const unleashSession = require('./middleware/session');
const responseTime = require('./middleware/response-time');
const requestLogger = require('./middleware/request-logger');
const validator = require('./middleware/validator');
const simpleAuthentication = require('./middleware/simple-authentication');

module.exports = function(config) {
const app = express();
Expand Down Expand Up @@ -38,6 +39,10 @@ module.exports = function(config) {
app.use(baseUriPath, express.static(config.publicFolder));
}

if (config.adminAuthentication === 'unsecure') {
simpleAuthentication(app);
}

if (typeof config.preRouterHook === 'function') {
config.preRouterHook(app);
}
Expand Down
9 changes: 9 additions & 0 deletions lib/authentication-required.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict';

module.exports = class AuthenticationRequired {
constructor({ type, path, message }) {
this.type = type;
this.path = path;
this.message = message;
}
};
3 changes: 2 additions & 1 deletion lib/extract-user.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

function extractUsername(req) {
return req.cookies.username || 'unknown';
return req.user ? req.user.email : 'unknown';
}

module.exports = extractUsername;
48 changes: 48 additions & 0 deletions lib/middleware/simple-authentication.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
'use strict';

const User = require('../user');
const AuthenticationRequired = require('../authentication-required');

function unsecureAuthentication(app) {
app.post('/api/admin/login', (req, res) => {
const user = req.body;
req.session.user = new User({ email: user.email });
res
.status(200)
.json(req.session.user)
.end();
});

app.use('/api/admin/', (req, res, next) => {
if (req.session.user && req.session.user.email) {
req.user = req.session.user;
}
next();
});

app.use('/api/admin/', (req, res, next) => {
if (req.user) {
next();
} else {
return res
.status('401')
.json(
new AuthenticationRequired({
path: '/api/admin/login',
type: 'unsecure',
message:
'You have to indetify yourself in order to use Unleash.',
})
)
.end();
}
});

app.use((req, res, next) => {
// Updates active sessions every hour
req.session.nowInHours = Math.floor(Date.now() / 3600e3);
next();
});
}

module.exports = unsecureAuthentication;
1 change: 1 addition & 0 deletions lib/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const DEFAULT_OPTIONS = {
enableRequestLogger: isDev(),
secret: 'UNLEASH-SECRET',
sessionAge: THIRTY_DAYS,
adminAuthentication: 'unsecure',
};

module.exports = {
Expand Down
2 changes: 2 additions & 0 deletions lib/routes/admin-api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const featureArchive = require('./archive.js');
const events = require('./event.js');
const strategies = require('./strategy');
const metrics = require('./metrics');
const user = require('./user');

const apiDef = {
version: 2,
Expand All @@ -31,6 +32,7 @@ exports.router = config => {
router.use('/strategies', strategies.router(config));
router.use('/events', events.router(config));
router.use('/metrics', metrics.router(config));
router.use('/user', user.router(config));

return router;
};
20 changes: 20 additions & 0 deletions lib/routes/admin-api/user.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict';

const { Router } = require('express');

exports.router = function() {
const router = Router();

router.get('/', (req, res) => {
if (req.user) {
return res
.status(200)
.json(req.user)
.end();
} else {
return res.status(404).end();
}
});

return router;
};
4 changes: 4 additions & 0 deletions lib/server-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const getApp = require('./app');
const { startMonitoring } = require('./metrics');
const { createStores } = require('./db');
const { createOptions } = require('./options');
const User = require('./user');
const AuthenticationRequired = require('./authentication-required');

function createApp(options) {
// Database dependecies (statefull)
Expand Down Expand Up @@ -44,4 +46,6 @@ function start(opts) {

module.exports = {
start,
User,
AuthenticationRequired,
};
14 changes: 14 additions & 0 deletions lib/user.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';

const gravatar = require('gravatar');
const assert = require('assert');

module.exports = class User {
constructor({ name, email, imageUrl } = {}) {
assert(email, 'Email is required');
this.email = email;
this.name = name;
this.imageUrl =
imageUrl || gravatar.url(email, { s: '42', d: 'retro' });
}
};
28 changes: 28 additions & 0 deletions lib/user.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
'use strict';

const { test } = require('ava');
const User = require('./user');

test('should create user', t => {
const user = new User({ name: 'ole', email: '[email protected]' });
t.is(user.name, 'ole');
t.is(user.email, '[email protected]');
t.is(
user.imageUrl,
'//www.gravatar.com/avatar/d8ffeba65ee5baf57e4901690edc8e1b?s=42&d=retro'
);
});

test('should require email', t => {
const error = t.throws(() => {
const user = new User(); // eslint-disable-line
}, Error);

t.is(error.message, 'Email is required');
});

test('Should create user with only email defined', t => {
const user = new User({ email: '[email protected]' });

t.is(user.email, '[email protected]');
});
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
]
},
"dependencies": {
"assert": "^1.4.1",
"async": "^2.1.5",
"body-parser": "^1.18.2",
"commander": "^2.9.0",
Expand All @@ -67,12 +68,15 @@
"errorhandler": "^1.5.0",
"express": "^4.16.2",
"express-validator": "^4.3.0",
"gravatar": "^1.6.0",
"install": "^0.10.1",
"joi": "^13.0.1",
"knex": "^0.14.0",
"log4js": "^2.0.0",
"moment": "^2.19.3",
"parse-database-url": "^0.3.0",
"passport": "^0.4.0",
"passport-google-auth": "^1.0.2",
"pg": "^7.4.0",
"pkginfo": "^0.4.1",
"prom-client": "^10.0.4",
Expand Down
36 changes: 36 additions & 0 deletions test/e2e/api/admin/feature.auth.e2e.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict';

const { test } = require('ava');
const { setupAppWithAuth } = require('./../../helpers/test-helper');

test.serial('creates new feature toggle with createdBy', async t => {
t.plan(1);
const { request, destroy } = await setupAppWithAuth('feature_api_auth');
// Login
await request.post('/api/admin/login').send({
email: '[email protected]',
});

// create toggle
await request.post('/api/admin/features').send({
name: 'com.test.Username',
enabled: false,
strategies: [{ name: 'default' }],
});

await request
.get('/api/admin/events')
.expect(res => {
t.true(res.body.events[0].createdBy === '[email protected]');
})
.then(destroy);
});

test.serial('should require authenticated user', async t => {
t.plan(0);
const { request, destroy } = await setupAppWithAuth('feature_api_auth');
return request
.get('/api/admin/features')
.expect(401)
.then(destroy);
});
62 changes: 62 additions & 0 deletions test/e2e/api/admin/feature.custom-auth.e2e.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
'use strict';

const { test } = require('ava');
const { setupAppWithCustomAuth } = require('./../../helpers/test-helper');
const AuthenticationRequired = require('./../../../../lib/authentication-required');
const User = require('./../../../../lib/user');

test.serial('should require authenticated user', async t => {
t.plan(0);
const preHook = app => {
app.use('/api/admin/', (req, res) =>
res
.status('401')
.json(
new AuthenticationRequired({
path: '/api/admin/login',
type: 'custom',
message: `You have to identify yourself.`,
})
)
.end()
);
};
const { request, destroy } = await setupAppWithCustomAuth(
'feature_api_custom_auth',
preHook
);
return request
.get('/api/admin/features')
.expect(401)
.then(destroy);
});

test.serial('creates new feature toggle with createdBy', async t => {
t.plan(1);
const user = new User({ email: '[email protected]' });

const preHook = app => {
app.use('/api/admin/', (req, res, next) => {
req.user = user;
next();
});
};
const { request, destroy } = await setupAppWithCustomAuth(
'feature_api_custom_auth',
preHook
);

// create toggle
await request.post('/api/admin/features').send({
name: 'com.test.Username',
enabled: false,
strategies: [{ name: 'default' }],
});

await request
.get('/api/admin/events')
.expect(res => {
t.true(res.body.events[0].createdBy === user.email);
})
.then(destroy);
});
18 changes: 7 additions & 11 deletions test/e2e/api/admin/feature.e2e.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,18 @@ test.serial('creates new feature toggle', async t => {
.then(destroy);
});

test.serial('creates new feature toggle with createdBy', async t => {
test.serial('creates new feature toggle with createdBy unknown', async t => {
t.plan(1);
const { request, destroy } = await setupApp('feature_api_serial');
await request
.post('/api/admin/features')
.send({
name: 'com.test.Username',
enabled: false,
strategies: [{ name: 'default' }],
})
.set('Cookie', ['username=ivaosthu'])
.set('Content-Type', 'application/json');
await request.post('/api/admin/features').send({
name: 'com.test.Username',
enabled: false,
strategies: [{ name: 'default' }],
});
await request
.get('/api/admin/events')
.expect(res => {
t.true(res.body.events[0].createdBy === 'ivaosthu');
t.true(res.body.events[0].createdBy === 'unknown');
})
.then(destroy);
});
Expand Down
33 changes: 32 additions & 1 deletion test/e2e/api/client/feature.e2e.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,36 @@
'use strict';

const { test } = require('ava');
const { setupApp } = require('./../../helpers/test-helper');

test.todo('e2e client feature');
test.serial('returns three feature toggles', async t => {
const { request, destroy } = await setupApp('feature_api_client');
return request
.get('/api/client/features')
.expect('Content-Type', /json/)
.expect(200)
.expect(res => {
t.true(res.body.features.length === 3);
})
.then(destroy);
});

test.serial('gets a feature by name', async t => {
t.plan(0);
const { request, destroy } = await setupApp('feature_api_client');
return request
.get('/api/client/features/featureX')
.expect('Content-Type', /json/)
.expect(200)
.then(destroy);
});

test.serial('cant get feature that dose not exist', async t => {
t.plan(0);
const { request, destroy } = await setupApp('feature_api_client');
return request
.get('/api/client/features/myfeature')
.expect('Content-Type', /json/)
.expect(404)
.then(destroy);
});
Loading

0 comments on commit 323320b

Please sign in to comment.