Skip to content
This repository was archived by the owner on Apr 8, 2019. It is now read-only.

Assingment 4 #119

Merged
merged 2 commits into from
May 12, 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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
test:
@node node_modules/lab/bin/lab -a code
test-cov:
@node node_modules/lab/bin/lab -a code -t 100 -L
@node node_modules/lab/bin/lab -a code -t 100 -Lv --lint-errors-threshold 0 --lint-warnings-threshold 0

Choose a reason for hiding this comment

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

Are these extra flags really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Debatable but Imo they will help prevent that the majority of the comments
on pr's are about following the hapi style.

On Fri, 1 May 2015 18:13 Eran Hammer [email protected] wrote:

In Makefile
#119 (comment)
:

@@ -1,7 +1,7 @@
test:
@node node_modules/lab/bin/lab -a code
test-cov:

  • @node node_modules/lab/bin/lab -a code -t 100 -L
  • @node node_modules/lab/bin/lab -a code -t 100 -Lv --lint-errors-threshold 0 --lint-warnings-threshold 0

Are these extra flags really needed?


Reply to this email directly or view it on GitHub
https://github.com/hueniverse/hueniversity/pull/119/files#r29511501.

test-cov-html:
@node node_modules/lab/bin/lab -a code -r html -o coverage.html

Expand Down
3 changes: 2 additions & 1 deletion lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

var Hapi = require('hapi');
var Version = require('./version');
var Private = require('./private');


// Declare internals
Expand All @@ -14,7 +15,7 @@ exports.init = function (port, next) {
var server = new Hapi.Server();
server.connection({ port: port });

server.register(Version, function (err) {
server.register([Version, Private], function (err) {

if (err) {
return next(err);
Expand Down
49 changes: 49 additions & 0 deletions lib/private.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Load modules

var Basic = require('hapi-auth-basic');
var Users = require('./users.json');

// Declare internals

var internals = {};

internals.validateFunc = function (username, password, callback) {

var user = Users[username];
if (!user || user.password !== password) {
return callback(null, false);
}

return callback(null, true, user);
};

exports.register = function (server, options, next) {

server.register(Basic, function (err) {

if (err) {
return next(err);
}

server.auth.strategy('simple', 'basic', { validateFunc: internals.validateFunc });
server.route({
method: 'GET',
path: '/private',
config: {
auth: 'simple',
description: 'Returns a greeting message to the authenticated user',
handler: function (request, reply) {

var html = '<div>Hello ' + request.auth.credentials.username + '</div>';
return reply(html);
}
}
});

return next();
});
};

exports.register.attributes = {
name: 'Private'
};
10 changes: 10 additions & 0 deletions lib/users.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"Foo": {
"username": "Foo",

Choose a reason for hiding this comment

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

Safer to add the username programatically than repeat information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean not have the username and assume the key is?

"password": "mysupersecuredpassword"
},
"Bar": {
"username": "Bar",
"password": "mysupersecuredpassword2"
}
}
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@
"homepage": "https://github.com/hueniverse/hueniversity",
"dependencies": {
"hapi": "8.x.x",
"hapi-auth-basic": "2.x.x",
"hoek": "2.x.x"
},
"scripts": {
"test": "node node_modules/lab/bin/lab -a code -t 100 -L",
"test": "node node_modules/lab/bin/lab -a code -t 100 -Lv --lint-errors-threshold 0 --lint-warnings-threshold 0",
"start": "node lib/start.js"
},
"devDependencies": {
Expand Down
3 changes: 1 addition & 2 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ var Version = require('../lib/version');
// Test shortcuts

var lab = exports.lab = Lab.script();
var describe = lab.experiment;
var expect = Code.expect;
var it = lab.test;

Expand Down Expand Up @@ -50,7 +49,7 @@ it('handles register plugin errors', { parallel: false }, function (done) {
name: 'fake version'
};

Hueniversity.init(0, function (err, server) {
Hueniversity.init(0, function (err) {

Choose a reason for hiding this comment

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

Better to keep the callback function with all its actual arguments. Kind of self documenting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like a waste to pass around unused objects, the callback from the init function is self documenting enough imo, but you have a point

expect(err).to.exist();
expect(err.message).to.equal('register version failed');
Expand Down
97 changes: 97 additions & 0 deletions test/private.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// Load modules

var Code = require('code');
var Lab = require('lab');
var Hueniversity = require('../lib');
var Users = require('../lib/users.json');
var Basic = require('hapi-auth-basic');

// Declare internals

var internals = {};


// Test shortcuts

var lab = exports.lab = Lab.script();
var describe = lab.experiment;
var expect = Code.expect;
var it = lab.test;


describe('/private', function () {

it('returns a greeting for the authenticated user', function (done) {

Hueniversity.init(0, function (err, server) {

expect(err).to.not.exist();

var request = { method: 'GET', url: '/private', headers: { authorization: internals.header(Users.Foo.username, Users.Foo.password) } };
server.inject(request, function (res) {

expect(res.statusCode, 'Status code').to.equal(200);
expect(res.result, 'result').to.equal('<div>Hello Foo</div>');

server.stop(done);
});
});
});

it('returns error on wrong password', function (done) {

Hueniversity.init(0, function (err, server) {

expect(err).to.not.exist();

var request = { method: 'GET', url: '/private', headers: { authorization: internals.header(Users.Foo.username, '') } };
server.inject(request, function (res) {

expect(res.statusCode, 'Status code').to.equal(401);

server.stop(done);
});
});
});

it('returns error on failed auth', function (done) {

Hueniversity.init(0, function (err, server) {

expect(err).to.not.exist();

var request = { method: 'GET', url: '/private', headers: { authorization: internals.header('I do not exist', '') } };
server.inject(request, function (res) {

expect(res.statusCode, 'Status code').to.equal(401);

server.stop(done);
});
});
});

it('returns error on failed registering of auth', { parallel: false }, function (done) {

var orig = Basic.register;
Basic.register = function (plugin, options, next) {

Basic.register = orig;
return next(new Error('fail'));
};
Basic.register.attributes = {
name: 'fake hapi-auth-basic'
};

Hueniversity.init(0, function (err) {

expect(err).to.exist();

done();
});
});
});

internals.header = function (username, password) {

return 'Basic ' + (new Buffer(username + ':' + password, 'utf8')).toString('base64');
};
4 changes: 2 additions & 2 deletions test/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
var Code = require('code');
var Lab = require('lab');
var Pkg = require('../package.json');
var Server = require('../lib');
var Hueniversity = require('../lib');


// Test shortcuts
Expand All @@ -18,7 +18,7 @@ describe('/version', function () {

it('returns the version from package.json', function (done) {

Server.init(0, function (err, server) {
Hueniversity.init(0, function (err, server) {

expect(err).to.not.exist();

Expand Down