From 8d6b9c7735dac3aaaec3151aa4437f39b7fbeed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Bruguera=20Mic=C3=B3?= Date: Mon, 1 Jan 2024 15:45:28 +0000 Subject: [PATCH 1/4] Remove login with email and password workaround. This workaround is no longer needed, as it appears Todoist has fixed it from their side (they accept download with the usual Bearer token now). Closes #1 --- README.md | 4 +- full_offline_backup_for_todoist/controller.py | 4 +- full_offline_backup_for_todoist/frontend.py | 19 +++---- full_offline_backup_for_todoist/runtime.py | 9 +-- .../tests/test_frontend.py | 4 +- .../tests/test_integration.py | 4 +- .../tests/test_runtime.py | 2 +- .../url_downloader.py | 56 ------------------- 8 files changed, 14 insertions(+), 88 deletions(-) diff --git a/README.md b/README.md index b92eb26..b559b2c 100644 --- a/README.md +++ b/README.md @@ -46,8 +46,6 @@ To create a backup from Todoist's servers, including attachments, and with traci ``python3 -m full_offline_backup_for_todoist --verbose download --with-attachments`` -**NOTE:** You will also be asked to for your Todoist email and password. This is **required** to download the attachments, as a workaround due to security restrictions introduced by Todoist in 2018 (see [issue #1](https://github.com/joanbm/full-offline-backup-for-todoist/issues/1)). As of today, there is no official way provided by Todoist to automate attachment download, and the current workaround may break at any time. - Print full help: ``python3 -m full_offline_backup_for_todoist -h`` @@ -58,7 +56,7 @@ The easiest way to get one is to open the **web version of Todoist**, go to the ## How can I automate the backup process? -To automate the backup process, you can use any automation tool you want (e.g. cron, Jenkins) that can run the utility. In order to pass the credentials non-interactively, you can set the `TODOIST_TOKEN`, `TODOIST_EMAIL` and `TODOIST_PASSWORD` environment variables before running it from your automation tool. +To automate the backup process, you can use any automation tool you want (e.g. cron, Jenkins) that can run the utility. In order to pass the credentials non-interactively, you can set the `TODOIST_TOKEN` environment variable before running it from your automation tool. # Disclaimer diff --git a/full_offline_backup_for_todoist/controller.py b/full_offline_backup_for_todoist/controller.py index 0735a8b..7b64bba 100644 --- a/full_offline_backup_for_todoist/controller.py +++ b/full_offline_backup_for_todoist/controller.py @@ -2,7 +2,7 @@ """ Provides frontend-independent access to the functions of the interface """ from abc import ABCMeta, abstractmethod -from typing import NamedTuple, Optional +from typing import NamedTuple from .tracer import Tracer from .virtual_fs import VirtualFs from .backup_downloader import TodoistBackupDownloader @@ -11,8 +11,6 @@ class TodoistAuth(NamedTuple): """ Represents the properties of a Todoist attachment """ token: str - email: Optional[str] - password: Optional[str] class ControllerDependencyInjector(metaclass=ABCMeta): """ Rudimentary dependency injection container for the controller """ diff --git a/full_offline_backup_for_todoist/frontend.py b/full_offline_backup_for_todoist/frontend.py index 0883a2c..aa83953 100644 --- a/full_offline_backup_for_todoist/frontend.py +++ b/full_offline_backup_for_todoist/frontend.py @@ -22,21 +22,17 @@ def __add_authorization_group(parser: argparse.ArgumentParser) -> None: token_group = parser.add_mutually_exclusive_group() token_group.add_argument("--token-file", type=str, help="path to a file containing the Todoist token") - parser.add_argument("--email", type=str, help="Todoist email") - parser.add_argument("--password-file", type=str, - help="path to a file containing the Todoist password") - # Those options are deprecated, since they are easy to use incorrectly - # (e.g. by getting the password logged to the history file) + # This option is deprecated, since it is easy to use incorrectly + # (e.g. by getting the token logged to the history file) # Using either interactive console input, environment variables or files is recommended token_group.add_argument("--token", type=str, help=argparse.SUPPRESS) - parser.add_argument("--password", type=str, help=argparse.SUPPRESS) def __parse_command_line_args(self, prog: str, arguments: List[str]) -> argparse.Namespace: epilog_str = f"Example: {prog} download\n" epilog_str += "(The necessary credentials will be asked through the command line.\n" epilog_str += " If you wish to automate backups, credentials can be passed through the\n" - epilog_str += " TODOIST_TOKEN, TODOIST_EMAIL and TODOIST_PASSWORD environment variables)" + epilog_str += " TODOIST_TOKEN environment variable)" parser = argparse.ArgumentParser(prog=prog, formatter_class=argparse.RawTextHelpFormatter, epilog=epilog_str) parser.add_argument("--verbose", action="store_true", help="print details to console") @@ -93,11 +89,10 @@ def get_credential(opt_file: Optional[str], opt_direct: Optional[str], token = get_credential(args.token_file, args.token, "TODOIST_TOKEN", "Todoist token (from https://todoist.com/app/settings/integrations/developer)", sensitive=True) - email = get_credential(None, args.email, "TODOIST_EMAIL", "Todoist email https://todoist.com/app/settings/account", - sensitive=False) if args.with_attachments else None - password = get_credential(None, args.password, "TODOIST_PASSWORD", "Todoist password (can be empty)", - sensitive=True) if args.with_attachments else None - return TodoistAuth(token, email, password) + for deprecated_env in ("TODOIST_EMAIL", "TODOIST_PASSWORD"): + if deprecated_env in environment: + print(f"WARNING: The {deprecated_env} environment variable is no longer necessary") + return TodoistAuth(token) def handle_download(self, args: argparse.Namespace, environment: Mapping[str, str]) -> None: """ Handles the download subparser with the specified command line arguments """ diff --git a/full_offline_backup_for_todoist/runtime.py b/full_offline_backup_for_todoist/runtime.py index 549d9a9..5c3ce33 100644 --- a/full_offline_backup_for_todoist/runtime.py +++ b/full_offline_backup_for_todoist/runtime.py @@ -6,7 +6,7 @@ from .backup_downloader import TodoistBackupDownloader from .backup_attachments_downloader import TodoistBackupAttachmentsDownloader from .tracer import Tracer, ConsoleTracer, NullTracer -from .url_downloader import URLDownloader, URLLibURLDownloader, TodoistAuthURLDownloader +from .url_downloader import URLLibURLDownloader class RuntimeControllerDependencyInjector(ControllerDependencyInjector): """ Implementation of the dependency injection container for the actual runtime objects """ @@ -14,12 +14,7 @@ class RuntimeControllerDependencyInjector(ControllerDependencyInjector): def __init__(self, auth: TodoistAuth, verbose: bool): super().__init__(auth, verbose) self.__tracer = ConsoleTracer() if verbose else NullTracer() - urldownloader: URLDownloader - if auth.email and auth.password: - self.__tracer.trace("NOTE: Using authentication workaround to download the attachments") - urldownloader = TodoistAuthURLDownloader(self.__tracer, auth.email, auth.password) - else: - urldownloader = URLLibURLDownloader(self.__tracer) + urldownloader = URLLibURLDownloader(self.__tracer) todoist_api = TodoistApi(auth.token, self.__tracer, urldownloader) self.__backup_downloader = TodoistBackupDownloader(self.__tracer, todoist_api) self.__backup_attachments_downloader = TodoistBackupAttachmentsDownloader( diff --git a/full_offline_backup_for_todoist/tests/test_frontend.py b/full_offline_backup_for_todoist/tests/test_frontend.py index 5c12141..d76aeab 100644 --- a/full_offline_backup_for_todoist/tests/test_frontend.py +++ b/full_offline_backup_for_todoist/tests/test_frontend.py @@ -33,9 +33,7 @@ def test_on_download_with_attachments_calls_controller_with_attachments(): # Act frontend.run("util", ["download", "--with-attachments"], - {"TODOIST_TOKEN": "1234", - "TODOIST_EMAIL": "asd@asd.asd", - "TODOIST_PASSWORD": "1234"}) + {"TODOIST_TOKEN": "1234"}) # Assert controller.download.assert_called_with(ANY, with_attachments=True) diff --git a/full_offline_backup_for_todoist/tests/test_integration.py b/full_offline_backup_for_todoist/tests/test_integration.py index 838c8f1..05712c8 100644 --- a/full_offline_backup_for_todoist/tests/test_integration.py +++ b/full_offline_backup_for_todoist/tests/test_integration.py @@ -86,9 +86,7 @@ def __compare_zip_files(self, zip_path_1, zip_path_2): self.assertEqual(content_1, content_2) @patch.object(sys, 'argv', ["program", "download", "--with-attachments"]) - @patch.object(os, 'environ', {"TODOIST_TOKEN": "mysecrettoken", - "TODOIST_EMAIL": "asd@asd.asd", - "TODOIST_PASSWORD": "mysecretpassword"}) + @patch.object(os, 'environ', {"TODOIST_TOKEN": "mysecrettoken"}) @patch.object(urllib.request.OpenerDirector, 'open', autospec=True) @unittest.skip("Not yet adapted to mock the attachment download workaround") def test_integration_download_with_attachments(self, mock_opener_open): diff --git a/full_offline_backup_for_todoist/tests/test_runtime.py b/full_offline_backup_for_todoist/tests/test_runtime.py index cf63617..23a41be 100644 --- a/full_offline_backup_for_todoist/tests/test_runtime.py +++ b/full_offline_backup_for_todoist/tests/test_runtime.py @@ -14,7 +14,7 @@ def test_runtime_dependency_injector_caches_values(self): # Arrange # Act - runtimedi = RuntimeControllerDependencyInjector(TodoistAuth("1234", None, None), False) + runtimedi = RuntimeControllerDependencyInjector(TodoistAuth("1234"), False) tracer1 = runtimedi.tracer tracer2 = runtimedi.tracer backup_downloader1 = runtimedi.backup_downloader diff --git a/full_offline_backup_for_todoist/url_downloader.py b/full_offline_backup_for_todoist/url_downloader.py index 3e5b58d..9f67494 100644 --- a/full_offline_backup_for_todoist/url_downloader.py +++ b/full_offline_backup_for_todoist/url_downloader.py @@ -3,7 +3,6 @@ from abc import ABCMeta, abstractmethod import urllib.request import urllib.parse -import http.cookiejar import time from typing import cast, Dict, Optional from .tracer import Tracer @@ -61,58 +60,3 @@ class URLLibURLDownloader(URLDownloader): def get(self, url: str, data: Optional[Dict[str, str]]=None) -> bytes: opener = self._build_opener_with_app_useragent() return self._download_with_retry(opener, url, data) - -class TodoistAuthURLDownloader(URLDownloader): - """ Implementation of a class to download the contents of an URL through URLLib, - authenticating before with Todoist's servers using a username/password """ - - URL_SHOWLOGIN = 'https://todoist.com/Users/showLogin' - URL_POSTLOGIN = 'https://todoist.com/Users/login' - - LOGIN_PARAM_CSRF = "csrf" - LOGIN_PARAM_EMAIL = "email" - LOGIN_PARAM_PASSWORD = "password" - - __email: str - __password: str - __opener: Optional[urllib.request.OpenerDirector] - - def __init__(self, tracer: Tracer, email: str, password: str): - super().__init__(tracer) - self.__email = email - self.__password = password - self.__opener = None - - def get(self, url: str, data: Optional[Dict[str, str]]=None) -> bytes: - if not self.__opener: - # Set up a cookie jar, to gather the login's cookies - cookiejar = http.cookiejar.CookieJar() - cookie_process = urllib.request.HTTPCookieProcessor(cookiejar) - self.__opener = self._build_opener_with_app_useragent(cookie_process) - - self._tracer.trace("Auth Step 1: Get CSRF token") - - # Ping the login page, in order to get a CSRF token as a cookie - with self.__opener.open(TodoistAuthURLDownloader.URL_SHOWLOGIN) as _: - pass - - self._tracer.trace("Auth Step 2: Building login request params") - - # Build the parameters (CSRF, email and password) for the login POST request - csrf_value = next(c.value for c in cookiejar - if c.name == TodoistAuthURLDownloader.LOGIN_PARAM_CSRF) - params = { - TodoistAuthURLDownloader.LOGIN_PARAM_CSRF: csrf_value, - TodoistAuthURLDownloader.LOGIN_PARAM_EMAIL: self.__email, - TodoistAuthURLDownloader.LOGIN_PARAM_PASSWORD: self.__password} - params_str = urllib.parse.urlencode(params).encode('utf-8') - - self._tracer.trace("Auth Step 3: Send login request") - - # Send the login POST request, which will give us our identifier cookie - with self.__opener.open(TodoistAuthURLDownloader.URL_POSTLOGIN, params_str) as _: - pass - - self._tracer.trace("Auth completed") - - return self._download_with_retry(self.__opener, url, data) From bd6dc201fb202d1b1973335e2c57cdae10c97d3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Bruguera=20Mic=C3=B3?= Date: Mon, 1 Jan 2024 15:46:31 +0000 Subject: [PATCH 2/4] Fix test disabled due to email/password login workaround. --- full_offline_backup_for_todoist/frontend.py | 5 +++-- full_offline_backup_for_todoist/tests/test_integration.py | 5 ++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/full_offline_backup_for_todoist/frontend.py b/full_offline_backup_for_todoist/frontend.py index aa83953..a237b60 100644 --- a/full_offline_backup_for_todoist/frontend.py +++ b/full_offline_backup_for_todoist/frontend.py @@ -87,11 +87,12 @@ def get_credential(opt_file: Optional[str], opt_direct: Optional[str], return environment[env_var] return getpass.getpass(prompt + ": ") if sensitive else input(prompt + ": ") - token = get_credential(args.token_file, args.token, "TODOIST_TOKEN", - "Todoist token (from https://todoist.com/app/settings/integrations/developer)", sensitive=True) for deprecated_env in ("TODOIST_EMAIL", "TODOIST_PASSWORD"): if deprecated_env in environment: print(f"WARNING: The {deprecated_env} environment variable is no longer necessary") + + token = get_credential(args.token_file, args.token, "TODOIST_TOKEN", + "Todoist token (from https://todoist.com/app/settings/integrations/developer)", sensitive=True) return TodoistAuth(token) def handle_download(self, args: argparse.Namespace, environment: Mapping[str, str]) -> None: diff --git a/full_offline_backup_for_todoist/tests/test_integration.py b/full_offline_backup_for_todoist/tests/test_integration.py index 05712c8..8efd56c 100644 --- a/full_offline_backup_for_todoist/tests/test_integration.py +++ b/full_offline_backup_for_todoist/tests/test_integration.py @@ -45,9 +45,9 @@ def setUp(self): Path(self.__get_test_file("sources/Project_2181147712.csv")).read_bytes(), ("POST", "/https://api.todoist.com/sync/v9/templates/export_as_file", b"project_id=2181147713", 'mysecrettoken'): Path(self.__get_test_file("sources/Project_2181147713.csv")).read_bytes(), - ("GET, /https://d1x0mwiac2rqwt.cloudfront.net/g75-kL8pwVYNObSczLnVXe4FIyJd8YQL6b8yCilGyix09bMdJmxbtrGMW9jIeIwJ/by/16542905/as/bug.txt", None, None): + ("GET", "/https://d1x0mwiac2rqwt.cloudfront.net/g75-kL8pwVYNObSczLnVXe4FIyJd8YQL6b8yCilGyix09bMdJmxbtrGMW9jIeIwJ/by/16542905/as/bug.txt", None, None): Path(self.__get_test_file("sources/bug.txt")).read_bytes(), - ("GET, /https://d1x0mwiac2rqwt.cloudfront.net/s0snyb7n9tJXYijOK2LV6hjVar4YUkwYbHv3PBFYM-N4nJEtujC046OlEdZpKfZm/by/16542905/as/sample_image.png", None, None): + ("GET", "/https://d1x0mwiac2rqwt.cloudfront.net/s0snyb7n9tJXYijOK2LV6hjVar4YUkwYbHv3PBFYM-N4nJEtujC046OlEdZpKfZm/by/16542905/as/sample_image.png", None, None): Path(self.__get_test_file("sources/sample_image.png")).read_bytes(), } @@ -88,7 +88,6 @@ def __compare_zip_files(self, zip_path_1, zip_path_2): @patch.object(sys, 'argv', ["program", "download", "--with-attachments"]) @patch.object(os, 'environ', {"TODOIST_TOKEN": "mysecrettoken"}) @patch.object(urllib.request.OpenerDirector, 'open', autospec=True) - @unittest.skip("Not yet adapted to mock the attachment download workaround") def test_integration_download_with_attachments(self, mock_opener_open): """ Integration test for downloading the backup with attachments """ From 5cef0b0502f8f1c47fa11dc961484048b9a9bdc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Bruguera=20Mic=C3=B3?= Date: Mon, 1 Jan 2024 15:52:49 +0000 Subject: [PATCH 3/4] Refactor / simplify unused code. --- full_offline_backup_for_todoist/frontend.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/full_offline_backup_for_todoist/frontend.py b/full_offline_backup_for_todoist/frontend.py index a237b60..f622da6 100644 --- a/full_offline_backup_for_todoist/frontend.py +++ b/full_offline_backup_for_todoist/frontend.py @@ -66,9 +66,9 @@ def __huge_warning(text: str) -> None: @staticmethod def __get_auth(args: argparse.Namespace, environment: Mapping[str, str]) -> TodoistAuth: def get_credential(opt_file: Optional[str], opt_direct: Optional[str], - env_var: str, prompt: str, sensitive: bool) -> str: + env_var: str, prompt: str) -> str: if opt_file: - if sensitive and os.name == "posix": # OpenSSH-like check + if os.name == "posix": # OpenSSH-like check file_stat = os.stat(opt_file) if file_stat.st_uid == os.getuid() and file_stat.st_mode & 0o077 != 0: ConsoleFrontend.__huge_warning( @@ -76,7 +76,7 @@ def get_credential(opt_file: Optional[str], opt_direct: Optional[str], "accessible by other users is deprecated.") return Path(opt_file).read_text('utf-8') - if sensitive and opt_direct: + if opt_direct: ConsoleFrontend.__huge_warning( "WARNING: Passing credentials through the command line is deprecated.\n" f" Pass it through the {env_var} environment variable,\n" @@ -85,14 +85,14 @@ def get_credential(opt_file: Optional[str], opt_direct: Optional[str], if env_var in environment: return environment[env_var] - return getpass.getpass(prompt + ": ") if sensitive else input(prompt + ": ") + return getpass.getpass(prompt + ": ") for deprecated_env in ("TODOIST_EMAIL", "TODOIST_PASSWORD"): if deprecated_env in environment: print(f"WARNING: The {deprecated_env} environment variable is no longer necessary") token = get_credential(args.token_file, args.token, "TODOIST_TOKEN", - "Todoist token (from https://todoist.com/app/settings/integrations/developer)", sensitive=True) + "Todoist token (from https://todoist.com/app/settings/integrations/developer)") return TodoistAuth(token) def handle_download(self, args: argparse.Namespace, environment: Mapping[str, str]) -> None: From 9c1205ad00a17886b6c2f4ef08949bc1fadf8164 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Bruguera=20Mic=C3=B3?= Date: Mon, 1 Jan 2024 15:54:36 +0000 Subject: [PATCH 4/4] Refactor URLLibURLDownloader with better separation / abstraction. --- .../url_downloader.py | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/full_offline_backup_for_todoist/url_downloader.py b/full_offline_backup_for_todoist/url_downloader.py index 9f67494..94dfab6 100644 --- a/full_offline_backup_for_todoist/url_downloader.py +++ b/full_offline_backup_for_todoist/url_downloader.py @@ -13,11 +13,23 @@ class URLDownloader(metaclass=ABCMeta): """ Implementation of a class to download the contents of an URL """ _tracer: Tracer - __bearer_token: Optional[str] + _bearer_token: Optional[str] def __init__(self, tracer: Tracer): self._tracer = tracer - self.__bearer_token = None + self._bearer_token = None + + def set_bearer_token(self, bearer_token: Optional[str]) -> None: + """ Sets the value of the 'Authorization: Bearer XXX' HTTP header """ + self._bearer_token = bearer_token + + @abstractmethod + def get(self, url: str, data: Optional[Dict[str, str]]=None) -> bytes: + """ Download the contents of the specified URL with a GET request. + You can specify any additional data parameters to pass to the destination. """ + +class URLLibURLDownloader(URLDownloader): + """ Implementation of a class to download the contents of an URL through URLLib """ def _download(self, opener: urllib.request.OpenerDirector, url: str, data: Optional[Dict[str, str]]=None) -> bytes: @@ -38,25 +50,13 @@ def _download_with_retry(self, opener: urllib.request.OpenerDirector, url: str, return self._download(opener, url, data) - def set_bearer_token(self, bearer_token: Optional[str]) -> None: - """ Sets the value of the 'Authorization: Bearer XXX' HTTP header """ - self.__bearer_token = bearer_token - - @abstractmethod - def get(self, url: str, data: Optional[Dict[str, str]]=None) -> bytes: - """ Download the contents of the specified URL with a GET request. - You can specify any additional data parameters to pass to the destination. """ - def _build_opener_with_app_useragent( self, *handlers: urllib.request.BaseHandler) -> urllib.request.OpenerDirector: opener = urllib.request.build_opener(*handlers) opener.addheaders = ([('User-agent', 'full-offline-backup-for-todoist')] + - ([('Authorization', 'Bearer ' + self.__bearer_token)] if self.__bearer_token else [])) + ([('Authorization', 'Bearer ' + self._bearer_token)] if self._bearer_token else [])) return opener -class URLLibURLDownloader(URLDownloader): - """ Implementation of a class to download the contents of an URL through URLLib """ - def get(self, url: str, data: Optional[Dict[str, str]]=None) -> bytes: opener = self._build_opener_with_app_useragent() return self._download_with_retry(opener, url, data)