JWT Authentication option

[NachoJusticia]

Hi, I would like to add JWT authentication because I think it would very interesting for this hapijs generator.
I have a hapijs backend in which I use the module hapi-auth-jwt2 and it is a plugin for the server object.
I have been able to:

• ask the user if he wants authentication.
• add an authKey that is needed for JWT in the three config scripts.
  but I don't know how to include the corresponding code of the authentication ONLY if the user has selected this option.

The code I would like to add is something like this:

const JWT = require('hapi-auth-jwt2');

const validation = (decoded, callback) => {

    if (!decoded.user.username) {
        callback(null, false); // JWT Validation failed
    }
    else {
        callback(null, true);  // JWT Validation ok
    }
};

module.exports = (server) =>

    new Promise((resolve, reject) => {

        server.register(JWT, (error) => {

            if (error) {
                reject(error);
            }
            else {
                let requireAuthenticationAllPlugins = true;

                if (process.env.NODE_ENV === 'development') {
                    requireAuthenticationAllPlugins = false;
                }
                server.auth.strategy('jwt', 'jwt', requireAuthenticationAllPlugins, {
                    key: Config.authKey,
                    verifyOptions: {
                        algorithms: ['HS256']
                    },
                    validateFunc: validation
                });

                server.register(plugins, (err) => {

                    if (err) {
                        reject(err);
                    }
                    else {
                        resolve();
                    }
                });
            }
        });
    });

Maybe I can put the plugin registration in an independent module like I show above, but another solution could be add it directly here:

Server.init(internals.manifest, internals.composeOptions, (err, server) => {

server.register(JWT, (error) => {

            if (error) {
                reject(error);
            }
            else {
                let requireAuthenticationAllPlugins = true;

                if (process.env.NODE_ENV === 'development') {
                    requireAuthenticationAllPlugins = false;
                }
                server.auth.strategy('jwt', 'jwt', requireAuthenticationAllPlugins, {
                    key: Config.authKey,
                    verifyOptions: {
                        algorithms: ['HS256']
                    },
                    validateFunc: validation
                });

                server.register(plugins, (err) => {

                    if (err) {
                        reject(err);
                    }
                    else {
                        resolve();
                    }
                });
            }
        });

    Hoek.assert(!err, err);
    server.log(process.env.npm_package_name + ' v' + process.env.npm_package_version + ' started at: ' + server.info.uri);
});

default, development and test JSON files:

{
  "server": {
    "port": <%- service.port %>,
    <% if(useAuthentication){ -%>
    "authKey": "H5SA7hdVmtwT0lpemh2KN7GGln1oOU99_change_for_your_jwt_authkey"
    <% } %>
  }
}

Could you please give me some guides about this problem?
Thanks :)

Perdona por el inglés, no sé porqué cuando me dí cuenta ya llevaba más de medio Pull Request 👍

[danielo515]

Hello @NachoJusticia

Let's first discuss about the inclusion of the JWT code. I would prefer to have them in separate files, not only because it will be more tidy and organized on the generated project, but also because it's easier to manage by the generator code. If most or all of the logic lives on separate files, all you have to do is to include or skip them based on a bolean. You have an example on the DockerFile inclusion:

https://github.com/danielo515/generator-hapi-swagger-es6/blob/master/generators/app/index.js#L97-L100

As you can see, if the user wants docker we copy the dockerfile, if the user does not want docker we do nothing. You can do the same for JWT if it lives on a separate plugin, which for me is also the preferred way to include stuff in a hapi server.

About the code you have posted.... Why are you returning a promise for the registration ? Does hapi accept a promise instead of a callback ? Do you have any link to the documentation ?

Thanks and regards

Está bien que lo hayas puesto en inglés, esto es un proyecto open-source y así más gente puede entenderlo y participar. Un saludo

[NachoJusticia]

Hi again @danielo515 and thank you for your answer.

Regarding the JWT code, I thought in put this code in separate files as I showed you in my previous comment. But I think that even doing this I don't get how to achieve the problem that I have to include this plugin to the hapi server object and I guess I should do it in this file.

I am returning a promise because I load a plugins module like this

LoadPlugins(server).then(() => {
     ...
}

This works fine.

[danielo515]

Hello @NachoJusticia

LoadPlugins(server).then(() => {
...
}
This works fine.

That is not how we register plugins on the generated project, we use glue.
I'm not saying that your approach is good or bad, but I want to be consistent on the style, and I want to register all the plugins through glue.

You are right about where to include the plugin registration, just add a conditional registration on the start.js file
Something like this

 registrations: [
....
       <% if(useJwt){ %>
        {
            plugin: './api/login'
        },
        {
            plugin: './api/logout'
        },
       {
            plugin: './yourJWTStuff'
        },
<% } %>
....
]

[NachoJusticia]

Ok @danielo515 , then I will try to be consistent with the plugin registration like you suggest, I imagine this won't be a problem for me.
I understand now how to add the new plugin by condition thanks to your example above.

Thank you!

[NachoJusticia]

Hi @danielo515
I think I have done a lot of progress with the JWT authentication.
You can see my changes in my fork

After fixing some bugs that my code had, I am able to launch the server and get no errors. The problem is that I am able to see only 1 endpoint in the hapi-swagger documentation site (the vision endpoint). Maybe it is a problem related with Glue...

Then, I would like you to have a look at my code and help me please! 👍

I'm registering the plugins like this:

registrations: [
...
               {
            plugin: './api/version'
        },
        {
            plugin: './api/healthcheck'
        },
        <% if(useAuthentication) { %>{
            plugin: './api/users/register.POST'
        },
        {
            plugin: './api/users/login.POST'
        },
        {
            plugin: './../authentication'
        }<% } %>
]

In addition, I have created a simple DAO module and I decorate the server when init:

Server.init(internals.manifest, internals.composeOptions, (err, server) => {

    Hoek.assert(!err, err);
    const URL = 'mongodb://localhost:27017';

    Mongo.connect(URL)
        .then((db) => {

            const _DAO = DAO(db);
            server.decorate('request', 'DAO', _DAO);
            server.log(process.env.npm_package_name + ' v' + process.env.npm_package_version + ' started at: ' + server.info.uri);
        })
        .catch((error) => {

            if (error) {
                throw (error);
            }
        });
});

Well I think it's not a good idea to put more parts of my code here, it should be easier for you to have a look at my code directly.

Thank!

[NachoJusticia]

Just one question:
why do you put return next() in version plugin but in the healthcheck you do just next()?
The same happends with the reply of the handler.

healthcheck.js

'use strict';

exports.register = function (server, options, next) {
    server.route({
        path: '/ops/healthcheck',
        method: 'GET',
        handler(request, reply) {
            reply({ message: 'ok' });
        }
    });
    next();
};
exports.register.attributes = {
    name: 'healthcheck'
};

version.js

'use strict';
const Joi = require('joi');
const internals = {
    response: {
        version: process.env.npm_package_version
    }
};
exports.register = (server, options, next) => {
    server.route({
        method: 'GET',
        path: '/ops/version',
        config: {
            description: 'Returns the version of the server',
            notes: 'Based on the package version',
            tags: ['meta', 'ops', 'api'],
            response: {
                status: {
                    200: Joi.object()
                }
            },
            handler(request, reply) {
                return reply(internals.response);
            },
        }
    });
    return next();
};
exports.register.attributes = {
    name: 'version'
};

[NachoJusticia]

Ok I have just noticed that I am missing this line:

            tags: ['meta', 'users', 'api'],

in the plugins that are not available in the /documentation site. I can continue working now!

[NachoJusticia]

I have added 3 endpoints:

• users/register to sign-in 👍
• users/login to sign-up 👍
• users/me to use authentication 👎 Not working yet. My idea with this endpoint is to accept an Authentication header and response with the user information of the corresponding token.

In the hapi-auth-jwt2 module they put a URL to a issue that they have with glue... https://www.npmjs.com/package/hapi-auth-jwt2

Then I have done what a user propose there, and it seems it works fine with glue now.
Anyhow, the users/me endpoint is not working because it doesn't allow me to use authentication header. In this endpoint, I am specifying that I want to use JWT authentication like this:

    server.route({
        method: 'GET',
        path: '/users/me',
        config: {
            auth: 'jwt',
            description: 'Get the current user information details - [ Authentication required ]',
            tags: ['meta', 'users', 'api'],
            handler(request, reply) {

                return reply(request.auth.credentials);
            }
        }
    });

In the following image you can see that the endpoint /users/me doesn't allow to send the Authorization header, that is my problem...

Now I post you an image of a similar backend (without Glue) that I have and in this case the /users/me endpoint works fine:

As you can see when an user logins he receive a token in the Authorization header, but I have also included it in the object response because this way the developer will notice easier that he can handle this token. Anyhow you can tell me if putting the token just in the Authentication header is enough.

Do you think I am missing something important here?
Thanks again.

[danielo515]

Hello @NachoJusticia

Thanks for your efforts. I have been checking your work and there are a couple of things that I want to check with you before we go any further. Most of them are style things.

1. The most noticeable is that you have not configured your editor to use lf instead of crlf linebreaks. Probably this is my fault because I have not configured it on the .editorconfig file, but this is importantl. Please configure your VSCode to use lf for line-breaks. You can fix this problem on the affected files just running dos2unix filename or dos2unix generators/**
2. You are working directly on your master branch. If you want to collaborate with anyone else this is not a good thing. It's easier to anyone that wants to check your work locally to just add your remote repository and checkout to your working branch. But if you use your master branch any potential collaborator has to pull your master branch, checkout to a new branch to avoid pushing to their master branch, come back to their master branch and clean it up. Please, work with branches, it is way easier and the best thing is that it's also easier to isolate the new additions.
3. You have some weird constructs on several templates. Seems that the ejs structures had been placed randomly. For example:

<% if(useAuthentication){ -%>"hapi-auth-jwt2": "^7.3.0",
    <%
    } %>

Why is that split on three lines when it can fit on just one ? And, why are you duplicating that for both hapi-auth-jwt2 and jsonwebtoken ? EJS is weird and annoying enough, try to use as less of it's syntax as possible.

{
  "server": {
    "port": <%- service.port %><% if(useAuthentication){ -%>,
    "authKey": "H5SA7hdVmtwT0lpemh2KN7GGln1oOU99"<%} %>
  }
}

What's the reasoning behind that ? Why are you merging two lines that have nothing to do with each other ? Put the useAuthentication on it's own line

There are several other problems. Please branch your work, and go ahead with a pull request, review will be far easier there.

Thanks and regards

[NachoJusticia]

Hi @danielo515

1. I executed the command that you suggested to me and it worked fine, but nothing has changed for git.
2. Ok!
3. It is just the opposite to a random placement. If you try to launch the generator you should see that the result of

<% if(useAuthentication){ -%>"hapi-auth-jwt2": "^7.3.0",
    <%
    } %>

is
"hapi-auth-jwt2": "^7.3.0", in line with the rest of the dependencies.

Anyhow you are right, I am checking the value of useAuthentication twice. Then I am going to write it as:

    <% if(useAuthentication){ -%>"hapi-auth-jwt2": "^7.3.0",
    "jsonwebtoken": "^7.1.9",
    <%
    } %>

3.2. The reasoning behind that

{
  "server": {
    "port": <%- service.port %><% if(useAuthentication){ -%>,
    "authKey": "H5SA7hdVmtwT0lpemh2KN7GGln1oOU99"<%} %>
  }
}

is that I only want to get

  "server": {
    "port": 3333
  }
}

when the user doesn't want authentication. Therefore, I put the , inside the if(useAuthentication).
Please launch my project in order to see if I am wrong with the things I am saying, because I don't think so.

Anyhow I am going to write here the result of some of the files when I select that I want authentication:
default.json:

{
  "server": {
    "port": 3000,
    "authKey": "H5SA7hdVmtwT0lpemh2KN7GGln1oOU99"
  }
}

package.json --> just the dependencies:

"dependencies": {
    "boom": "^4.3.1",
    "chalk": "^2.1.0",
    "getconfig": "^3.1.0",
    "glue": "^4.2.0",
    "good": "^7.3.0",
    "good-console": "^6.4.0",
    "good-squeeze": "^5.0.2",
    "hapi": "^16.6.2",
    "hapi-auth-jwt2": "^7.3.0",
    "jsonwebtoken": "^7.1.9",
    "hapi-swagger": "^7.8.1",
    "hoek": "5.0.0",
    "inert": "^4.2.1",
    "joi": "^11.1.1",
    "lout": "^10.0.3",
    "mongodb": "^2.2.31",
    "vision": "^4.1.1"
  },

[danielo515]

Hello again @NachoJusticia

We should prefer template maintainability over beautiful code output. The main reason is that it can become very hard to maintain and read certain template structures while editing the final code can be trivial, and probably some linter will take care of it.
That said, you are missing one EJS feature: slurp

So the following

 <% if(useAuthentication){ -%>"hapi-auth-jwt2": "^7.3.0",
    "jsonwebtoken": "^7.1.9",
    <%
    } %>

can be written as

 <% if(useAuthentication){ -%>
    "hapi-auth-jwt2": "^7.3.0",
    "jsonwebtoken": "^7.1.9",
    <% } -%>

And no difference on the output should happen. Have you tried the output of your templates before changing them ?

We should also agree on some EJS style-guides. For example, I always prefer <% } -%> instead of breaking it on several lines.

Again, thanks for your time and keep up the good work.

[NachoJusticia]

Hi @danielo515,
I think it's a good idea to prefer tamplate maintainability too, so I have just made the changes like you suggested me.

Tell me if there anything else you want to change.
I would like also you help me with the me endpoint, I am not able to use the Authorization header at this point. After searching this problem I think it could be a problem related to glue.

[danielo515]

As I said @NachoJusticia go ahead with the PR so we can better work together on the code.

Thanks and regards

[NachoJusticia]

@danielo515 I opened the PR 10 days ago

[danielo515]

Sorry @NachoJusticia I didn't noticed

[NachoJusticia]

Then, I think we can close this issue and continue in the PR, do you agree?