Conversation
samlown
commented
Jan 28, 2026
- Moves from using Enrollments to Session models that make it easier to handle authentication details when running embedded apps inside the Invopop console.
There was a problem hiding this comment.
Pull request overview
This PR introduces a recommended session-based approach for embedded applications to replace the previous enrollment-based authentication method. The changes provide better support for applications running inside the Invopop Console by introducing a new Session model that handles authentication tokens and expiration more gracefully.
Changes:
- Added
Sessionstruct and management methods in the invopop client library for opinionated session handling - Implemented session middleware and cookie-based session storage in echopop package
- Moved enrollment-related functions to a separate file and deprecated the old
AuthEnrollmentmiddleware - Added
TokenExpiresfield to theEnrollmentstruct to support token expiration tracking
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| invopop/access_session.go | New Session struct with authorization, token management, and serialization methods |
| pkg/echopop/session.go | Session middleware for loading/storing sessions from headers or cookies |
| pkg/echopop/service.go | Added session key configuration option and AuthToken helper function |
| pkg/echopop/enrollments.go | Moved enrollment authentication functions from echopop.go (deprecated old middleware) |
| pkg/echopop/echopop.go | Removed enrollment functions (relocated to enrollments.go) |
| invopop/invopop.go | Added WithAuthToken method and deprecated SetAuthToken |
| invopop/errors.go | Added ErrAccessDenied error constant |
| invopop/access_enrollments.go | Added TokenExpires field to Enrollment struct |
| invopop/access.go | Added NewSession and NewSessionWithToken factory methods |
| go.mod, go.sum | Added gorilla/sessions and labstack/echo-contrib dependencies |
💡 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 10 out of 11 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Try to extract the session from the cookie | ||
| err := extractSessionFromCookie(c, sess) | ||
| if err != nil { | ||
| return fmt.Errorf("extracting session from cookie: %w", err) |
There was a problem hiding this comment.
The session middleware returns an error when extracting session from cookie fails. However, for a non-authenticated session load, it might be more appropriate to log the error and continue with an empty session rather than failing the request. Consider whether cookie extraction failures should be treated as soft failures (especially for initial page loads where no session exists yet).
| return fmt.Errorf("extracting session from cookie: %w", err) | |
| // For non-authenticated loads, treat cookie extraction failures as soft: | |
| // log the problem and continue with an empty session instead of failing. | |
| c.Logger().Warnf("failed to extract session from cookie: %v", err) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>