Session handler overhaul - #125
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the session handler system to use the login server for authentication kickoffs instead of the provider proxy. The changes introduce encrypted session codes for secure client-agent communication, add comprehensive CLI commands for session management, migrate from standard log to structured logrus logging, and implement context-aware authentication flows with interrupt handling.
Key Changes:
- Introduced encrypted session codes (
CodeWrapper) to secure authentication flows between client/CLI and agent - Refactored
AwaitRefreshandAwaitProviderRefreshto acceptcontext.Contextfor cancellation support - Added new CLI subcommands:
sessions register,sessions list,sessions create,sessions remove,sessions refresh - Migrated all
logusage tologrusfor structured logging across the codebase
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/auth.go | Removed one-time code generation, added CodeWrapper for session validation with expiry |
| internal/models/session.go | Added Code field to SessionCreateRequest for verification |
| internal/models/common.go | Added ENCODED_SESSION_CODE constant for new encoding type |
| internal/sessions/manager.go | Updated AwaitRefresh/AwaitProviderRefresh signatures to accept context, replaced log with logrus |
| internal/sessions/manager_test.go | Comprehensive test suite for session manager functionality |
| internal/daemon/sessions.go | Added session code validation and decryption before session creation |
| internal/daemon/auth.go | Updated to pass session codes through auth flow, improved logging |
| internal/daemon/middleware.go | Enhanced cookie processing to check both local and session manager providers, fixed redirect loop |
| internal/daemon/server.go | Creates encrypted session codes for auth kickoff, migrated to logrus |
| internal/daemon/static/auth_callback.html | Refactored to use Alpine.js for state management, added manual registration UI |
| internal/daemon/static/auth.html | Updated auth links to include callback and code parameters |
| internal/common/secure.go | New cryptographically secure random string generator |
| internal/common/secure_test.go | Tests for secure random string generation |
| internal/common/context.go | New context helpers for interrupt signal handling |
| internal/common/requests.go | Refactored to support custom resty client injection |
| cmd/cli/sessions*.go | New session management subcommands with interactive UI |
| cmd/cli/login.go | Refactored to use new authKickStart with context support |
| cmd/cli/request.go | Updated to use context-aware session refresh |
| cmd/cli/main.go | Added automatic secure secret generation if not configured |
| cmd/cli/server.go, agent.go | Migrated to use common.NewInterruptChannel |
| internal/providers/github/rbac.go | Migrated log to logrus |
| internal/config/*.go | Migrated log to logrus, renamed variables for clarity |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This stops using the provider proxy for cli / agent auth kickoffs and instead deferes to the thand login server to kick off auth requests. This resolves the session TODOs