shared authentication for easee chargers#29502
Draft
tolot27 wants to merge 6 commits into
Draft
Conversation
tolot27
commented
Apr 30, 2026
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
plugin/auth/easee_test.gothe tests callnewEaseeFromConfig(t.Context(), ...), but*testing.Tdoes not provide aContext()method in the standard library; consider switching tocontext.Background()(or a helper that returns a context) so the tests compile cleanly. - The new
(*tokenSource).TokenSourceWithInitialmethod incharger/easee/identity.goappears unused; if it's not part of an immediately upcoming change, consider removing it to avoid dead code and keep the identity API minimal. - In
plugin/auth/easee.gothere is a commented-out import line forgithub.com/evcc-io/evcc/api; since the package is already imported normally, you can safely delete the commented line to keep the imports tidy.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `plugin/auth/easee_test.go` the tests call `newEaseeFromConfig(t.Context(), ...)`, but `*testing.T` does not provide a `Context()` method in the standard library; consider switching to `context.Background()` (or a helper that returns a context) so the tests compile cleanly.
- The new `(*tokenSource).TokenSourceWithInitial` method in `charger/easee/identity.go` appears unused; if it's not part of an immediately upcoming change, consider removing it to avoid dead code and keep the identity API minimal.
- In `plugin/auth/easee.go` there is a commented-out import line for `github.com/evcc-io/evcc/api`; since the package is already imported normally, you can safely delete the commented line to keep the imports tidy.
## Individual Comments
### Comment 1
<location path="plugin/auth/easee.go" line_range="42-44" />
<code_context>
+}
+
+// easeeSubject derives a stable settings DB key from the user email.
+func easeeSubject(user string) string {
+ h := sha256.Sum256([]byte(user))
+ return "easee-" + hex.EncodeToString(h[:])[:8]
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Keying the shared token source only by user may cause issues when the password changes.
Because easeeSubject is derived only from the user string, NewEaseeTokenSource will reuse an in-memory TokenSource even after that user’s password changes. If a refresh triggers a re-login, it will keep using the old password and can fail repeatedly until the process restarts and the cache resets. Consider including a password-derived or versioned component in the key so that a password change leads to a new TokenSource, while still avoiding logging or persisting the actual password.
</issue_to_address>
### Comment 2
<location path="charger/easee/identity.go" line_range="68" />
<code_context>
- token, err := c.authenticate()
+// TokenSourceWithInitial creates a token source, using initialToken if non-nil,
+// or performing a fresh login otherwise. Unlike TokenSource, this does not cache.
+func (c *tokenSource) TokenSourceWithInitial(initialToken *oauth2.Token) (oauth2.TokenSource, error) {
+ if initialToken == nil {
+ token, err := c.Authenticate()
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the identity API by removing the extra login/TokenSourceWithInitial helpers and letting callers compose Authenticate and RefreshToken directly.
You can keep the new functionality (external auth + initial tokens) but reduce surface area and hops by:
1. **Remove the `login()` hop and reuse `Authenticate()` in `RefreshToken`**
`login()` only exists to convert `*Token` → `*oauth2.Token` via `AsOAuth2Token()`, and `RefreshToken` can simply call `Authenticate()` on refresh failure.
```go
// Authenticate performs the initial username/password login and returns an oauth2.Token.
func (c *tokenSource) Authenticate() (*oauth2.Token, error) {
data := struct {
Username string `json:"userName"`
Password string `json:"password"`
}{
Username: c.user,
Password: c.password,
}
uri := fmt.Sprintf("%s/%s", API, "accounts/login")
req, err := request.New(http.MethodPost, uri, request.MarshalJSON(data), request.JSONEncoding)
if err != nil {
return nil, err
}
var token Token
if err := c.DoJSON(req, &token); err != nil {
return nil, err
}
return token.AsOAuth2Token(), nil
}
// RefreshToken refreshes an existing oauth2 token, falls back to a full re-login.
func (c *tokenSource) RefreshToken(oauthToken *oauth2.Token) (*oauth2.Token, error) {
data := struct {
AccessToken string `json:"accessToken"`
RefreshToken string `json:"refreshToken"`
}{
AccessToken: oauthToken.AccessToken,
RefreshToken: oauthToken.RefreshToken,
}
uri := fmt.Sprintf("%s/%s", API, "accounts/refresh_token")
req, err := request.New(http.MethodPost, uri, request.MarshalJSON(data), request.JSONEncoding)
if err != nil {
return nil, err
}
var token *Token
if err := c.DoJSON(req, &token); err != nil {
// re-login on refresh failure
return c.Authenticate()
}
return token.AsOAuth2Token(), nil
}
```
Then you can delete `login()` entirely, reducing one public/semipublic concept (`Authenticate` vs `login`) without changing behavior.
2. **Inline `TokenSourceWithInitial` policy at the caller**
`TokenSourceWithInitial` encodes a very specific policy (“use initial token if present, otherwise authenticate, then wrap in refresh source”) that’s only needed by plugin auth. The same behavior can be expressed directly where it’s used, keeping `tokenSource` focused on login + refresh.
Instead of:
```go
id := NewIdentity(log, user, password)
src, err := id.TokenSourceWithInitial(initialToken)
```
do at the call site:
```go
id := NewIdentity(log, user, password)
token := initialToken
if token == nil {
var err error
token, err = id.Authenticate()
if err != nil {
return nil, err
}
}
src := oauth.RefreshTokenSource(token, id.RefreshToken)
```
Then you can remove `TokenSourceWithInitial` from `tokenSource`, which shrinks the API and keeps the “token source wiring” logic in the plugin layer where the policy actually belongs.
These two changes keep all current capabilities (cached token source, external auth, initial tokens, refresh with re-login) but consolidate responsibilities into:
- `NewIdentity(log, user, password)`
- `(*tokenSource).Authenticate()`
- `(*tokenSource).RefreshToken()`
with callers composing them into token sources as needed.
</issue_to_address>
### Comment 3
<location path="plugin/auth/easee.go" line_range="47" />
<code_context>
+ return "easee-" + hex.EncodeToString(h[:])[:8]
+}
+
+// NewEaseeTokenSource returns a shared, persistent oauth2.TokenSource for the
+// given Easee account. Multiple callers with the same user email share one
+// token source so the Easee cloud API is not called unnecessarily.
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the token load/save logic from `NewEaseeTokenSource` into small helper functions to narrow its responsibilities and simplify its control flow.
You can keep the same behavior while making `NewEaseeTokenSource` narrower in scope by extracting the persistence concerns into small helpers. That reduces the number of responsibilities in the function and makes the side effects more obvious and testable, without changing the shared/persistent semantics.
### 1. Extract loading/persisting helpers
Pull DB interaction out of `NewEaseeTokenSource`:
```go
func loadEaseeToken(subject string) *oauth2.Token {
if !settings.Exists(subject) {
return nil
}
var storedToken oauth2.Token
if err := settings.Json(subject, &storedToken); err != nil {
return nil
}
if storedToken.RefreshToken == "" {
return nil
}
return &storedToken
}
func persistEaseeToken(log *util.Logger, subject string, token *oauth2.Token) {
if err := settings.SetJson(subject, token); err != nil {
log.WARN.Printf("failed to persist Easee token: %v", err)
}
}
```
### 2. Use those helpers in `NewEaseeTokenSource`
This keeps `NewEaseeTokenSource` mostly as an orchestration function:
```go
func NewEaseeTokenSource(user, password string) (oauth2.TokenSource, error) {
easeeInstancesMu.Lock()
defer easeeInstancesMu.Unlock()
subject := easeeSubject(user)
if ts, ok := easeeInstances[subject]; ok {
return ts, nil
}
log := util.NewLogger("easee").Redact(user, password)
id := easee.NewIdentity(log, user, password)
initialToken := loadEaseeToken(subject)
if initialToken == nil {
token, err := id.Authenticate()
if err != nil {
return nil, err
}
persistEaseeToken(log, subject, token)
initialToken = token
}
refreshWithPersist := func(token *oauth2.Token) (*oauth2.Token, error) {
newToken, err := id.RefreshToken(token)
if err != nil {
return nil, err
}
persistEaseeToken(log, subject, newToken)
return newToken, nil
}
ts := oauth.RefreshTokenSource(initialToken, refreshWithPersist)
easeeInstances[subject] = ts
return ts, nil
}
```
This keeps all current behavior (shared map, subject hashing, persisted tokens, refresh + persist), but the main function is shorter and easier to follow, and DB interactions are isolated in small, focused helpers.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
tolot27
force-pushed
the
easee_authplugin
branch
2 times, most recently
from
11dd4a6 to
858a137
Compare
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
easeeSubjectkey only uses the first 8 hex characters of the SHA-256 hash (~32 bits), which makes collisions between different user/password pairs more likely over time; consider using the full hash or at least a longer prefix to avoid two accounts accidentally sharing a token source. loadEaseeTokensilently drops errors fromsettings.Jsonand ignores tokens without a refresh token, which can mask configuration/data issues; consider logging JSON decode errors and optionally clearing or repairing the stored value to avoid repeated silent failures.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `easeeSubject` key only uses the first 8 hex characters of the SHA-256 hash (~32 bits), which makes collisions between different user/password pairs more likely over time; consider using the full hash or at least a longer prefix to avoid two accounts accidentally sharing a token source.
- `loadEaseeToken` silently drops errors from `settings.Json` and ignores tokens without a refresh token, which can mask configuration/data issues; consider logging JSON decode errors and optionally clearing or repairing the stored value to avoid repeated silent failures.
## Individual Comments
### Comment 1
<location path="plugin/auth/easee_test.go" line_range="126-50" />
<code_context>
+ require.ErrorIs(t, err, api.ErrMissingCredentials)
+}
+
+// TestNewEaseeFromConfig_CacheHit verifies that a cached entry is returned
+// when config is decoded successfully.
+func TestNewEaseeFromConfig_CacheHit(t *testing.T) {
+ defer resetEaseeInstances()
+
+ user := "config@example.com"
+ subject := easeeSubject(user, "pass")
+ fakeTS := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "cached"})
+ easeeInstancesMu.Lock()
+ easeeInstances[subject] = fakeTS
+ easeeInstancesMu.Unlock()
+
+ ts, err := newEaseeFromConfig(t.Context(), map[string]any{
+ "user": user,
+ "password": "pass",
+ })
+ require.NoError(t, err)
+ assert.Equal(t, fakeTS, ts)
+}
</code_context>
<issue_to_address>
**suggestion (testing):** Extend config tests to cover the `auth`-based configuration path on the charger side
These tests cover `newEaseeFromConfig` for missing credentials and cache hits, but they don’t exercise the new shared-auth path used via charger configuration (`auth: { source: "easee", ... }`). If there’s a charger-config decoding test harness, please add an integration-style test that builds a charger config using the `auth` block, verifies it resolves to a token source, and confirms that reusing the same `auth` reference across multiple chargers results in shared tokens. This will validate the wiring between charger config and the auth plugin.
Suggested implementation:
```golang
func TestNewEaseeFromConfig_MissingUser(t *testing.T) {
_, err := newEaseeFromConfig(t.Context(), map[string]any{"password": "secret"})
require.ErrorIs(t, err, api.ErrMissingCredentials)
}
// TestEaseeAuthViaChargerConfig_SharedTokenSource verifies that the charger
// configuration path using an `auth` block with source "easee" resolves to a
// token source and that reusing the same auth reference across multiple
// chargers results in shared tokens.
func TestEaseeAuthViaChargerConfig_SharedTokenSource(t *testing.T) {
t.Helper()
ctx := t.Context()
// This config is intentionally expressed as a generic map so the test stays
// close to the actual (e.g. YAML/JSON) configuration structure:
//
// auth:
// easeeDefault:
// source: easee
// user: config@example.com
// password: secret
// chargers:
// - name: c1
// auth: easeeDefault
// - name: c2
// auth: easeeDefault
cfg := map[string]any{
"auth": map[string]any{
"easeeDefault": map[string]any{
"source": "easee",
"user": "config@example.com",
"password": "secret",
},
},
"chargers": []any{
map[string]any{
"name": "c1",
"auth": "easeeDefault",
},
map[string]any{
"name": "c2",
"auth": "easeeDefault",
},
},
}
// Decode the charger configuration using the production charger config
// decoding path. This is expected to resolve the `auth` reference into a
// token source via the auth plugin (easee).
chargers, err := decodeChargersFromConfig(ctx, cfg)
require.NoError(t, err, "charger config decoding must succeed")
require.Len(t, chargers, 2, "expected two chargers from config")
// The decoded charger objects must expose the underlying token source
// they are configured with so we can assert that shared auth references
// share the same token source instance.
ts1 := chargers[0].TokenSource()
ts2 := chargers[1].TokenSource()
require.NotNil(t, ts1, "first charger must have a token source from auth plugin")
require.NotNil(t, ts2, "second charger must have a token source from auth plugin")
// Reusing the same `auth` reference ("easeeDefault") across chargers must
// result in shared token sources (same underlying oauth2.TokenSource).
require.Same(t, ts1, ts2, "chargers sharing the same auth reference must share the same token source")
}
func TestEaseeSubject_Stable(t *testing.T) {
```
To make this test compile and correctly exercise the real charger configuration path, you will need to:
1. **Provide `decodeChargersFromConfig`**:
- Implement or expose a helper in the relevant package (likely the charger or config package) with a signature similar to:
```go
// decodeChargersFromConfig decodes the given generic config map into
// fully-wired charger instances, resolving `auth` references via the
// auth plugin system (including the easee auth source).
func decodeChargersFromConfig(ctx context.Context, cfg map[string]any) ([]ChargerWithTokenSource, error)
```
- `ChargerWithTokenSource` should be (or wrap) your actual charger type and must expose a `TokenSource() oauth2.TokenSource` method so the test can inspect the resolved token source.
2. **Wire `auth` blocks through the existing config/charger machinery**:
- Ensure that the `auth` map on the root config is decoded such that:
- A block like:
```yaml
auth:
easeeDefault:
source: easee
user: config@example.com
password: secret
```
is passed through your auth plugin registry and ends up calling `newEaseeFromConfig` (or the equivalent production path for easee).
- Ensure that charger entries like:
```yaml
chargers:
- name: c1
auth: easeeDefault
```
resolve the `auth` reference by name, reuse the same underlying `oauth2.TokenSource` for all chargers with the same reference, and pass that token source into the charger instances returned by `decodeChargersFromConfig`.
3. **Align types and imports**:
- Adjust the test to import the correct package for the charger type and token source access (e.g. `charger.Charger` vs an internal type).
- If your chargers already expose the token source via a different method or field, adapt the `TokenSource()` call accordingly in the test.
</issue_to_address>
### Comment 2
<location path="plugin/auth/easee_test.go" line_range="59-68" />
<code_context>
+// TestNewEaseeTokenSource_DifferentPassword verifies that two calls with the same
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for persistence and token loading behavior (`loadEaseeToken` / `persistEaseeToken`)
Current tests for `NewEaseeTokenSource` cover in-memory cache behavior but not persistence. It would be valuable to add tests that verify: (1) an existing stored token is used instead of performing a fresh login, (2) tokens are persisted after login/refresh, and (3) stored tokens without a refresh token are ignored. If possible, point `settings` at a test-friendly or temporary backend so `loadEaseeToken` / `persistEaseeToken` behavior is covered.
Suggested implementation:
```golang
// TestNewEaseeTokenSource_DifferentPassword verifies that two calls with the same
// email but different passwords return different token-sources.
func TestNewEaseeTokenSource_DifferentPassword(t *testing.T) {
defer resetEaseeInstances()
// Pre-populate both cache entries to avoid real HTTP requests.
for _, pw := range []string{"pass", "newpass"} {
subject := easeeSubject("user@example.com", pw)
ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "fake-at-" + pw, RefreshToken: "fake-rt"})
easeeInstancesMu.Lock()
easeeInstances[subject] = ts
easeeInstancesMu.Unlock()
}
ts1, err := NewEaseeTokenSource("user@example.com", "pass")
require.NoError(t, err)
require.NotNil(t, ts1)
ts2, err := NewEaseeTokenSource("user@example.com", "newpass")
require.NoError(t, err)
require.NotNil(t, ts2)
assert.NotEqual(t, ts1, ts2, "same user with different passwords must return different token-sources")
}
// TestEaseeTokenPersistence_PersistAndLoad verifies that a persisted token
// can be loaded again via loadEaseeToken.
func TestEaseeTokenPersistence_PersistAndLoad(t *testing.T) {
defer resetEaseeInstances()
email := "persist@example.com"
original := &oauth2.Token{
AccessToken: "persist-at",
RefreshToken: "persist-rt",
}
err := persistEaseeToken(email, original)
require.NoError(t, err, "persistEaseeToken should not error")
loaded, err := loadEaseeToken(email)
require.NoError(t, err, "loadEaseeToken should not error")
if assert.NotNil(t, loaded, "loadEaseeToken should return a token when one was persisted") {
assert.Equal(t, original.AccessToken, loaded.AccessToken, "access token must round-trip through persistence")
assert.Equal(t, original.RefreshToken, loaded.RefreshToken, "refresh token must round-trip through persistence")
}
}
// TestEaseeTokenPersistence_IgnoresTokenWithoutRefresh verifies that stored
// tokens without a refresh token are ignored by loadEaseeToken.
func TestEaseeTokenPersistence_IgnoresTokenWithoutRefresh(t *testing.T) {
defer resetEaseeInstances()
email := "nort@example.com"
// Persist a token that has no refresh token.
err := persistEaseeToken(email, &oauth2.Token{
AccessToken: "no-rt-at",
})
require.NoError(t, err, "persistEaseeToken should not error for tokens without refresh token")
loaded, err := loadEaseeToken(email)
require.NoError(t, err, "loadEaseeToken should not error when ignoring invalid stored token")
assert.Nil(t, loaded, "loadEaseeToken should ignore stored tokens without a refresh token")
```
These tests assume the following signatures and behavior:
1. `persistEaseeToken(email string, tok *oauth2.Token) error`
2. `loadEaseeToken(email string) (*oauth2.Token, error)`
3. `loadEaseeToken` returns `(nil, nil)` when the stored token is considered invalid (e.g., missing `RefreshToken`).
If your actual function signatures differ, adjust the parameters and expectations accordingly.
To fully isolate persistence for tests and avoid touching real user settings, you should:
1. Point the global `settings` backend used by `loadEaseeToken` / `persistEaseeToken` to a test-specific or in-memory backend in a test `init` function or a test helper (e.g., something like `settings.Instance = settings.NewMemoryInstance()`).
2. Ensure that any global initialization of `settings` in production code does not override the test backend.
If `loadEaseeToken` uses a subject (e.g. `easeeSubject`) instead of an email string, update the tests to call it with the correct key and to use the same key in `persistEaseeToken`.
</issue_to_address>
### Comment 3
<location path="plugin/auth/easee.go" line_range="21" />
<code_context>
+ registry.AddCtx("easee", newEaseeFromConfig)
+}
+
+var (
+ easeeInstancesMu sync.Mutex
+ easeeInstances = make(map[string]oauth2.TokenSource)
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing the custom Easee token caching and lifecycle logic from plugin/auth and delegating to the existing easee.TokenSource with a thin persistence wrapper instead.
You’re re-implementing token source lifecycle, caching, and refresh in `plugin/auth` that already exists (or mostly exists) in `charger/easee/identity.go`. You can keep persistence while dropping the extra cache and Easee-specific logic from the plugin layer.
### 1. Drop the extra cache in `plugin/auth/easee.go`
Instead of `easeeInstancesMu` and `easeeInstances`, rely on the existing `easee.TokenSource` cache (via `tokenSourceCache`) and just add persistence around it.
```go
// plugin/auth/easee.go
func newEaseeFromConfig(_ context.Context, other map[string]any) (oauth2.TokenSource, error) {
var cc struct {
User string
Password string
}
if err := util.DecodeOther(other, &cc); err != nil {
return nil, err
}
if cc.User == "" || cc.Password == "" {
return nil, api.ErrMissingCredentials
}
// delegate all Easee auth & caching to easee package
log := util.NewLogger("easee").Redact(cc.User, cc.Password)
// this is assumed to be the existing abstraction in charger/easee/identity.go
baseTS, err := easee.TokenSource(log, cc.User, cc.Password)
if err != nil {
return nil, err
}
// thin persistence wrapper, see below
subject := easeeSubject(cc.User, cc.Password)
return persistentTokenSource(log, subject, baseTS), nil
}
```
### 2. Keep persistence as a thin wrapper (no extra Easee-specific cache)
Your load/persist helpers can stay, but they should not manage an additional map or identity; just wrap the existing `TokenSource`.
```go
// plugin/auth/easee.go
func persistentTokenSource(log *util.Logger, subject string, base oauth2.TokenSource) oauth2.TokenSource {
// best-effort load; failure just means we start with base.Token()
initial := loadEaseeToken(subject)
refreshWithPersist := func(_ *oauth2.Token) (*oauth2.Token, error) {
// always delegate to base
newToken, err := base.Token()
if err != nil {
return nil, err
}
persistEaseeToken(log, subject, newToken)
return newToken, nil
}
// if we have an initial token, seed the refresh source, otherwise just persist on first use
if initial != nil {
return oauth.RefreshTokenSource(initial, refreshWithPersist)
}
return oauth.RefreshTokenSource(nil, refreshWithPersist)
}
```
### 3. Avoid Easee-specific logic in `plugin/auth`
If possible, move `loadEaseeToken`/`persistEaseeToken` (and even `easeeSubject`) into the `easee` package as generic “persistent token” helpers so `plugin/auth` only does:
```go
// plugin/auth/easee.go
func newEaseeFromConfig(... ) (oauth2.TokenSource, error) {
// decode config
// ...
log := util.NewLogger("easee").Redact(cc.User, cc.Password)
return easee.PersistentTokenSource(log, cc.User, cc.Password)
}
```
This keeps all Easee-specific identity, caching, and persistence together and lets the plugin layer remain a thin configuration wrapper, while preserving the new persistence behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| // Both calls must return the same cached instance without making HTTP requests. | ||
| ts1, err := NewEaseeTokenSource("user@example.com", "pass") | ||
| require.NoError(t, err) |
Contributor
There was a problem hiding this comment.
suggestion (testing): Extend config tests to cover the auth-based configuration path on the charger side
These tests cover newEaseeFromConfig for missing credentials and cache hits, but they don’t exercise the new shared-auth path used via charger configuration (auth: { source: "easee", ... }). If there’s a charger-config decoding test harness, please add an integration-style test that builds a charger config using the auth block, verifies it resolves to a token source, and confirms that reusing the same auth reference across multiple chargers results in shared tokens. This will validate the wiring between charger config and the auth plugin.
Suggested implementation:
func TestNewEaseeFromConfig_MissingUser(t *testing.T) {
_, err := newEaseeFromConfig(t.Context(), map[string]any{"password": "secret"})
require.ErrorIs(t, err, api.ErrMissingCredentials)
}
// TestEaseeAuthViaChargerConfig_SharedTokenSource verifies that the charger
// configuration path using an `auth` block with source "easee" resolves to a
// token source and that reusing the same auth reference across multiple
// chargers results in shared tokens.
func TestEaseeAuthViaChargerConfig_SharedTokenSource(t *testing.T) {
t.Helper()
ctx := t.Context()
// This config is intentionally expressed as a generic map so the test stays
// close to the actual (e.g. YAML/JSON) configuration structure:
//
// auth:
// easeeDefault:
// source: easee
// user: config@example.com
// password: secret
// chargers:
// - name: c1
// auth: easeeDefault
// - name: c2
// auth: easeeDefault
cfg := map[string]any{
"auth": map[string]any{
"easeeDefault": map[string]any{
"source": "easee",
"user": "config@example.com",
"password": "secret",
},
},
"chargers": []any{
map[string]any{
"name": "c1",
"auth": "easeeDefault",
},
map[string]any{
"name": "c2",
"auth": "easeeDefault",
},
},
}
// Decode the charger configuration using the production charger config
// decoding path. This is expected to resolve the `auth` reference into a
// token source via the auth plugin (easee).
chargers, err := decodeChargersFromConfig(ctx, cfg)
require.NoError(t, err, "charger config decoding must succeed")
require.Len(t, chargers, 2, "expected two chargers from config")
// The decoded charger objects must expose the underlying token source
// they are configured with so we can assert that shared auth references
// share the same token source instance.
ts1 := chargers[0].TokenSource()
ts2 := chargers[1].TokenSource()
require.NotNil(t, ts1, "first charger must have a token source from auth plugin")
require.NotNil(t, ts2, "second charger must have a token source from auth plugin")
// Reusing the same `auth` reference ("easeeDefault") across chargers must
// result in shared token sources (same underlying oauth2.TokenSource).
require.Same(t, ts1, ts2, "chargers sharing the same auth reference must share the same token source")
}
func TestEaseeSubject_Stable(t *testing.T) {To make this test compile and correctly exercise the real charger configuration path, you will need to:
-
Provide
decodeChargersFromConfig:- Implement or expose a helper in the relevant package (likely the charger or config package) with a signature similar to:
// decodeChargersFromConfig decodes the given generic config map into // fully-wired charger instances, resolving `auth` references via the // auth plugin system (including the easee auth source). func decodeChargersFromConfig(ctx context.Context, cfg map[string]any) ([]ChargerWithTokenSource, error)
ChargerWithTokenSourceshould be (or wrap) your actual charger type and must expose aTokenSource() oauth2.TokenSourcemethod so the test can inspect the resolved token source.
- Implement or expose a helper in the relevant package (likely the charger or config package) with a signature similar to:
-
Wire
authblocks through the existing config/charger machinery:- Ensure that the
authmap on the root config is decoded such that:- A block like:
is passed through your auth plugin registry and ends up calling
auth: easeeDefault: source: easee user: config@example.com password: secret
newEaseeFromConfig(or the equivalent production path for easee).
- A block like:
- Ensure that charger entries like:
resolve the
chargers: - name: c1 auth: easeeDefault
authreference by name, reuse the same underlyingoauth2.TokenSourcefor all chargers with the same reference, and pass that token source into the charger instances returned bydecodeChargersFromConfig.
- Ensure that the
-
Align types and imports:
- Adjust the test to import the correct package for the charger type and token source access (e.g.
charger.Chargervs an internal type). - If your chargers already expose the token source via a different method or field, adapt the
TokenSource()call accordingly in the test.
- Adjust the test to import the correct package for the charger type and token source access (e.g.
Comment on lines
+59
to
+68
| // TestNewEaseeTokenSource_DifferentPassword verifies that two calls with the same | ||
| // email but different passwords return different token-sources. | ||
| func TestNewEaseeTokenSource_DifferentPassword(t *testing.T) { | ||
| defer resetEaseeInstances() | ||
|
|
||
| // Pre-populate both cache entries to avoid real HTTP requests. | ||
| for _, pw := range []string{"pass", "newpass"} { | ||
| subject := easeeSubject("user@example.com", pw) | ||
| ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "fake-at-" + pw, RefreshToken: "fake-rt"}) | ||
| easeeInstancesMu.Lock() |
Contributor
There was a problem hiding this comment.
suggestion (testing): Consider adding tests for persistence and token loading behavior (loadEaseeToken / persistEaseeToken)
Current tests for NewEaseeTokenSource cover in-memory cache behavior but not persistence. It would be valuable to add tests that verify: (1) an existing stored token is used instead of performing a fresh login, (2) tokens are persisted after login/refresh, and (3) stored tokens without a refresh token are ignored. If possible, point settings at a test-friendly or temporary backend so loadEaseeToken / persistEaseeToken behavior is covered.
Suggested implementation:
// TestNewEaseeTokenSource_DifferentPassword verifies that two calls with the same
// email but different passwords return different token-sources.
func TestNewEaseeTokenSource_DifferentPassword(t *testing.T) {
defer resetEaseeInstances()
// Pre-populate both cache entries to avoid real HTTP requests.
for _, pw := range []string{"pass", "newpass"} {
subject := easeeSubject("user@example.com", pw)
ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "fake-at-" + pw, RefreshToken: "fake-rt"})
easeeInstancesMu.Lock()
easeeInstances[subject] = ts
easeeInstancesMu.Unlock()
}
ts1, err := NewEaseeTokenSource("user@example.com", "pass")
require.NoError(t, err)
require.NotNil(t, ts1)
ts2, err := NewEaseeTokenSource("user@example.com", "newpass")
require.NoError(t, err)
require.NotNil(t, ts2)
assert.NotEqual(t, ts1, ts2, "same user with different passwords must return different token-sources")
}
// TestEaseeTokenPersistence_PersistAndLoad verifies that a persisted token
// can be loaded again via loadEaseeToken.
func TestEaseeTokenPersistence_PersistAndLoad(t *testing.T) {
defer resetEaseeInstances()
email := "persist@example.com"
original := &oauth2.Token{
AccessToken: "persist-at",
RefreshToken: "persist-rt",
}
err := persistEaseeToken(email, original)
require.NoError(t, err, "persistEaseeToken should not error")
loaded, err := loadEaseeToken(email)
require.NoError(t, err, "loadEaseeToken should not error")
if assert.NotNil(t, loaded, "loadEaseeToken should return a token when one was persisted") {
assert.Equal(t, original.AccessToken, loaded.AccessToken, "access token must round-trip through persistence")
assert.Equal(t, original.RefreshToken, loaded.RefreshToken, "refresh token must round-trip through persistence")
}
}
// TestEaseeTokenPersistence_IgnoresTokenWithoutRefresh verifies that stored
// tokens without a refresh token are ignored by loadEaseeToken.
func TestEaseeTokenPersistence_IgnoresTokenWithoutRefresh(t *testing.T) {
defer resetEaseeInstances()
email := "nort@example.com"
// Persist a token that has no refresh token.
err := persistEaseeToken(email, &oauth2.Token{
AccessToken: "no-rt-at",
})
require.NoError(t, err, "persistEaseeToken should not error for tokens without refresh token")
loaded, err := loadEaseeToken(email)
require.NoError(t, err, "loadEaseeToken should not error when ignoring invalid stored token")
assert.Nil(t, loaded, "loadEaseeToken should ignore stored tokens without a refresh token")These tests assume the following signatures and behavior:
persistEaseeToken(email string, tok *oauth2.Token) errorloadEaseeToken(email string) (*oauth2.Token, error)loadEaseeTokenreturns(nil, nil)when the stored token is considered invalid (e.g., missingRefreshToken).
If your actual function signatures differ, adjust the parameters and expectations accordingly.
To fully isolate persistence for tests and avoid touching real user settings, you should:
- Point the global
settingsbackend used byloadEaseeToken/persistEaseeTokento a test-specific or in-memory backend in a testinitfunction or a test helper (e.g., something likesettings.Instance = settings.NewMemoryInstance()). - Ensure that any global initialization of
settingsin production code does not override the test backend.
If loadEaseeToken uses a subject (e.g. easeeSubject) instead of an email string, update the tests to call it with the correct key and to use the same key in persistEaseeToken.
| registry.AddCtx("easee", newEaseeFromConfig) | ||
| } | ||
|
|
||
| var ( |
Contributor
There was a problem hiding this comment.
issue (complexity): Consider removing the custom Easee token caching and lifecycle logic from plugin/auth and delegating to the existing easee.TokenSource with a thin persistence wrapper instead.
You’re re-implementing token source lifecycle, caching, and refresh in plugin/auth that already exists (or mostly exists) in charger/easee/identity.go. You can keep persistence while dropping the extra cache and Easee-specific logic from the plugin layer.
1. Drop the extra cache in plugin/auth/easee.go
Instead of easeeInstancesMu and easeeInstances, rely on the existing easee.TokenSource cache (via tokenSourceCache) and just add persistence around it.
// plugin/auth/easee.go
func newEaseeFromConfig(_ context.Context, other map[string]any) (oauth2.TokenSource, error) {
var cc struct {
User string
Password string
}
if err := util.DecodeOther(other, &cc); err != nil {
return nil, err
}
if cc.User == "" || cc.Password == "" {
return nil, api.ErrMissingCredentials
}
// delegate all Easee auth & caching to easee package
log := util.NewLogger("easee").Redact(cc.User, cc.Password)
// this is assumed to be the existing abstraction in charger/easee/identity.go
baseTS, err := easee.TokenSource(log, cc.User, cc.Password)
if err != nil {
return nil, err
}
// thin persistence wrapper, see below
subject := easeeSubject(cc.User, cc.Password)
return persistentTokenSource(log, subject, baseTS), nil
}2. Keep persistence as a thin wrapper (no extra Easee-specific cache)
Your load/persist helpers can stay, but they should not manage an additional map or identity; just wrap the existing TokenSource.
// plugin/auth/easee.go
func persistentTokenSource(log *util.Logger, subject string, base oauth2.TokenSource) oauth2.TokenSource {
// best-effort load; failure just means we start with base.Token()
initial := loadEaseeToken(subject)
refreshWithPersist := func(_ *oauth2.Token) (*oauth2.Token, error) {
// always delegate to base
newToken, err := base.Token()
if err != nil {
return nil, err
}
persistEaseeToken(log, subject, newToken)
return newToken, nil
}
// if we have an initial token, seed the refresh source, otherwise just persist on first use
if initial != nil {
return oauth.RefreshTokenSource(initial, refreshWithPersist)
}
return oauth.RefreshTokenSource(nil, refreshWithPersist)
}3. Avoid Easee-specific logic in plugin/auth
If possible, move loadEaseeToken/persistEaseeToken (and even easeeSubject) into the easee package as generic “persistent token” helpers so plugin/auth only does:
// plugin/auth/easee.go
func newEaseeFromConfig(... ) (oauth2.TokenSource, error) {
// decode config
// ...
log := util.NewLogger("easee").Redact(cc.User, cc.Password)
return easee.PersistentTokenSource(log, cc.User, cc.Password)
}This keeps all Easee-specific identity, caching, and persistence together and lets the plugin layer remain a thin configuration wrapper, while preserving the new persistence behavior.
Contributor
Author
There was a problem hiding this comment.
@sourcery-ai is plugin/auth/easee.go necessary at all or can everything implemented in charger/easee/identity.go?
Contributor
There was a problem hiding this comment.
Good question. Conceptually, yes: all the Easee-specific auth logic (identity, token lifecycle, shared per-user TokenSource, persistence, etc.) can and probably should live in charger/easee/identity.go (or nearby in the same package), and plugin/auth/easee.go can be reduced to a very thin glue layer.
However, you still need some plugin/auth/easee.go file to register the provider with the generic auth plugin registry and to decode the plugin configuration. The plugin layer is what bridges from the configuration (other map) into the charger-specific code.
A clean split could look like this:
charger/easee/identity.go:- Implements
TokenSource(log, user, password)(as it already does) - Optionally exposes something like
PersistentTokenSource(log, user, password)if you want to encapsulate persistence there as well.
- Implements
plugin/auth/easee.go:- Only does:
- Decode
user/passwordfromother - Basic validation (missing credentials →
api.ErrMissingCredentials) - Setup logger with redaction
- Call into
easee.TokenSourceoreasee.PersistentTokenSource - Register itself in
initwithregistry.AddCtx("easee", newEaseeFromConfig)
- Decode
- No extra maps, mutexes, or token lifecycle logic.
- Only does:
So in terms of logic, you can move almost everything into charger/easee/identity.go. But you keep plugin/auth/easee.go as a minimal adapter, something like:
package auth
import (
"context"
"github.com/evcc-io/evcc/api"
"github.com/evcc-io/evcc/charger/easee"
"github.com/evcc-io/evcc/util"
"golang.org/x/oauth2"
)
func init() {
registry.AddCtx("easee", newEaseeFromConfig)
}
func newEaseeFromConfig(_ context.Context, other map[string]any) (oauth2.TokenSource, error) {
var cc struct {
User string
Password string
}
if err := util.DecodeOther(other, &cc); err != nil {
return nil, err
}
if cc.User == "" || cc.Password == "" {
return nil, api.ErrMissingCredentials
}
log := util.NewLogger("easee").Redact(cc.User, cc.Password)
// or easee.PersistentTokenSource if you move persistence there
return easee.TokenSource(log, cc.User, cc.Password)
}If you prefer full encapsulation, you can also:
- Implement
PersistentTokenSourceincharger/easee(using settings DB, etc.). - Have
plugin/auth/easee.goonly call that and avoid any auth-specific logic in the plugin layer.
So: plugin/auth/easee.go is still necessary as the registration/config adapter, but all substantial auth behavior can live in charger/easee/identity.go (or subpackage) to keep concerns properly separated.
implement account for storing user, password together with token
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements an authentication provider for Easee charges. Only a single authentication will be necessary for all chargers of the same user (bound to the email address).
Fixes #29501
open tasks: