Should JWT secret be generated and versioned by default? #6238

DMehaffy · December 3, 2020, 5:07pm

This discussion has been migrated from our Github Discussion #6238

Hey guys!

Strapi does generate the JWT secret by default on create a project through Strapi’s starter. This secret is located at file extensions/users-permissions/config/jwt.json, which is versioned by default.

Although it’s possible to replace generated secret with an environment varible, this default behavior seems dangerous since Strapi users may end up commiting that secret and even deploying strapi to prod using that secret.

My suggestion is to standardize an environment variable instead of generating secret, so that strapi users have to set it by themselves.

What do you think about that?

DMehaffy · December 3, 2020, 5:10pm

Responses to the discussion on Github - Thread 1*

robertpiosik 200d ago

A very good catch. To avoid the installation process being more complicated, the secret could be maybe passed within cross-env, what do you think? It would be also commited though.

yagotome 200d ago

Author

Not sure! Where would it be?

If it’s only in develop script (not in start script), it would be much safer and development environment installation would keep easy, right?

robertpiosik 199d ago

In package.json, within develop & start scripts. One would then remove it from start script and pass the secret as a proper env variable in prod.

I think we should make some research how others handle this type of issue.

For instance, express-jwt doesn’t see any issue passing secret in code.

yagotome 199d ago

Author

In start script, it seems not to be a good ideia IMHO.

express-jwt doesn’t generate a secret silently in your source code, does it? It just shows an example of usage in docs, it isn’t an advice on how you should pass your secret.

What happens if someone accesses your source code and catches your jwt secret? They would be able to rewrite jwt payload and be authentic as any user. Right?

robertpiosik 199d ago

I was thinking about this issue and I come up with an idea of generating a gitignored file in the root dir which would contain the secret. It would be created upon server start if not present. @yagotome What do you think?

yagotome 199d ago

Author

Well, adding secret in a file ignored by git solves the problem! But how other collaborators would make their setup? If I git clone the project, I’d need to create this file, right? It’s OK for me if it’s documented.

robertpiosik 198d ago

The file would be auto generated if not present upon server start. The secret key equality is necessary in multi node setup where multiple servers are able to process requests made by authenticated once users otherwise it doesn’t matter so each collaborator could have different secrets in their setup.

yagotome 198d ago

Author

Great! Totally agree! So on deploy into a multi node server, one would only need to set their secret env so that all nodes in cluster have the same secret, right?

robertpiosik 198d ago

Almost! You would need to store the file along .env and copy it to the project’s working directory upon update. If you don’t do this and you keep creating the file every time your app gets update, you invalidate your users bearer tokes.

robertpiosik 198d ago

No solution is perfect

yagotome 197d ago

Author

Alright! But why not just lookup an env by default (such as JWT_SECRET) and if not set, use secret from uncommitted generated file? This seems neater for me. What about you?

robertpiosik 194d ago

What about storing the secret key it in the DB?

Uncommitted file would reflect the value stored in the database, value itself would be changeable in the admin dash for convenience.

yagotome 193d ago

Author

LGTM

DMehaffy · December 3, 2020, 5:11pm

Responses to the discussion on Github - Thread 2*

derrickmehaffy 195d ago

Collaborator

You can modify the jwt.json to parse a value from an environment variable (which is what I do in my own project): CAPIv2-Strapi/jwt.json at 0ac44cf612d714db2a4fd2afd151cd71f73de5e8 · canonn-science/CAPIv2-Strapi · GitHub

Likewise in the new stable release (and if you want to test, the rc.x releases) .env is supported out of the box, example on a WIP branch in which I am testing migrations: CAPIv2-Strapi/jwt.js at 59d6ce1c2761bc6c43b719eef12f8fefc3ba9e3b · canonn-science/CAPIv2-Strapi · GitHub

robertpiosik 193d ago

OP’s concern is about the default behaviour of the system which isn’t secure with its current jtw secret key storage implementation.

derrickmehaffy 192d ago

Collaborator

Nor is the database settings, there are certain parts of the application that are naturally insecure by default due to ease of use, adding these to the gitignore by default is going to confuse newer users when they go to deploy and suddenly stuff is broken.

As I already said in the rc.x releases and what will be the stable, it is secure by default:

dmehaffy:~/Development/test123/extensions/users-permissions/config$ cat jwt.js 
module.exports = {
  jwtSecret: process.env.JWT_SECRET || '353ab534-11ef-4fc3-8249-01f0a721e097'
};
dmehaffy:~/Development/test123/extensions/users-permissions/config$

(This was tested on rc.1)

The default will read the .env else fallback to a generated one