mirego / accent

The first developer-oriented translation tool. True asynchronous flow between translators and your team.
https://www.accent.reviews
BSD 3-Clause "New" or "Revised" License
1.31k stars 99 forks source link

Missing info about some ENV keys. #52

Closed louim closed 6 years ago

louim commented 6 years ago

Hey!

I noticed while setting up a few ENV keys that the readme doesn't mention:

I can draft a PR if you like.

remi commented 6 years ago

I think WEBAPP_EMAIL_HOST could feed from this variable by default?

No, because usually we’ll have a different domain (or subdomain) to host the Web application than the one we use to host the API on.

I can draft a PR if you like.

No worries, I’m about to send one that’ll fix it 😄

simonprev commented 6 years ago

In our production instance which sends email, we don’t have a SECRET_KEY_BASE env var and it works. @louim where did it fails, in your case, without SECRET_KEY_BASE configured?

louim commented 6 years ago

@simonprev It didn't fail without the SECRET_KEY_BASE, but while looking in the codebase to find what was missing for the email images, I found that ENV key referenced but not shown in the readme.

I'm not used to how Phoenix does things, but my understanding is that it's used to sign and encrypt tokens and cookies (as it does in Rails). ~So having a nil SECRET_KEY_BASE would allow anyone to forge token (because they are signed with an empty key) and auth as the user they like.~

Digging in the code, I see that you're using randomly generated tokens, and they are stored instead of being signed/validated, so I guess that the above does not apply. I'll defer to your knowledge of the project as to if it's used somewhere or not.