feat(auth): add security layers like login in dashboard and auth token for print jobs#7
feat(auth): add security layers like login in dashboard and auth token for print jobs#7
Conversation
…iables Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
…figuration and build tasks Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| http.SetCookie(w, &http.Cookie{ | ||
| Name: SessionCookieName, | ||
| Value: token, | ||
| Path: "/", | ||
| MaxAge: int(SessionDuration.Seconds()), | ||
| HttpOnly: true, | ||
| SameSite: http.SameSiteStrictMode, | ||
| }) |
Check warning
Code scanning / CodeQL
Cookie 'Secure' attribute is not set to true Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
In general, to fix this kind of issue you must explicitly set Secure: true on any cookie that carries sensitive information (such as session identifiers), ensuring it is only sent over HTTPS connections.
For this specific code, the best fix with minimal behavior change is:
- In
SetSessionCookie, addSecure: true,to thehttp.Cookieliteral so the session cookie is always marked secure. - For consistency and to avoid leaving residues of sensitive state, also mark the clearing cookie in
ClearSessionCookieasSecure: true. This ensures that when browsers clear the cookie, they match the same security attributes as when it was set (same name, path, and key flags likeSecureandHttpOnly).
No new imports or helpers are needed; we only modify the two existing cookie literals in internal/auth/auth.go at lines 143–150 and 156–163.
| @@ -147,6 +147,7 @@ | ||
| MaxAge: int(SessionDuration.Seconds()), | ||
| HttpOnly: true, | ||
| SameSite: http.SameSiteStrictMode, | ||
| Secure: true, | ||
| }) | ||
| return token | ||
| } | ||
| @@ -160,6 +161,7 @@ | ||
| MaxAge: -1, | ||
| HttpOnly: true, | ||
| SameSite: http.SameSiteStrictMode, | ||
| Secure: true, | ||
| }) | ||
| } | ||
|
|
| http.SetCookie(w, &http.Cookie{ | ||
| Name: SessionCookieName, | ||
| Value: "", | ||
| Path: "/", | ||
| MaxAge: -1, | ||
| HttpOnly: true, | ||
| SameSite: http.SameSiteStrictMode, | ||
| }) |
Check warning
Code scanning / CodeQL
Cookie 'Secure' attribute is not set to true Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
In general, to fix this issue you should explicitly set the Secure field of all sensitive cookies (especially session cookies) to true in their http.Cookie definitions. This ensures that browsers only send those cookies over HTTPS, preventing exposure over plaintext HTTP.
The best fix here is to modify both cookie-setting locations in internal/auth/auth.go:
- In
SetSessionCookie, addSecure: true,to thehttp.Cookieliteral so that newly created session cookies are marked secure. - In
ClearSessionCookie, also addSecure: true,so that when clearing the cookie the browser applies the delete operation to the secure cookie as well (and does not leave the original cookie intact).
No new imports or helper methods are required. The only changes are to the http.Cookie struct literals at lines 143–150 and 156–163 in internal/auth/auth.go.
| @@ -147,6 +147,7 @@ | ||
| MaxAge: int(SessionDuration.Seconds()), | ||
| HttpOnly: true, | ||
| SameSite: http.SameSiteStrictMode, | ||
| Secure: true, | ||
| }) | ||
| return token | ||
| } | ||
| @@ -160,6 +161,7 @@ | ||
| MaxAge: -1, | ||
| HttpOnly: true, | ||
| SameSite: http.SameSiteStrictMode, | ||
| Secure: true, | ||
| }) | ||
| } | ||
|
|
Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
There was a problem hiding this comment.
Pull request overview
This pull request introduces comprehensive security enhancements for the ticket-daemon service, adding authentication and authorization layers to protect both the dashboard UI and the WebSocket print job API. The PR also includes extensive CI/CD infrastructure, linting configurations, and documentation updates.
Changes:
- Adds session-based authentication for dashboard access with bcrypt password hashing and brute-force protection
- Implements token-based authentication for WebSocket print job submissions with rate limiting (30 jobs/minute)
- Introduces CI/CD workflows for automated testing, security scanning (CodeQL), PR automation, and scheduled status checks
- Migrates from nhooyr.io/websocket to github.com/coder/websocket library
- Improves file permissions for logs (0600) and directories (0750) for enhanced security
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/auth/auth.go | New auth package implementing session management, password validation, and login throttling |
| internal/server/rate_limit.go | New rate limiter for WebSocket print job submissions |
| internal/server/server.go | Integration of auth token validation and rate limiting for print jobs |
| internal/daemon/daemon.go | HTTP route handlers for login, logout, and protected dashboard with session validation |
| internal/config/config.go | Added build-time security variables (AuthToken, PasswordHashB64, ServiceName, ServerPort) |
| internal/daemon/logger.go | Improved file permissions (0600) and error handling |
| internal/assets/web/login.html | New login page UI |
| internal/assets/web/index.html | Template injection for auth token in meta tag |
| internal/assets/web/js/*.js | JavaScript updates to include auth token in WebSocket messages |
| go.mod/go.sum | Dependency updates: added github.com/coder/websocket and golang.org/x/crypto |
| .github/workflows/*.yml | New CI/CD workflows for testing, linting, security scanning, and PR automation |
| .golangci.yml | Comprehensive linting configuration |
| Taskfile.yml | Build automation with ldflags injection for credentials |
| README.md | Documentation updates for security features and build process |
| api/v1/*.md, *.json | API documentation updates for auth token requirement |
Comments suppressed due to low confidence (1)
internal/daemon/daemon.go:278
- Using r.RemoteAddr directly for IP tracking can be problematic when behind a proxy or load balancer, as it will show the proxy's IP rather than the actual client IP. Consider checking standard headers like X-Forwarded-For or X-Real-IP (while being careful about header spoofing). Additionally, r.RemoteAddr includes the port number (e.g., "192.168.1.1:12345"), which means the same IP with different ports will be treated as different clients. Extract only the IP portion using net.SplitHostPort.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
adcondev
force-pushed
the
feat/login
branch
2 times, most recently
from
bfc6d65 to
0919606
Compare
…patch Go modules Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
⚡ Benchmark ResultsTo add benchmarks, create functions starting with |
This pull request introduces a comprehensive set of improvements focused on project automation, code quality, security, and documentation. The main changes include the addition of CI/CD and security workflows, enhanced linting and testing configurations, improved PR automation, and updated documentation to clarify build and security practices.
CI/CD, Automation, and Security Workflows:
.github/workflows/ci.yml) for automated testing, linting, benchmarking, and build verification, including coverage reporting and artifact summaries..github/workflows/codeql.yml) with a custom configuration (.github/codeql-config.yml) to enhance code security scanning on push, PR, and schedule. [1] [2].github/workflows/pr-automation.yml) to automatically label PRs by size, check for merge conflicts, auto-assign authors, and comment on new PRs with next steps and guidelines..github/workflows/pr-status-check.yml) that summarizes Dependabot PRs, reviews needed, and auto-merge PRs, and creates maintenance issues if manual review is required.Code Quality and Testing Enhancements:
golangci-lintconfiguration (.golangci.yml) enabling multiple linters for bug detection, code style, performance, and security checks, and included integration test tags..github/pull_request_template.md) to standardize PR descriptions, testing strategies, and code quality checks.Documentation and Configuration Updates:
README.mdto clarify security practices, build-time variable injection, and logging paths. Added badges for CI, code coverage, and Go report card. [1] [2] [3] [4].env.examplefile for reference, documenting required environment variables for build-time configuration (not used at runtime).These changes collectively improve automation, enforce security and code quality, and make the project easier to maintain and contribute to.