-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
myStrom: add optional authentication token support #27047
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
Conversation
Add support for optional authentication token for myStrom WiFi Switch devices. The token can be configured on the device to restrict API access. When a token is set on the device, it must be provided via the Token HTTP header in all API requests. Devices without a token configured continue to work as before.
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.
Hey - I've found 2 issues, and left some high level feedback:
- The logic for attaching the Token header is duplicated in both Connection.Request and Connection.Report; consider centralizing this in a small helper (e.g., a method that builds requests with the base URI and optional token) to reduce repetition and keep future changes localized.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic for attaching the Token header is duplicated in both Connection.Request and Connection.Report; consider centralizing this in a small helper (e.g., a method that builds requests with the base URI and optional token) to reduce repetition and keep future changes localized.
## Individual Comments
### Comment 1
<location> `templates/definition/charger/mystrom.yaml:17-19` </location>
<code_context>
type: mystrom
uri: http://{{ .host }}
+ {{- if .token }}
+ token: {{ .token }}
+ {{- end }}
{{ include "switchsocket" . }}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Quote the rendered token value to avoid YAML parsing issues with special characters.
Unquoted tokens containing `:`, `#`, or braces can break YAML parsing. Please quote the interpolation (e.g. `token: "{{ .token }}"` or single quotes) so arbitrary token values are handled safely.
```suggestion
{{- if .token }}
token: "{{ .token }}"
{{- end }}
```
</issue_to_address>
### Comment 2
<location> `templates/definition/meter/mystrom.yaml:19` </location>
<code_context>
type: mystrom
uri: http://{{ .host }}
+ {{- if .token }}
+ token: {{ .token }}
+ {{- end }}
{{ include "switchsocket" . }}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Similarly, quote the token value here to keep YAML robust for arbitrary token strings.
This also keeps it consistent with the charger template; please use `token: "{{ .token }}"` here.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| type: mystrom | ||
| uri: http://{{ .host }} | ||
| {{- if .token }} | ||
| token: {{ .token }} |
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.
suggestion (bug_risk): Similarly, quote the token value here to keep YAML robust for arbitrary token strings.
This also keeps it consistent with the charger template; please use token: "{{ .token }}" here.
|
Thanks for the feedback. I've applied and tested the suggested changes. |
Add support for optional authentication token for myStrom WiFi Switch devices. The token can be configured on the device to restrict API access. When a token is set on the device, it must be provided via the
TokenHTTP header in all API requests. Devices without a token configured continue to work as before.The authentication is described in the Security section of their API documentation:
https://api.mystrom.ch/