Skip to content

fix: Refresh Token Failures - #298

Open
GuyKh wants to merge 3 commits into
mainfrom
400-response
Open

fix: Refresh Token Failures#298
GuyKh wants to merge 3 commits into
mainfrom
400-response

Conversation

@GuyKh

@GuyKh GuyKh commented Jan 19, 2026

Copy link
Copy Markdown
Owner

PR Type

Bug fix, Enhancement


Description

  • Handle 400 Bad Request token refresh failures with reconfigure flow

  • Add null checks for coordinator data in sensor and binary sensor setup

  • Implement reconfigure flow for authentication credential updates

  • Improve type hints using modern Python union syntax


Diagram Walkthrough

flowchart LR
  A["Token Refresh Error"] -->|400 Status| B["_handle_auth_error"]
  B -->|Trigger| C["Reconfigure Flow"]
  C -->|User Updates| D["async_step_reconfigure_mfa"]
  D -->|Validate & Update| E["Config Entry Updated"]
  F["Coordinator Setup"] -->|Check Data| G["Null Safety Guards"]
  G -->|Skip if Empty| H["Prevent Setup Errors"]
Loading

File Walkthrough

Relevant files
Error handling
__init__.py
Handle authentication failures explicitly                               

custom_components/iec/init.py

  • Import ConfigEntryAuthFailed exception for proper error handling
  • Re-raise ConfigEntryAuthFailed exceptions instead of suppressing them
  • Remove misleading comment about not failing setup on errors
+3/-1     
Bug fix
binary_sensor.py
Add null safety checks for coordinator data                           

custom_components/iec/binary_sensor.py

  • Add null check for coordinator.data before accessing keys
  • Add early return with warning log if coordinator data is unavailable
  • Prevent setup errors when data is not yet loaded
+8/-1     
coordinator.py
Add auth error handling and reconfigure flow trigger         

custom_components/iec/coordinator.py

  • Add _handle_auth_error method to handle authentication errors and
    trigger reconfigure flow
  • Detect 400 Bad Request status and log specific error message
  • Initiate reconfigure flow when authentication fails
  • Call _handle_auth_error before raising ConfigEntryAuthFailed in
    _async_update_data
  • Update type hints from tuple syntax (int, float) to union syntax int |
    float
+29/-3   
sensor.py
Add null safety checks for coordinator data                           

custom_components/iec/sensor.py

  • Add null check for coordinator.data before accessing keys in filter
  • Add early return with warning log if coordinator data is unavailable
  • Prevent setup errors when data is not yet loaded
+6/-1     
Enhancement
config_flow.py
Add reconfigure flow for credential updates                           

custom_components/iec/config_flow.py

  • Fix docstring capitalization from "IECConfigFlow" to "IecConfigFlow"
  • Add reconfigure_entry attribute to store reconfigure context
  • Implement async_step_reconfigure method to initiate reconfigure flow
  • Implement async_step_reconfigure_mfa method to handle MFA during
    reconfiguration
  • Support credential updates and config entry reload
+72/-1   

@qodo-code-review

qodo-code-review Bot commented Jan 19, 2026

Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
CancelledError swallowed: asyncio.CancelledError is caught and converted into a normal "cannot_connect"
error instead of being re-raised, which can break task cancellation semantics and lead to
hung/shutdown issues.

Referred Code
try:
    otp_type = await client.login_with_id()
except asyncio.CancelledError:
    errors["base"] = errors.get("base") or "cannot_connect"
    otp_type = "OTP"
except IECError:
    errors["base"] = errors.get("base") or "cannot_connect"
    otp_type = "OTP"

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unsafe user_input access: The code indexes user_input[CONF_TOTP_SECRET] without guarding for missing keys, which can
raise KeyError on unexpected/partial input instead of failing gracefully.

Referred Code
if user_input is not None and user_input[CONF_TOTP_SECRET] is not None:
    assert client
    data = {**self.reconfigure_entry.data, **user_input}
    errors = await _validate_login(self.hass, data, client)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing audit context: Authentication/reconfiguration-triggering events are logged without clear audit context
(e.g., config entry identifier/user context/outcome), making it difficult to reconstruct
who/what was affected.

Referred Code
    _LOGGER.error(
        "Token refresh failed with 400 Bad Request. Need to reconfigure integration."
    )
else:
    _LOGGER.error(
        "Authentication error occurred (status %s): %s", error.status, error
    )

entry = self.hass.config_entries.async_get_entry(self._config_entry.entry_id)
if entry:
    try:
        await self.hass.config_entries.flow_manager.async_init(
            DOMAIN, context={"source": "reconfigure"}, entry_id=entry.entry_id
        )
    except Exception as err:  # noqa: BLE001
        _LOGGER.error("Failed to start reconfigure flow: %s", err)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Sensitive data risk: Logging IECError via %s/exception traces may inadvertently include sensitive details
(e.g., token contents or identifiers) depending on IECError.str/payload formatting,
which is not verifiable from this diff.

Referred Code
_LOGGER.error(
    "Authentication error occurred (status %s): %s", error.status, error
)

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review

qodo-code-review Bot commented Jan 19, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Latest suggestions up to 3236c3d

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix invalid isinstance type check
Suggestion Impact:The commit updates the `isinstance` calls to use a tuple `(int, float)` instead of the union `int | float` for both kWh and kVA tariff parsing, matching the suggested fix. (The commit also includes an unrelated change to config entry flow initialization.)

code diff:

-                    if isinstance(kwh_val, int | float):
+                    if isinstance(kwh_val, (int, float)):
                         kwh_tariff = float(kwh_val)
-                    if isinstance(kva_val, int | float):
+                    if isinstance(kva_val, (int, float)):
                         kva_tariff = float(kva_val)
                     _LOGGER.debug(
                         "Fetched fallback tariffs from calculators/period: homeRate=%s, kvaRate=%s",
@@ -291,7 +291,7 @@
                         data = await resp.json(content_type=None)
                         rates = data.get("gadget_Calculator_Rates") or {}
                         kwh_val = rates.get("homeRate")
-                        if isinstance(kwh_val, int | float):
+                        if isinstance(kwh_val, (int, float)):
                             kwh_tariff = float(kwh_val)

Replace the int | float union type in isinstance() calls with a tuple (int,
float) to prevent a TypeError at runtime.

custom_components/iec/coordinator.py [267-295]

--                    if isinstance(kwh_val, int | float):
+-                    if isinstance(kwh_val, (int, float)):
                         kwh_tariff = float(kwh_val)
--                    if isinstance(kva_val, int | float):
+-                    if isinstance(kva_val, (int, float)):
                         kva_tariff = float(kva_val)
 ...
--                        if isinstance(kwh_val, int | float):
+-                        if isinstance(kwh_val, (int, float)):
                             kwh_tariff = float(kwh_val)

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies that using int | float in isinstance will cause a TypeError at runtime, which is a critical bug introduced in the PR.

High
Fix form schema validators
Suggestion Impact:The schema definition for CONF_USER_ID was corrected to use `str` as the validator and the existing entry value as the `default`, matching the intent of the suggestion and fixing the inverted validator/default bug.

code diff:

         schema = {
-            vol.Required(CONF_USER_ID): self.reconfigure_entry.data[CONF_USER_ID],
+            vol.Required(
+                CONF_USER_ID, default=self.reconfigure_entry.data[CONF_USER_ID]
+            ): str,
             vol.Required(CONF_TOTP_SECRET): str,
         }

Correct the voluptuous schema by using str as the validator for CONF_USER_ID and
setting its value as a default, instead of using the value as the validator.

custom_components/iec/config_flow.py [372-375]

 schema = {
-    vol.Required(CONF_USER_ID): self.reconfigure_entry.data[CONF_USER_ID],
+    vol.Required(CONF_USER_ID, default=self.reconfigure_entry.data[CONF_USER_ID]): str,
     vol.Required(CONF_TOTP_SECRET): str,
 }

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies an inverted mapping in the voluptuous schema that would cause form validation to fail, which is a critical bug in the new reconfigure flow.

High
Start reconfigure flow correctly
Suggestion Impact:The commit updates the reconfigure flow initiation to call hass.config_entries.flow.async_init and moves entry_id into the context dictionary, matching the suggested fix.

code diff:

@@ -821,10 +821,12 @@
                         )
                         if entry:
                             try:
-                                await self.hass.config_entries.flow_manager.async_init(
+                                await self.hass.config_entries.flow.async_init(
                                     DOMAIN,
-                                    context={"source": "reconfigure"},
-                                    entry_id=entry.entry_id,
+                                    context={
+                                        "source": "reconfigure",
+                                        "entry_id": entry.entry_id,
+                                    },
                                 )

Initiate the reconfigure flow using hass.config_entries.flow.async_init and pass
the entry_id within the context dictionary to ensure it starts correctly.

custom_components/iec/coordinator.py [824-828]

-await self.hass.config_entries.flow_manager.async_init(
+await self.hass.config_entries.flow.async_init(
     DOMAIN,
-    context={"source": "reconfigure"},
-    entry_id=entry.entry_id,
+    context={"source": "reconfigure", "entry_id": entry.entry_id},
 )

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly points out that the reconfigure flow is initiated with an incorrect API call, which would prevent the authentication recovery mechanism from working as intended.

High
  • Update

Previous suggestions

✅ Suggestions up to commit e2dd6ad
CategorySuggestion                                                                                                                                    Impact
General
Safely access MFA input
Suggestion Impact:The commit updated the conditional to use a truthy check on user_input and safely access CONF_TOTP_SECRET via .get(), preventing potential KeyError.

code diff:

-        if user_input is not None and user_input[CONF_TOTP_SECRET] is not None:
+        if user_input and user_input.get(CONF_TOTP_SECRET) is not None:

Replace direct dictionary access user_input[CONF_TOTP_SECRET] with the safer
.get() method to prevent a KeyError if the key is not present.

custom_components/iec/config_flow.py [333-337]

-if user_input is not None and user_input[CONF_TOTP_SECRET] is not None:
+if user_input and user_input.get(CONF_TOTP_SECRET) is not None:
     assert client
     data = {**self.reconfigure_entry.data, **user_input}
     errors = await _validate_login(self.hass, data, client)
     if not errors:
         ...
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out a potential KeyError and proposes using .get() for safer dictionary access, which improves the code's robustness.

Low

@GuyKh
GuyKh force-pushed the 400-response branch 3 times, most recently from 28217ef to 3236c3d Compare January 19, 2026 13:33
…re flow, and safe MFA access

- Replace int | float union types with (int, float) tuples in isinstance calls
- Correct voluptuous schema to use str validator with default for CONF_USER_ID
- Fix reconfigure flow initiation to use correct API with entry_id in context
- Use .get() method for safer MFA input access to prevent KeyError
@GuyKh
GuyKh force-pushed the 400-response branch 2 times, most recently from 9bd81c6 to e682904 Compare January 25, 2026 18:08
- Clear CONF_TOTP_SECRET from data during reconfigure initialization
- Remove CONF_USER_ID from reconfigure MFA schema
- Create new client with existing user ID for authentication
- Ensure no new entities are added, only update existing entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant