update demonstration code for better environment variable security

[srmullen]

Use dotenv rather than relying on Vite to load the environment variables. This will help prevent accidental exposure of secret keys.

[Dan6erbond]

Great work! I'll take this for a quick spin and merge after dealing with the Lockfiles conflict. ;)

[Dan6erbond]

So, just attempted a console.log() over all the environment variables. As you can see I augmented the types as well in my changes, and they're all being logged as undefined:

undefined undefined undefined undefined undefined undefined undefined undefined undefined

Are you sure this method works, if so, did you possibly miss something in the implementation?

[srmullen]

So I was just checking it out. All seems good from my end. I'm able to log the env variables successfully. Where are your env variables defined? dotenv expects them to be in the root directory of the node package, so sk-auth/app here.

I also updated this pull request for the Twitch Provider

[Dan6erbond]

Hmm. There's a lot of discrepancy in our results it seems. I double-checked that my .env file is under sk-auth/app and the variables are named correctly, it's still logging them all as undefined.

This is my folder structure:

[srmullen]

Hmm... And you're logging them after the dotenv.config() call?

[Dan6erbond]

Yep, these are the first modified lines of appAuth.ts:

import { config } from "dotenv";

config();

console.log(
  process.env.GOOGLE_OAUTH_CLIENT_ID,
  process.env.GOOGLE_OAUTH_CLIENT_SECRET,
  process.env.TWITCH_OAUTH_CLIENT_ID,
  process.env.TWITCH_OAUTH_CLIENT_SECRET,
  process.env.FACEBOOK_OAUTH_CLIENT_ID,
  process.env.FACEBOOK_OAUTH_CLIENT_SECRET,
  process.env.TWITTER_API_KEY,
  process.env.TWITTER_API_SECRET,
  process.env.REDDIT_API_KEY,
  process.env.REDDIT_API_SECRET,
  process.env.JWT_SECRET_KEY,
);

Edit: Interestingly enough, doing console.log(config()); does show all the parsed environment variables, so it seems to be an issue with the way they're accessed.

[srmullen]

Ok. I think you need to do it as process.env['GOOGLE_OAUTH_CLIENT_ID']. I'm not sure why that is. I think it might be an issue with SvelteKit.

[Dan6erbond]

Wow. Interesting, you're right, that worked. I'm not sure if this is much better than using Vite's built-in solution, though, since you lose type-safety with few benefits. It's a matter of opinion.

Vite says the environment variables are only leaked if they're accessed by client-side code, which may very well happen so we need to keep an eye on it.

I think I'll close this PR for now but include a note in the README.md that users need to be aware of this when setting up the authentication library.

[srmullen]

Yeah, ultimately it's up to users of the library to keep their keys secret. The Vite documentation specifically mentions not putting secrets into VITE_ prefixed vars, so I think it best to make sure that is honored. The only way I know of to do that at the moment is what I've proposed here, so users would likely have to do this anyways.

[Dan6erbond]

I agree with you there. It's important for this to be mentioned to users, but in order to keep the demo app simple, since it only runs on our local machines, I'd like to avoid unnecessary dependencies.