Repository: mattrayner/podpointclient (language: python)

Issue:
Correctly handle token refreshes with Google
**Is your feature request related to a problem? Please describe.**
Currently every time the Google token expires, we are creating a new one, sending the user creds to Google and receiving a token in response. We should behave like good API citizens and instead, refresh the token.

**Describe the solution you'd like**
We should identify wether or not a token has expired (or is neat expiry) and change the request that we send to be a 'refresh' rather than a new authorisation.

We will need to handle the various errors that can be sent by Google as part of the refresh, and use these to inform the user of issues. Additionally, we should have automatic re-auth. In instances where we are able to, we should attempt a 'full' re-authentication when the refresh fails.

**Describe alternatives you've considered**
We could leave the system as it is... It works, but we should always strive to be better, and leaving it is certainly not best practices.

**Additional context**
After Pod Point changed their auth system to Google, the ability to refresh opened up.


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml
index 288cbb5..60d81ee 100644
--- a/.github/workflows/push.yml
+++ b/.github/workflows/push.yml
@@ -5,8 +5,6 @@ on:
     branches:
       - main
       - dev
-    tags:
-      - '*'
 
 jobs:
   tests:
@@ -39,11 +37,3 @@ jobs:
         with:
           name: code-coverage-report
           path: htmlcov/*
-      - name: Build release package
-        if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags')
-        run: make package
-      - name: Publish package
-        if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags')
-        uses: pypa/gh-action-pypi-publish@release/v1
-        with:
-          password: ${{ secrets.PYPI_API_TOKEN }}
diff --git a/.github/workflows/tag.yml b/.github/workflows/tag.yml
new file mode 100644
index 0000000..50bae6e
--- /dev/null
+++ b/.github/workflows/tag.yml
@@ -0,0 +1,44 @@
+name: Push actions
+
+on:
+  push:
+    tags:
+      - '*'
+
+jobs:
+  tests:
+    runs-on: "ubuntu-latest"
+    name: Test and release
+    steps:
+      - name: Check out code from GitHub
+        uses: "actions/checkout@v3"
+      - name: Setup Python
+        uses: "actions/setup-python@v4"
+        with:
+          python-version: "3.10"
+      - name: Install requirements
+        run: python3 -m pip install -r requirements_test.txt -r requirements_release.txt
+      - name: Run tests
+        run: |
+          python3 -m pytest \
+            -vv \
+            -qq \
+            --timeout=9 \
+            --durations=10 \
+            --cov podpointclient \
+            --cov-report term \
+            --cov-report html \
+            -o console_output_style=count \
+            -p no:sugar \
+            tests
+      - name: Archive code coverage results
+        uses: actions/upload-artifact@v3
+        with:
+          name: code-coverage-report
+          path: htmlcov/*
+      - name: Build release package
+        run: make package
+      - name: Publish package
+        uses: pypa/gh-action-pypi-publish@release/v1
+        with:
+          password: ${{ secrets.PYPI_API_TOKEN }}
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6653b0a..a988109 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Pod Point Client Changelog
 
+## v1.5.0
+
+* Add support for refreshing expired tokens, rather than grabbing new ones each time
+* Update example.py to demonstrate token expiry
+
 ## v1.4.3
 
 * Remove additional / from pod point api calls
diff --git a/Makefile b/Makefile
index bbff0a6..49c8b7f 100644
--- a/Makefile
+++ b/Makefile
@@ -13,6 +13,7 @@ spec:
 		--cov-report html \
 		-o console_output_style=count \
 		-p no:sugar \
+		-s \
 		tests
 
 lint:
@@ -25,4 +26,4 @@ package:
 	python3 setup.py sdist
 
 publish: clean package
-	twine upload dist/* --verbose
\ No newline at end of file
+	twine upload dist/* --verbose
diff --git a/example.py b/example.py
index f93d283..936698b 100644
--- a/example.py
+++ b/example.py
@@ -1,3 +1,4 @@
+from datetime import datetime, timedelta
 from podpointclient.client import PodPointClient
 import asyncio
 import aiohttp
@@ -19,10 +20,14 @@ async def main(username: str, password: str, http_debug: bool = False, loop=None
 
     # Verify credentials work
     verified = await client.async_credentials_verified()
-    print(f"  Credentials verified: {verified}")
+    print(f"Credentials verified: {verified}")
+    print(f"  Token expiry: {client.auth.access_token_expiry}")
+
+    print("Sleeping 2s")
+    time.sleep(2)
 
-    print("Getting user details")
     # Get user information
+    print("Getting user details")
     user = await client.async_get_user()
     print(f"  Account balance {user.account.balance}p")
 
@@ -35,7 +40,7 @@ async def main(username: str, password: str, http_debug: bool = False, loop=None
     pod = pods[0]
     print(f"Selecting first pod: {pod.ppid}")
 
-    # Get firmware information for the pod
+    # Get firmware information for the pod
     firmwares = await client.async_get_firmware(pod=pod)
     firmware = firmwares[0]
     print(f"Gettnig firmware data for {pod.ppid}")
@@ -58,6 +63,17 @@ async def main(username: str, password: str, http_debug: bool = False, loop=None
     energy_used = charges[0].kwh_used
     print(f"  kW charged: {energy_used}")
 
+    # Expire token and exchange a refresh
+    print("Expiring token and refreshing...")
+    client.auth.access_token_expiry = datetime.now() - timedelta(minutes=10)
+    updated = await client.auth.async_update_access_token()
+    print(f"  Token updated? {updated} - New expiry: {client.auth.access_token_expiry}")
+
+    # Get user information again
+    print("Getting user details with new token")
+    user = await client.async_get_user()
+    print(f"  Account balance {user.account.balance}p")
+
 if __name__ == "__main__":
     import time
     import argparse
diff --git a/podpointclient/endpoints.py b/podpointclient/endpoints.py
index 6a72c48..cec3cbe 100644
--- a/podpointclient/endpoints.py
+++ b/podpointclient/endpoints.py
@@ -17,6 +17,10 @@
 """Google endpoint, used for auth"""
 GOOGLE_KEY = '?key=AIzaSyCwhF8IOl_7qHXML0pOd5HmziYP46IZAGU'
 PASSWORD_VERIFY = f"/verifyPassword{GOOGLE_KEY}"
+TOKEN = f"/token{GOOGLE_KEY}"
 
 GOOGLE_BASE = 'www.googleapis.com/identitytoolkit/v3/relyingparty'
 GOOGLE_BASE_URL = f"https://{GOOGLE_BASE}"
+
+GOOGLE_TOKEN_BASE = 'securetoken.googleapis.com/v1'
+GOOGLE_TOKEN_BASE_URL = f"https://{GOOGLE_TOKEN_BASE}"
\ No newline at end of file
diff --git a/podpointclient/helpers/api_wrapper.py b/podpointclient/helpers/api_wrapper.py
index b7aa65b..4ee71b0 100644
--- a/podpointclient/helpers/api_wrapper.py
+++ b/podpointclient/helpers/api_wrapper.py
@@ -125,12 +125,20 @@ async def __wrapper(
                     )
 
                 elif method == "post":
-                    response = await self._session.post(
-                        url,
-                        headers=headers,
-                        params=params,
-                        json=data
-                    )
+                    if isinstance(data, str):
+                        response = await self._session.post(
+                            url,
+                            headers=headers,
+                            params=params,
+                            data=data
+                        )
+                    else:
+                        response = await self._session.post(
+                            url,
+                            headers=headers,
+                            params=params,
+                            json=data
+                        )
 
                 elif method == "delete":
                     response = await self._session.delete(url, headers=headers, params=params)
diff --git a/podpointclient/helpers/auth.py b/podpointclient/helpers/auth.py
index ccad223..71519cd 100644
--- a/podpointclient/helpers/auth.py
+++ b/podpointclient/helpers/auth.py
@@ -7,7 +7,7 @@
 
 from ..errors import APIError, AuthError, SessionError
 from .session import Session
-from ..endpoints import GOOGLE_BASE_URL, PASSWORD_VERIFY
+from ..endpoints import GOOGLE_BASE_URL, PASSWORD_VERIFY, GOOGLE_TOKEN_BASE_URL, TOKEN
 from .functions import HEADERS
 from .api_wrapper import APIWrapper
 
@@ -61,7 +61,9 @@ async def async_update_access_token(self) -> bool:
 
         try:
             _LOGGER.debug('Updating access token')
-            access_token_updated: bool = await self.__update_access_token()
+            access_token_updated: bool = await self.__update_access_token(
+                refresh=self.access_token_expired()
+            )
 
             _LOGGER.debug(
                 "Updated access token. New expiration: %s",
@@ -90,23 +92,45 @@ async def async_update_access_token(self) -> bool:
 
     async def __update_access_token(self, refresh: bool = False) -> bool:
         return_value = False
+        id_token_response = 'idToken'
+        refresh_token_response = 'refreshToken'
+        expires_in_response = 'expiresIn'
 
         try:
             wrapper = APIWrapper(session=self._session)
+
+            if refresh:
+                _LOGGER.debug('Refreshing access token')
+                headers = HEADERS.copy()
+                headers["Content-type"] = 'application/x-www-form-urlencoded'
+
+                url = f"{GOOGLE_TOKEN_BASE_URL}{TOKEN}"
+                body = f"grant_type=refresh_token&refresh_token={self.refresh_token}"
+
+                id_token_response = 'id_token'
+                refresh_token_response = 'refresh_token'
+                expires_in_response = 'expires_in'
+            else:
+                _LOGGER.debug('Getting a new access token')
+
+                url = f"{GOOGLE_BASE_URL}{PASSWORD_VERIFY}"
+                body = {"email": self.email, "returnSecureToken": True, "password": self.password}
+                headers = HEADERS.copy()
+
             response = await wrapper.post(
-                url=f"{GOOGLE_BASE_URL}{PASSWORD_VERIFY}",
-                body={"email": self.email, "returnSecureToken": True, "password": self.password},
-                headers=HEADERS,
+                url=url,
+                body=body,
+                headers=headers,
                 exception_class=AuthError)
 
             if response.status != 200:
                 await self.__handle_response_error(response, AuthError)
 
             json = await response.json()
-            self.access_token = json["idToken"]
-            self.refresh_token = json["refreshToken"]
+            self.access_token = json[id_token_response]
+            self.refresh_token = json[refresh_token_response]
             self.access_token_expiry = datetime.now() + timedelta(
-                seconds=int(json["expiresIn"]) - 10
+                seconds=int(json[expires_in_response]) - 10
             )
             return_value = True
 
@@ -134,4 +158,7 @@ async def __handle_response_error(self, response, error_class):
         status = response.status
         response = await response.text()
 
+        if self._http_debug:
+            _LOGGER.debug(response)
+
         raise error_class(status, response)
diff --git a/podpointclient/version.py b/podpointclient/version.py
index 7527438..de4174c 100644
--- a/podpointclient/version.py
+++ b/podpointclient/version.py
@@ -1,3 +1,3 @@
 """Version for the podpointclient library"""
 
-__version__ = "1.4.3"
+__version__ = "1.5.0"


Test patch:
diff --git a/tests/fixtures/refresh.json b/tests/fixtures/refresh.json
new file mode 100644
index 0000000..639b8da
--- /dev/null
+++ b/tests/fixtures/refresh.json
@@ -0,0 +1,8 @@
+{
+  "access_token": "1234",
+  "id_token": "1234",
+  "refresh_token": "1234",
+  "expires_in": "1234",
+  "token_type": "Bearer",
+  "user_id": "11111111-1111-1111-1111-11111111111"
+}
\ No newline at end of file
diff --git a/tests/helpers.py b/tests/helpers.py
index cb76754..1debf66 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -1,7 +1,7 @@
 import json
 import os
 
-from podpointclient.endpoints import GOOGLE_BASE_URL, PASSWORD_VERIFY, API_BASE_URL, AUTH, CHARGE_SCHEDULES, PODS, SESSIONS, UNITS, USERS, CHARGES, FIRMWARE
+from podpointclient.endpoints import GOOGLE_BASE_URL, PASSWORD_VERIFY, API_BASE_URL, AUTH, CHARGE_SCHEDULES, PODS, SESSIONS, UNITS, USERS, CHARGES, FIRMWARE, GOOGLE_TOKEN_BASE_URL, TOKEN
 
 class Mocks:
     def __init__(self, m = None) -> None:
@@ -9,6 +9,7 @@ def __init__(self, m = None) -> None:
 
     def happy_path(self, include_timestamp=False):
         auth_response = self.auth_response()
+        refresh_response = self.refresh_response()
         session_response = self.session_response()
         pods_response = self.pods_response()
         pods_response_schedule_disabled = self.pods_response_schedule_disabled()
@@ -25,6 +26,7 @@ def happy_path(self, include_timestamp=False):
             question_timestamp = f'?{timestamp}'
 
         self.m.post(f'{GOOGLE_BASE_URL}{PASSWORD_VERIFY}', payload=auth_response)
+        self.m.post(f'{GOOGLE_TOKEN_BASE_URL}{TOKEN}', payload=refresh_response)
         self.m.post(f'{API_BASE_URL}{SESSIONS}', payload=session_response)
         self.m.get(f'{API_BASE_URL}{USERS}/1234{PODS}?perpage=1&page=1{and_timestamp}', payload=pods_response)
         self.m.get(f'{API_BASE_URL}{USERS}/1234{PODS}?perpage=5&page=1&include=statuses,price,model,unit_connectors,charge_schedules,charge_override{and_timestamp}', payload=pods_response)
@@ -37,6 +39,9 @@ def happy_path(self, include_timestamp=False):
     def auth_response(self):
         return self.__json_load_fixture('auth')
 
+    def refresh_response(self):
+        return self.__json_load_fixture('refresh')
+
     def session_response(self):
         return self.__json_load_fixture('session')
 
diff --git a/tests/test_api_wrapper.py b/tests/test_api_wrapper.py
index fedfcd9..7ce6952 100644
--- a/tests/test_api_wrapper.py
+++ b/tests/test_api_wrapper.py
@@ -17,7 +17,7 @@ async def test_get(aiohttp_client):
         assert "OK" == await result.text()
 
 @pytest.mark.asyncio
-async def test_post(aiohttp_client):
+async def test_post_with_dictionary_data(aiohttp_client):
   with aioresponses() as m:
     m.post('https://google.com/api/v1/test', body="OK")
 
@@ -27,6 +27,17 @@ async def test_post(aiohttp_client):
         assert 200 == result.status
         assert "OK" == await result.text()
 
+@pytest.mark.asyncio
+async def test_post_with_string_data(aiohttp_client):
+  with aioresponses() as m:
+    m.post('https://google.com/api/v1/test', body="OK")
+
+    async with aiohttp.ClientSession() as session:
+      wrapper = APIWrapper(session)
+      async with await wrapper.post("https://google.com/api/v1/test", body="foo", headers={}) as result:
+        assert 200 == result.status
+        assert "OK" == await result.text()
+
 @pytest.mark.asyncio
 async def test_put(aiohttp_client):
   with aioresponses() as m:
diff --git a/tests/test_auth.py b/tests/test_auth.py
index d4f87a6..b906ffb 100644
--- a/tests/test_auth.py
+++ b/tests/test_auth.py
@@ -8,7 +8,7 @@
 from aioresponses import aioresponses
 import logging
 
-from podpointclient.endpoints import GOOGLE_BASE_URL, PASSWORD_VERIFY, API_BASE_URL, API_VERSION, AUTH, SESSIONS
+from podpointclient.endpoints import GOOGLE_BASE_URL, PASSWORD_VERIFY, API_BASE_URL, API_VERSION, AUTH, SESSIONS, GOOGLE_TOKEN_BASE_URL, TOKEN
 
 import pytest
 
@@ -149,6 +149,14 @@ async def test_update_access_token_when_not_set(aiohttp_client):
         "idToken": "1234",
         "refreshToken": "1234"
     }
+    refresh_response = {
+        "access_token": "1234",
+        "id_token": "1234",
+        "refresh_token": "1234",
+        "expires_in": "1234",
+        "token_type": "Bearer",
+        "user_id": "11111111-1111-1111-1111-11111111111"
+    }
     session_response = {
         "sessions": {
             "id": "1234",
@@ -157,7 +165,7 @@ async def test_update_access_token_when_not_set(aiohttp_client):
     }
 
     with aioresponses() as m:
-        m.post(f'{GOOGLE_BASE_URL}{PASSWORD_VERIFY}', payload=auth_response)
+        m.post(f'{GOOGLE_TOKEN_BASE_URL}{TOKEN}', payload=refresh_response)
         m.post(f'{API_BASE_URL}{SESSIONS}', payload=session_response)
 
         async with aiohttp.ClientSession() as session:
@@ -179,7 +187,7 @@ async def test_update_access_token_when_token_valid():
 
 async def test_auth_401_error():
     with aioresponses() as m:
-        m.post(f'{GOOGLE_BASE_URL}{PASSWORD_VERIFY}', status=401 , body="foo error")
+        m.post(f'{GOOGLE_TOKEN_BASE_URL}{TOKEN}', status=401 , body="foo error")
 
         async with aiohttp.ClientSession() as session:
             auth = expired_subject(session)
@@ -190,14 +198,17 @@ async def test_auth_401_error():
             assert "Auth Error (401) - foo error" in str(exc_info.value)
 
 async def test_auth_json_error():
-    # MISSING ELEMENT
-    auth_response = {
-        "expiresIn": "1234",
-        "refreshToken": "1234"
+    # MISSING id_token ELEMENT
+    refresh_response = {
+        "access_token": "1234",
+        "refresh_token": "1234",
+        "expires_in": "1234",
+        "token_type": "Bearer",
+        "user_id": "11111111-1111-1111-1111-11111111111"
     }
 
     with aioresponses() as m:
-        m.post(f'{GOOGLE_BASE_URL}{PASSWORD_VERIFY}', payload=auth_response)
+        m.post(f'{GOOGLE_TOKEN_BASE_URL}{TOKEN}', payload=refresh_response)
 
         async with aiohttp.ClientSession() as session:
             auth = expired_subject(session)
@@ -205,17 +216,20 @@ async def test_auth_json_error():
             with pytest.raises(AuthError) as exc_info:   
                 await auth.async_update_access_token()
 
-            assert "Auth Error (200) - Error processing access token response. 'idToken' not found in json." in str(exc_info.value)
+            assert "Auth Error (200) - Error processing access token response. 'id_token' not found in json." in str(exc_info.value)
 
     # INVALID EXPIRES_IN
-    auth_response = {
-        "expiresIn": "F14A3",
-        "idToken": "1234",
-        "refreshToken": "1234"
+    refresh_response = {
+        "access_token": "1234",
+        "refresh_token": "1234",
+        "id_token": "1234",
+        "expires_in": "F14A3",
+        "token_type": "Bearer",
+        "user_id": "11111111-1111-1111-1111-11111111111"
     }
 
     with aioresponses() as m:
-        m.post(f'{GOOGLE_BASE_URL}{PASSWORD_VERIFY}', payload=auth_response)
+        m.post(f'{GOOGLE_TOKEN_BASE_URL}{TOKEN}', payload=refresh_response)
 
         async with aiohttp.ClientSession() as session:
             auth = expired_subject(session)
@@ -227,14 +241,17 @@ async def test_auth_json_error():
 
 
 async def test_session_401_error():
-    auth_response = {
-        "expiresIn": "1234",
-        "idToken": "1234",
-        "refreshToken": "1234"
+    refresh_response = {
+        "access_token": "1234",
+        "id_token": "1234",
+        "refresh_token": "1234",
+        "expires_in": "1234",
+        "token_type": "Bearer",
+        "user_id": "11111111-1111-1111-1111-11111111111"
     }
 
     with aioresponses() as m:
-        m.post(f'{GOOGLE_BASE_URL}{PASSWORD_VERIFY}', payload=auth_response)
+        m.post(f'{GOOGLE_TOKEN_BASE_URL}{TOKEN}', payload=refresh_response)
         m.post(f'{API_BASE_URL}{SESSIONS}', status=401, body="bar error")
 
         async with aiohttp.ClientSession() as session:
@@ -246,10 +263,13 @@ async def test_session_401_error():
             assert "Session Error (401) - bar error" in str(exc_info.value)
 
 async def test_session_json_error():
-    auth_response = {
-        "idToken": "1234",
-        "expiresIn": "1234",
-        "refreshToken": "1234"
+    refresh_response = {
+        "access_token": "1234",
+        "id_token": "1234",
+        "refresh_token": "1234",
+        "expires_in": "1234",
+        "token_type": "Bearer",
+        "user_id": "11111111-1111-1111-1111-11111111111"
     }
     session_response = {
         "sessions": {
@@ -258,7 +278,7 @@ async def test_session_json_error():
     }
 
     with aioresponses() as m:
-        m.post(f'{GOOGLE_BASE_URL}{PASSWORD_VERIFY}', payload=auth_response)
+        m.post(f'{GOOGLE_TOKEN_BASE_URL}{TOKEN}', payload=refresh_response)
         m.post(f'{API_BASE_URL}{SESSIONS}', payload=session_response)
 
         async with aiohttp.ClientSession() as session: