Skip to content

Commit

Permalink
Fix nonce, add auto retry on auth exception, fix session id int, impr…
Browse files Browse the repository at this point in the history
…ove code quality (#336)

This pull request includes several changes to improve error handling,
enhance functionality, and update configurations. The most important
changes include adding new exception handling for invalid sessions,
updating the `postCreateCommand` in the devcontainer, and adding custom
funding options.

### Error Handling Improvements:
* Added `InvalidSessionException` to handle invalid session errors and
updated relevant methods to raise this exception when needed
(`sagemcom_api/client.py`, `sagemcom_api/const.py`,
`sagemcom_api/exceptions.py`).
[[1]](diffhunk://#diff-052218051094ff1cc2c8352cd555cfb9eb02bf261eb5e635cf73b798809306a5R32)
[[2]](diffhunk://#diff-052218051094ff1cc2c8352cd555cfb9eb02bf261eb5e635cf73b798809306a5R46)
[[3]](diffhunk://#diff-052218051094ff1cc2c8352cd555cfb9eb02bf261eb5e635cf73b798809306a5R220-R225)
[[4]](diffhunk://#diff-57cf505b5e01fc69f170c5a1b839fb40b45a97a9c0ebc1fe53f4bd524ae9ac04R10)
[[5]](diffhunk://#diff-63c847110513149341a9f7e94d412bfb7c67d79df07a321774a9a2f3329ca34fR4-L43)
* Introduced `retry_login` function to retry login on specific
exceptions using backoff (`sagemcom_api/client.py`).
[[1]](diffhunk://#diff-052218051094ff1cc2c8352cd555cfb9eb02bf261eb5e635cf73b798809306a5R58-R62)
[[2]](diffhunk://#diff-052218051094ff1cc2c8352cd555cfb9eb02bf261eb5e635cf73b798809306a5R260-R270)

### Functional Enhancements:
* Modified `get_hosts` method to include `capability-flags` in the
options for retrieving hosts (`sagemcom_api/client.py`).
* Corrected the nonce generation logic to use the correct range
(`sagemcom_api/client.py`).

### Configuration Updates:
* Updated `postCreateCommand` in `.devcontainer/devcontainer.json` to
include `pre-commit install-hooks` for better pre-commit setup.
* Added a custom funding option in `.github/FUNDING.yml` to include a
PayPal link.
  • Loading branch information
iMicknl authored Aug 9, 2024
1 parent 3da2376 commit 7e039cb
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 24 deletions.
2 changes: 1 addition & 1 deletion .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// Use 'forwardPorts' to make a list of ports inside the container available locally.
// "forwardPorts": [],
// Use 'postCreateCommand' to run commands after the container is created.
"postCreateCommand": "poetry config virtualenvs.in-project true && poetry install --with dev --no-interaction && . .venv/bin/activate && pre-commit install",
"postCreateCommand": "poetry config virtualenvs.in-project true && poetry install --with dev --no-interaction && . .venv/bin/activate && pre-commit install && pre-commit install-hooks",
// Configure tool-specific properties.
"customizations": {
"vscode": {
Expand Down
1 change: 1 addition & 0 deletions .github/FUNDING.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
custom: ["https://paypal.me/imick"]
35 changes: 32 additions & 3 deletions sagemcom_api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
from __future__ import annotations

import asyncio
from collections.abc import Mapping
import hashlib
import json
import math
import random
from types import TracebackType
from typing import Any
import urllib.parse

from aiohttp import (
Expand All @@ -27,6 +29,7 @@
DEFAULT_USER_AGENT,
XMO_ACCESS_RESTRICTION_ERR,
XMO_AUTHENTICATION_ERR,
XMO_INVALID_SESSION_ERR,
XMO_LOGIN_RETRY_ERR,
XMO_MAX_SESSION_COUNT_ERR,
XMO_NO_ERR,
Expand All @@ -40,6 +43,7 @@
AccessRestrictionException,
AuthenticationException,
BadRequestException,
InvalidSessionException,
LoginRetryErrorException,
LoginTimeoutException,
MaximumSessionCountException,
Expand All @@ -51,6 +55,11 @@
from .models import Device, DeviceInfo, PortMapping


async def retry_login(invocation: Mapping[str, Any]) -> None:
"""Retry login via backoff if an exception occurs."""
await invocation["args"][0].login()


# pylint: disable=too-many-instance-attributes
class SagemcomClient:
"""Client to communicate with the Sagemcom API."""
Expand Down Expand Up @@ -118,7 +127,7 @@ async def close(self) -> None:

def __generate_nonce(self):
"""Generate pseudo random number (nonce) to avoid replay attacks."""
self._current_nonce = math.floor(random.randrange(0, 1) * 500000)
self._current_nonce = math.floor(random.randrange(0, 500000))

def __generate_request_id(self):
"""Generate sequential request ID."""
Expand Down Expand Up @@ -187,6 +196,7 @@ def __get_response_value(self, response, index=0):
(ClientConnectorError, ClientOSError, ServerDisconnectedError),
max_tries=5,
)
# pylint: disable=too-many-branches
async def __post(self, url, data):
async with self.session.post(url, data=data) as response:
if response.status == 400:
Expand All @@ -207,6 +217,12 @@ async def __post(self, url, data):
):
return result

if error["description"] == XMO_INVALID_SESSION_ERR:
self._session_id = 0
self._server_nonce = ""
self._request_id = -1
raise InvalidSessionException(error)

# Error in one of the actions
if error["description"] == XMO_REQUEST_ACTION_ERR:
# pylint:disable=fixme
Expand Down Expand Up @@ -241,6 +257,17 @@ async def __post(self, url, data):

return result

@backoff.on_exception(
backoff.expo,
(
AuthenticationException,
LoginRetryErrorException,
LoginTimeoutException,
InvalidSessionException,
),
max_tries=2,
on_backoff=retry_login,
)
async def __api_request_async(self, actions, priority=False):
"""Build request to the internal JSON-req API."""
self.__generate_request_id()
Expand All @@ -252,7 +279,7 @@ async def __api_request_async(self, actions, priority=False):
payload = {
"request": {
"id": self._request_id,
"session-id": str(self._session_id),
"session-id": int(self._session_id),
"priority": priority,
"actions": actions,
"cnonce": self._current_nonce,
Expand Down Expand Up @@ -435,7 +462,9 @@ async def get_device_info(self) -> DeviceInfo:

async def get_hosts(self, only_active: bool | None = False) -> list[Device]:
"""Retrieve hosts connected to Sagemcom F@st device."""
data = await self.get_value_by_xpath("Device/Hosts/Hosts")
data = await self.get_value_by_xpath(
"Device/Hosts/Hosts", options={"capability-flags": {"interface": True}}
)
devices = [Device(**d) for d in data]

if only_active:
Expand Down
1 change: 1 addition & 0 deletions sagemcom_api/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

XMO_ACCESS_RESTRICTION_ERR = "XMO_ACCESS_RESTRICTION_ERR"
XMO_AUTHENTICATION_ERR = "XMO_AUTHENTICATION_ERR"
XMO_INVALID_SESSION_ERR = "XMO_INVALID_SESSION_ERR"
XMO_NON_WRITABLE_PARAMETER_ERR = "XMO_NON_WRITABLE_PARAMETER_ERR"
XMO_NO_ERR = "XMO_NO_ERR"
XMO_REQUEST_ACTION_ERR = "XMO_REQUEST_ACTION_ERR"
Expand Down
48 changes: 28 additions & 20 deletions sagemcom_api/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,43 +1,51 @@
"""Exceptions for the Sagemcom F@st client."""


class BaseSagemcomException(Exception):
"""Base exception for Sagemcom F@st client."""


# Broad exceptions provided by this library
class BadRequestException(BaseSagemcomException):
"""Bad request."""


class UnauthorizedException(BaseSagemcomException):
"""Unauthorized."""


class UnknownException(BaseSagemcomException):
"""Unknown exception."""


# Exceptions provided by SagemCom API
class AccessRestrictionException(Exception):
class AccessRestrictionException(BaseSagemcomException):
"""Raised when current user has access restrictions."""


class AuthenticationException(Exception):
class AuthenticationException(UnauthorizedException):
"""Raised when authentication is not correct."""


class LoginRetryErrorException(Exception):
class InvalidSessionException(UnauthorizedException):
"""Raised when session is invalid."""


class LoginRetryErrorException(BaseSagemcomException):
"""Raised when too many login retries are attempted."""


class LoginTimeoutException(Exception):
class LoginTimeoutException(BaseSagemcomException):
"""Raised when a timeout is encountered during login."""


class NonWritableParameterException(Exception):
class NonWritableParameterException(BaseSagemcomException):
"""Raised when provided parameter is not writable."""


class UnknownPathException(Exception):
class UnknownPathException(BaseSagemcomException):
"""Raised when provided path does not exist."""


class MaximumSessionCountException(Exception):
class MaximumSessionCountException(BaseSagemcomException):
"""Raised when the maximum session count is reached."""


# Broad exceptions provided by this library
class BadRequestException(Exception):
"""TODO."""


class UnauthorizedException(Exception):
"""TODO."""


class UnknownException(Exception):
"""TODO."""

0 comments on commit 7e039cb

Please sign in to comment.