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

first go at an interface for auth plugins #120

Merged
merged 1 commit into from
Oct 13, 2015
Merged
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
50 changes: 50 additions & 0 deletions registry/domain/authentication.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
'use strict';

var path = require('path');
var strings = require('../../resources/');
var format = require('stringformat');

var builtin = {
basic: {
validate: function(authConfig){
var isValid = authConfig.username && authConfig.password;
return {
isValid: isValid,
message: isValid ? '' : strings.errors.registry.CONFIGURATION_PUBLISH_BASIC_AUTH_CREDENTIALS_MISSING
};
},
middleware: function(authConfig){
var express = require('express');
return express.basicAuth(authConfig.username, authConfig.password);
}
}
};

var scheme;

module.exports.validate = function(authConfig){
if(builtin[authConfig.type]){
scheme = builtin[authConfig.type];
}
else {
var cwd = process.cwd();
module.paths.push(cwd, path.join(cwd, 'node_modules'));
Copy link
Member

Choose a reason for hiding this comment

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

What is this for? Can't find any documentation about module.paths

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's equivalent to require.paths.push(). It basically adds the parent node_modules folder to the list of paths searched by require. So that you can require a module in the upper node_modules folder.

I nicked it from here: https://github.com/mochajs/mocha/blob/master/lib/mocha.js#L28

It's probably not the best way to do it. But I wasn't sure how to proceed in a world where npm3 might or might not flatten the modules folder.

Modifying the paths at runtime is bad, but I figured it's better than using ../../.

Copy link
Member

Choose a reason for hiding this comment

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

Ah! Ok, got it.

I think I'm more for a hapi-style initialisation in the way we did the plugin, so that you do the require on the initialisation and you solve this problem for free, but I'm ok with this too and we still can revisit later in case we have troubles with npm3.


var moduleName = 'oc-auth-' + authConfig.type;

try {
scheme = require(moduleName);
} catch(err){
return {
isValid: false,
message: format(strings.errors.registry.CONFIGURATION_PUBLISH_AUTH_MODULE_NOT_FOUND, moduleName)
};
}
}

return scheme.validate(authConfig);
};

module.exports.middleware = function(authConfig){
return scheme.middleware(authConfig);
};
5 changes: 3 additions & 2 deletions registry/domain/options-sanitiser.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ var _ = require('underscore');

var dependenciesResolver = require('./dependencies-resolver');
var settings = require('../../resources/settings');
var auth = require('./authentication');

module.exports = function(input){
var options = _.clone(input);

if(!options.publishAuth){
options.beforePublish = function(req, res, next){ next(); };
} else {
options.beforePublish = express.basicAuth(options.publishAuth.username, options.publishAuth.password);
options.beforePublish = auth.middleware(options.publishAuth);
}

if(!options.prefix){
Expand All @@ -28,4 +29,4 @@ module.exports = function(input){
}

return options;
};
};
16 changes: 7 additions & 9 deletions registry/domain/validators/registry-configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
var _ = require('underscore');

var strings = require('../../../resources');
var auth = require('../authentication');

module.exports = function(conf){

Expand All @@ -13,7 +14,7 @@ module.exports = function(conf){
response.message = message || 'registry configuration is not valid';
return response;
};

if(!conf || !_.isObject(conf) || _.keys(conf).length === 0){
return returnError(strings.errors.registry.CONFIGURATION_EMPTY);
}
Expand All @@ -23,7 +24,7 @@ module.exports = function(conf){
if(!!prefix){
if(prefix.substr(0, 1) !== '/'){
return returnError(strings.errors.registry.CONFIGURATION_PREFIX_DOES_NOT_START_WITH_SLASH);
}
}

if(prefix.substr(prefix.length - 1) !== '/'){
return returnError(strings.errors.registry.CONFIGURATION_PREFIX_DOES_NOT_END_WITH_SLASH);
Expand All @@ -33,12 +34,9 @@ module.exports = function(conf){
var publishAuth = conf.publishAuth;

if(!!publishAuth){
if(publishAuth.type !== 'basic'){
return returnError(strings.errors.registry.CONFIGURATION_PUBLISH_AUTH_NOT_SUPPORTED);
} else {
if(!publishAuth.username || !publishAuth.password){
return returnError(strings.errors.registry.CONFIGURATION_PUBLISH_AUTH_CREDENTIALS_MISSING);
}
var res = auth.validate(publishAuth);
if(!res.isValid){
return returnError(res.message);
}
}

Expand All @@ -65,4 +63,4 @@ module.exports = function(conf){
}

return response;
};
};
4 changes: 2 additions & 2 deletions resources/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ module.exports = {
CONFIGURATION_A_DEPENDENCY_NOT_FOUND: 'Registry configuration is not valid: a dependency is not valid.',
CONFIGURATION_EMPTY: 'Registry configuration is empty',
CONFIGURATION_ONREQUEST_MUST_BE_FUNCTION: 'Registry configuration is not valid: registry.on\'s callback must be a function',
CONFIGURATION_PUBLISH_AUTH_CREDENTIALS_MISSING: 'Registry configuration is not valid: basic auth requires username and password',
CONFIGURATION_PUBLISH_AUTH_NOT_SUPPORTED: 'Registry configuration is not valid: auth not supported',
CONFIGURATION_PUBLISH_BASIC_AUTH_CREDENTIALS_MISSING: 'Registry configuration is not valid: basic auth requires username and password',
CONFIGURATION_PUBLISH_AUTH_MODULE_NOT_FOUND: 'Registry configuration is not valid: module "{0}" not found',
CONFIGURATION_PREFIX_DOES_NOT_END_WITH_SLASH: 'Registry configuration is not valid: prefix should end with "/"',
CONFIGURATION_PREFIX_DOES_NOT_START_WITH_SLASH: 'Registry configuration is not valid: prefix should start with "/"',
CONFIGURATION_ROUTES_HANDLER_MUST_BE_FUNCTION: 'Registry configuration is not valid: handler should be a function',
Expand Down
4 changes: 2 additions & 2 deletions test/unit/registry-domain-validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ describe('registry : domain : validator', function(){

describe('when specified and not supported', function(){

var conf = { publishAuth: { type: 'oauth' }};
var conf = { publishAuth: { type: 'blarg' }};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
expect(validate(conf).message).to.equal('Registry configuration is not valid: auth not supported');
expect(validate(conf).message).to.equal('Registry configuration is not valid: module "oc-auth-blarg" not found');
});
});

Expand Down