Skip to content

Commit

Permalink
resolving merge conflict with new 3.x features
Browse files Browse the repository at this point in the history
  • Loading branch information
jonwinton committed Aug 31, 2017
2 parents 293a71d + 9e55c18 commit e1605d3
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 32 deletions.
8 changes: 7 additions & 1 deletion lib/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const _ = require('lodash'),
flash = require('express-flash'),
responses = require('./responses'),
handlebars = require('handlebars'),
sites = require('./services/sites'),
basicAuth = require('basic-auth'),
fs = require('fs'),
path = require('path'),
Expand Down Expand Up @@ -104,9 +105,14 @@ function isAuthenticated(site) {
// try to authenticate with api key
passport.authenticate('apikey', { session: false})(req, res, next);
} else {
// We hae an issue where sites on subdomains of another clay site will not
// receive the correct site as an argument for this function, so we need
// to double-check
let possibleSite = sites.getSiteFromPrefix(`${site.host}${req.originalUrl}`);

req.session.returnTo = req.originalUrl; // redirect to this page after logging in
// otherwise redirect to login
res.redirect(`${getAuthUrl(site)}/login`);
res.redirect(`${getAuthUrl(possibleSite || site)}/login`);
}
};
};
Expand Down
20 changes: 20 additions & 0 deletions lib/auth.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const filename = __filename.split('/').pop().split('.').shift(),
sinon = require('sinon'),
passport = require('passport'),
winston = require('winston'),
sites = require('./services/sites'),
passportTwitter = require('passport-twitter'),
passportGoogle = require('passport-google-oauth'),
passportSlack = require('passport-slack'),
Expand All @@ -28,6 +29,7 @@ describe(_.startCase(filename), function () {
sandbox.stub(responses, 'unauthorized');
// ldap is called directly, so we can't stub it
sandbox.stub(passportAPIKey, 'Strategy');
sandbox.stub(sites);
sandbox.stub(db);
});

Expand Down Expand Up @@ -84,6 +86,24 @@ describe(_.startCase(filename), function () {
redirect: sandbox.spy()
};

sites.getSiteFromPrefix.returns({ host: 'domain.com', prefix: 'domain.com' });
fn({ path: '/' })(req, res); // never calls next(), but checks synchronously
expect(req.session.returnTo).to.equal(req.originalUrl);
expect(res.redirect.callCount).to.equal(1);
});

