-
Notifications
You must be signed in to change notification settings - Fork 79
Single User Auth #202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Single User Auth #202
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #202 +/- ##
==========================================
- Coverage 80.09% 78.64% -1.45%
==========================================
Files 44 49 +5
Lines 3381 3695 +314
Branches 425 483 +58
==========================================
+ Hits 2708 2906 +198
- Misses 665 780 +115
- Partials 8 9 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@benvinegar Will work on increasing test coverage, but would love your feedback on the implementation! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements single user authentication via a secret password for the Counterscale application. It adds login/logout functionality with session management and protects dashboard routes from unauthorized access.
- Adds authentication system with password-based login and session storage
- Protects all dashboard and resource routes with authentication middleware
- Updates UI to show login form and logout link based on authentication state
Reviewed Changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/server/app/lib/auth.ts | Core authentication functions for login, logout, and auth checks |
| packages/server/app/lib/session.ts | Session storage configuration using cookies |
| packages/server/app/routes/_index.tsx | Login form UI and authentication logic |
| packages/server/app/routes/logout.tsx | Logout route handler |
| packages/server/app/routes/dashboard.tsx | Added authentication requirement |
| packages/server/app/routes/resources.*.tsx | Added authentication requirement to all resource routes |
| packages/server/app/root.tsx | Added user state and logout link |
| packages/cli/src/install.ts | Added app password prompt during installation |
| vitest.config.ts | Added process isolation for tests |
| Various test files | Updated tests to mock authentication and handle new requirements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, there's two major things that need to change.
No plaintext passwords in CF secrets
We can't store plaintext passwords, even if it's stored in CF secret – it has to be salted + hashed so it is non-retrievable.
That means that when the user submits a password to log in, we must similarly salt + hash that and compare it to the stored/hashed password.
The "salt" is a deployment-specific value that needs to be generated during CLI installation/deployment. It could be randomly generated for simplicity. (This can be the same secret value we use for JWTs, re: section below.)
If the salt is randomly generated behind the scenes (user doesn't know it), and the CF secret got deleted or changed somehow, that would mean we'd lose our ability to validate user-submitted passwords going forward. But I think that's okay, because redeploying should fix it (we generate a new random salt).
No plaintext values in session cookie
We can't have the cookie be trivially inspectable or modifiable. Otherwise users could 1) examine the value and see a plaintext password, or 2) modify the length of their expiry by toggling values.
The solution for this is to use JSON Web Tokens (JWTs), which stores 1) that the user has an active, valid session and 2) the length of the session. These tokens are encrypted so they don't expose the data inside or let users modify that data.
I made a lot of progress JWTs in my branch. Take a look at createToken and verifyToken in app/lib/auth.ts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments.
|
Noting that I'll make a follow up PR adding a CLI command to roll/update the password |
|
@benvinegar I think this is ready to merge. I just added a small refactor to the cli to explicitly check if certain secrets exist which will allow existing deploys to be updated with a password by running the install command. I ran the CLI locally to update my production deployment and auth is working deployed after setting password with the CLI. FYI, I do want to fast follow with a new cli command for updating password/revoking existing JWTs, but don't want to bloat this PR more |
|
@stordahl Hey I've just been slammed, didn't get a chance to run it. But I see my asks have been addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, last thing:
Enter the password you will use to access the Counterscale Dashboard
We have to make setting a password optional. (You could do "leave blank for none" but I think it's better to have a Y/N step.)
This is pretty important. For example, I think my demo site will basically stop working if it needs a password to access.
|
So, I tested the choice to set a password or not. Looks great. First time – no password, great. Second time – prompt doesn't come back, no way to set a password ever again. I think I'd prefer we just prompt them everytime, because deploying is pretty infrequent. |
|
@benvinegar makes sense and is a trivial change! |
|
@benvinegar I think you observed that because you still had |
Overview
This PR introduces single user authentication for the dashboard. The intention is to allow a single password to be set during install, and then use that password to authenticate a user. The flow works as follows...
npx @counterscale/cli install, the CLI will prompt the user to optionally enable authentication to access the dashboard. If they choose "yes", they will be prompted to input their desired passwordCF_AUTH_ENABLEDCF_JWT_SECRETCF_PASSWORD_HASHMajor Changes
@counterscale/cliauthmodule that handles generating the hashed password andCF_JWT_SECRETauthmodule with theinstallcommandCF_JWT_SECRETandCF_PASSWORD_HASHfor use in.dev.varsfor local developmentcd packages/cli && pnpm run generate-secrets@counterscale/serverauthandsessionmodules that handle user login/logout including JWT creation/signing and verification via RR loaders/ actions