it('falls back to default site if site service returns undefined', function () {
var req = {
isAuthenticated: () => false,
get: () => false,
originalUrl: 'domain.com',
session: {}
},
res = {
redirect: sandbox.spy()
};

sites.getSiteFromPrefix.returns(undefined);
fn({ path: '/' })(req, res); // never calls next(), but checks synchronously
expect(req.session.returnTo).to.equal(req.originalUrl);
expect(res.redirect.callCount).to.equal(1);
Expand Down
32 changes: 17 additions & 15 deletions lib/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ function addSiteLocals(site) {
};
}

function addAssetDirectory(site) {
function addAssetDirectory(site, maxAge = 0) {
if (!files.fileExists(site.assetDir)) {
throw new Error('Asset directory does not exist: ' + site.assetDir);
}

return express.static(site.assetDir);
return express.static(site.assetDir, {
maxAge
});
}

/**
Expand Down Expand Up @@ -208,8 +210,8 @@ function addSiteController(router, site, providers) {
* @param {object} sessionStore
* @param {object} site
*/
function addSite(router, providers, sessionStore, site) {
site = _.defaults(site, { path: '/' });
function addSite(router, { providers, sessionStore, cacheControl = {} }, site) {
site = _.defaults(site, { path: '/' }); // If path isn't explicitly set, set it.
const path = site.path,
siteRouter = express.Router();

Expand All @@ -221,7 +223,7 @@ function addSite(router, providers, sessionStore, site) {
siteRouter.use(addUser);
siteRouter.use(addUri);
siteRouter.use(addSiteLocals(site));
siteRouter.use(addAssetDirectory(site));
siteRouter.use(addAssetDirectory(site, cacheControl.staticMaxAge));
siteRouter.use(addCORS(site));

// add query params to res.locals
Expand Down Expand Up @@ -313,12 +315,11 @@ function vhost(hostname, router) {
* @example addHost('www.example.com');
*/
function addHost(options) {
var sites = options.sites || [getDefaultSiteSettings(options.hostname)],
providers = options.providers || [];

var sites = options.sites || [getDefaultSiteSettings(options.hostname)]; // Explicit variable for mutation later down the line
const hostRouter = express.Router();

_.each(sites, addSite.bind(null, hostRouter, providers, options.sessionStore));
// _.each(sites, addSite.bind(null, hostRouter, providers, options.sessionStore));
_.each(sites, addSite.bind(null, hostRouter, options));

// once all sites are added, wrap them in a vhost
options.router.use(vhost(options.hostname, hostRouter));
Expand All @@ -333,7 +334,7 @@ function addHost(options) {
* var app = express();
* require('./routes)(app);
*/
function loadFromConfig(router, providers, sessionStore) {
function loadFromConfig(router, providers, sessionStore, cacheControl) {
const sitesMap = siteService.sites(),
siteHosts = _.uniq(_.map(sitesMap, 'host'));

Expand All @@ -342,11 +343,12 @@ function loadFromConfig(router, providers, sessionStore) {
const sites = _.filter(sitesMap, {host: hostname}).sort(sortByDepthOfPath);

addHost({
router: router,
hostname: hostname,
sites: sites,
providers: providers,
sessionStore: sessionStore
router,
hostname,
sites,
providers,
sessionStore,
cacheControl
});
});
return router;
Expand Down
4 changes: 2 additions & 2 deletions lib/routes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ describe(_.startCase(filename), function () {
files.tryRequire.returns(_.noop);
files.getFiles.returns(['a.b, c.d, e.f']);

fn(router, [], null, {slug: 'example', assetDir: 'example'});
fn(router, {providers: [], sessionStore: null}, {slug: 'example', assetDir: 'example'});

sinon.assert.calledWith(files.tryRequire, './routes/a');
sinon.assert.calledWith(files.tryRequire, './routes/a');
Expand All @@ -199,7 +199,7 @@ describe(_.startCase(filename), function () {
files.tryRequire.returns(_.noop);
files.getFiles.returns(['a.b, c.d, e.f']);

fn(router, ['foo'], null, siteStub);
fn(router, {providers: ['foo'], sessionStore: null}, siteStub);

sinon.assert.calledWith(auth.init, innerRouter, ['foo'], siteStub);
});
Expand Down
29 changes: 15 additions & 14 deletions lib/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const bluebird = require('bluebird'),
routes = require('./routes'),
bootstrap = require('./bootstrap'),
render = require('./render'),
plugins = require('./plugins');
amphoraPlugins = require('./plugins');

bluebird.config({
longStackTraces: true
Expand All @@ -21,24 +21,25 @@ bluebird.config({
* @param {object} [options.sessionStore]
* @returns {Promise}
*/
module.exports = function (options) {
let app, providers, router, sessionStore, renderers, appPlugins, env;

options = options || {};
app = options.app || express(); // use new express app if not passed in
providers = options.providers || [];
sessionStore = options.sessionStore;
renderers = options.renderers; // TODO: DOCUMENT THIS
appPlugins = options.plugins; // TODO: DOCUMENT THIS
env = options.env || []; // TODO: DOCUMENT THIS
module.exports = function (options = {}) {
let {
app = express(),
providers = [],
sessionStore,
renderers,
plugins = [],
env = [],
cacheControl = {}
} = options, router;
// TODO: DOCUMENT RENDERERS, ENV, PLUGINS, AND CACHE CONTROL

// Init plugins
if (appPlugins) {
plugins.registerPlugins(appPlugins);
if (plugins.length) {
amphoraPlugins.registerPlugins(plugins);
}

// init the router
router = routes(app, providers, sessionStore);
router = routes(app, providers, sessionStore, cacheControl);

// if engines were passed in, send them to the renderer
if (renderers) {
Expand Down
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
{
"name": "amphora",
<<<<<<< HEAD
"version": "4.0.0-rc1",
=======
"version": "3.6.0",
>>>>>>> master
"description": "An API mixin for Express that saves, publishes and composes data with the key-value store of your choice.",
"main": "index.js",
"scripts": {
Expand Down

0 comments on commit e1605d3

Please sign in to comment.