Repository: 12rambau/sepal_ui

Issue:
get_mosaics from planet api fails
`get_mosaics` --and probably-- `get_quads` fails when the authentication process is done `from_login`... test has been passed because we are only testing the initialization of the `Planet` `from_key` but not to get elements from it

Discussion:

Patch:
diff --git a/sepal_ui/message/en/locale.json b/sepal_ui/message/en/locale.json
index abdea912..b65a232d 100644
--- a/sepal_ui/message/en/locale.json
+++ b/sepal_ui/message/en/locale.json
@@ -85,14 +85,17 @@
     "exception": {
       "empty": "Please fill the required field(s).",
       "invalid": "Invalid email or password",
-      "nosubs": "Your credentials do not have any valid planet subscription."
+      "nosubs": "Your credentials do not have any valid planet subscription.",
+      "no_secret_file": "The credentials file does not exist, use a different login method."
     },
     "widget": {
       "username": "Planet username",
       "password": "Planet password",
       "apikey": "Planet API key",
+      "store": "Remember credentials file in the session.",
       "method": {
         "label": "Login method",
+        "from_file": "From saved credentials",
         "credentials": "Credentials",
         "api_key": "Planet API key"
       }
diff --git a/sepal_ui/planetapi/planet_model.py b/sepal_ui/planetapi/planet_model.py
index 7aee2ed4..c3939499 100644
--- a/sepal_ui/planetapi/planet_model.py
+++ b/sepal_ui/planetapi/planet_model.py
@@ -6,8 +6,8 @@ from typing import Dict, List, Optional, Union
 
 import nest_asyncio
 import planet.data_filter as filters
-import requests
 import traitlets as t
+from deprecated.sphinx import deprecated
 from planet import DataClient
 from planet.auth import Auth
 from planet.exceptions import NoPermission
@@ -21,7 +21,6 @@ nest_asyncio.apply()
 
 
 class PlanetModel(Model):
-
     SUBS_URL: str = (
         "https://api.planet.com/auth/v1/experimental/public/my/subscriptions"
     )
@@ -56,11 +55,18 @@ class PlanetModel(Model):
         if credentials:
             self.init_session(credentials)
 
-    def init_session(self, credentials: Union[str, List[str]]) -> None:
+    @deprecated(
+        version="3.0",
+        reason="credentials member is deprecated, use self.auth._key instead",
+    )
+    def init_session(
+        self, credentials: Union[str, List[str]], write_secrets: bool = False
+    ) -> None:
         """Initialize planet client with api key or credentials. It will handle errors.
 
         Args:
-            credentials: planet API key or username and password pair of planet explorer.
+            credentials: planet API key, username and password pair or a secrets planet.json file.
+            write_secrets: either to write the credentials in the secret file or not. Defaults to True.
         """
         if isinstance(credentials, str):
             credentials = [credentials]
@@ -70,13 +76,21 @@ class PlanetModel(Model):
 
         if len(credentials) == 2:
             self.auth = Auth.from_login(*credentials)
+
+        # Check if the str is a path to a secret file
+        elif len(credentials) == 1 and credentials[0].endswith(".json"):
+            self.auth = Auth.from_file(credentials[0])
+
         else:
             self.auth = Auth.from_key(credentials[0])
 
-        self.credentials = credentials
+        self.credentials = self.auth._key
         self.session = Session(auth=self.auth)
         self._is_active()
 
+        if self.active and write_secrets:
+            self.auth.store()
+
         return
 
     def _is_active(self) -> None:
@@ -213,10 +227,11 @@ class PlanetModel(Model):
                     "quad_download": true
                 }
         """
-        url = "https://api.planet.com/basemaps/v1/mosaics?api_key={}"
-        res = requests.get(url.format(self.credentials[0]))
+        mosaics_url = "https://api.planet.com/basemaps/v1/mosaics"
+        request = self.session.request("GET", mosaics_url)
+        response = asyncio.run(request)
 
-        return res.json().get("mosaics", [])
+        return response.json().get("mosaics", [])
 
     def get_quad(self, mosaic: dict, quad_id: str) -> dict:
         """Get a quad response for a specific mosaic and quad.
@@ -245,10 +260,13 @@ class PlanetModel(Model):
                     "percent_covered": 100
                 }
         """
-        url = "https://api.planet.com/basemaps/v1/mosaics/{}/quads/{}?api_key={}"
-        res = requests.get(url.format(mosaic["id"], quad_id, self.credentials[0]))
+        quads_url = "https://api.planet.com/basemaps/v1/mosaics/{}/quads/{}"
+        quads_url = quads_url.format(mosaic["id"], quad_id)
+
+        request = self.session.request("GET", quads_url)
+        response = asyncio.run(request)
 
-        return res.json() or {}
+        return response.json() or {}
 
     @staticmethod
     def search_status(d: dict) -> List[Dict[str, bool]]:
diff --git a/sepal_ui/planetapi/planet_view.py b/sepal_ui/planetapi/planet_view.py
index 1de11832..72f3d771 100644
--- a/sepal_ui/planetapi/planet_view.py
+++ b/sepal_ui/planetapi/planet_view.py
@@ -1,5 +1,6 @@
 """The ``Card`` widget to use in application to interface with Planet."""
 
+from pathlib import Path
 from typing import Optional
 
 import ipyvuetify as v
@@ -12,7 +13,6 @@ from sepal_ui.scripts.decorator import loading_button
 
 
 class PlanetView(sw.Layout):
-
     planet_model: Optional[PlanetModel] = None
     "Backend model to manipulate interface actions"
 
@@ -47,7 +47,7 @@ class PlanetView(sw.Layout):
     ):
         """Stand-alone interface to capture planet lab credentials.
 
-        It also validate its  subscription and connect to the client stored in the model.
+        It also validate its  subscription and connect to the client from_file in the model.
 
         Args:
             btn (sw.Btn, optional): Button to trigger the validation process in the associated model.
@@ -67,18 +67,27 @@ class PlanetView(sw.Layout):
         )
         self.w_password = sw.PasswordField(label=ms.planet.widget.password)
         self.w_key = sw.PasswordField(label=ms.planet.widget.apikey, v_model="").hide()
+        self.w_secret_file = sw.TextField(
+            label=ms.planet.widget.store,
+            v_model=str(Path.home() / ".planet.json"),
+            readonly=True,
+            class_="mr-2",
+        ).hide()
         self.w_info_view = InfoView(model=self.planet_model)
 
         self.w_method = v.Select(
             label=ms.planet.widget.method.label,
             class_="mr-2",
-            v_model="credentials",
+            v_model="",
             items=[
+                {"value": "from_file", "text": ms.planet.widget.method.from_file},
                 {"value": "credentials", "text": ms.planet.widget.method.credentials},
                 {"value": "api_key", "text": ms.planet.widget.method.api_key},
             ],
         )
 
+        self.w_store = sw.Checkbox(label=ms.planet.widget.store, v_model=True)
+
         w_validation = v.Flex(
             style_="flex-grow: 0 !important;",
             children=[self.btn],
@@ -87,17 +96,22 @@ class PlanetView(sw.Layout):
         self.children = [
             self.w_method,
             sw.Layout(
+                attributes={"id": "planet_credentials"},
                 class_="align-center",
                 children=[
                     self.w_username,
                     self.w_password,
                     self.w_key,
+                    self.w_secret_file,
                 ],
             ),
+            self.w_store,
         ]
 
         if not btn:
-            self.children[-1].set_children(w_validation, "last")
+            self.get_children(attr="id", value="planet_credentials")[0].set_children(
+                w_validation, "last"
+            )
 
         # Set it here to avoid displacements when using button
         self.set_children(self.w_info_view, "last")
@@ -108,36 +122,82 @@ class PlanetView(sw.Layout):
         self.w_method.observe(self._swap_inputs, "v_model")
         self.btn.on_event("click", self.validate)
 
+        self.set_initial_method()
+
+    def validate_secret_file(self) -> None:
+        """Validate the secret file path."""
+        if not Path(self.w_secret_file.v_model).exists():
+            self.w_secret_file.error_messages = [ms.planet.exception.no_secret_file]
+            return False
+
+        self.w_secret_file.error_messages = []
+        return True
+
+    def set_initial_method(self) -> None:
+        """Set the initial method to connect to planet lab."""
+        self.w_method.v_model = (
+            "from_file" if self.validate_secret_file() else "credentials"
+        )
+
     def reset(self) -> None:
         """Empty credentials fields and restart activation mode."""
-        self.w_username.v_model = None
-        self.w_password.v_model = None
-        self.w_key.v_model = None
+        self.w_username.v_model = ""
+        self.w_password.v_model = ""
+        self.w_key.v_model = ""
         self.planet_model.__init__()
 
         return
 
     def _swap_inputs(self, change: dict) -> None:
-        """Swap between credentials and api key inputs."""
+        """Swap between credentials and api key inputs.
+
+        Args:
+            change.new: values of from_file, credentials, api_key
+        """
         self.alert.reset()
         self.reset()
 
-        self.w_username.toggle_viz()
-        self.w_password.toggle_viz()
-        self.w_key.toggle_viz()
+        # small detail, but validate the file every time the method is changed
+        self.validate_secret_file()
+
+        if change["new"] == "credentials":
+            self.w_username.show()
+            self.w_password.show()
+            self.w_secret_file.hide()
+            self.w_store.show()
+            self.w_key.hide()
+
+        elif change["new"] == "api_key":
+            self.w_username.hide()
+            self.w_password.hide()
+            self.w_secret_file.hide()
+            self.w_store.show()
+            self.w_key.show()
+        else:
+            self.w_username.hide()
+            self.w_password.hide()
+            self.w_key.hide()
+            self.w_store.hide()
+            self.w_secret_file.show()
 
         return
 
-    @loading_button(debug=True)
+    @loading_button()
     def validate(self, *args) -> None:
         """Initialize planet client and validate if is active."""
         self.planet_model.__init__()
 
         if self.w_method.v_model == "credentials":
             credentials = [self.w_username.v_model, self.w_password.v_model]
+
+        elif self.w_method.v_model == "api_key":
+            credentials = self.w_key.v_model
+
         else:
-            credentials = [self.w_key.v_model]
+            if not self.validate_secret_file():
+                raise Exception(ms.planet.exception.no_secret_file)
+            credentials = self.w_secret_file.v_model
 
-        self.planet_model.init_session(credentials)
+        self.planet_model.init_session(credentials, write_secrets=self.w_store.v_model)
 
         return


Test patch:
diff --git a/tests/test_planetapi/test_PlanetModel.py b/tests/test_planetapi/test_PlanetModel.py
index 68450a05..3741c62f 100644
--- a/tests/test_planetapi/test_PlanetModel.py
+++ b/tests/test_planetapi/test_PlanetModel.py
@@ -1,6 +1,7 @@
 """Test the planet PlanetModel model."""
 
 import os
+from pathlib import Path
 from typing import Any, Union
 
 import planet
@@ -42,7 +43,7 @@ def test_init(planet_key: str, cred: list) -> None:
 @pytest.mark.skipif("PLANET_API_KEY" not in os.environ, reason="requires Planet")
 @pytest.mark.parametrize("credentials", ["planet_key", "cred"])
 def test_init_client(credentials: Any, request: FixtureRequest) -> None:
-    """Check init the client with 2 methods.
+    """Check init the client with 3 methods.
 
     Args:
         credentials: any credentials as set in the parameters
@@ -53,16 +54,66 @@ def test_init_client(credentials: Any, request: FixtureRequest) -> None:
     planet_model.init_session(request.getfixturevalue(credentials))
     assert planet_model.active is True
 
-    # check the content of cred
-    # I use a proxy to avoid exposing the credentials in the logs
-    cred = request.getfixturevalue(credentials)
-    cred = [cred] if isinstance(cred, str) else cred
-    is_same = planet_model.credentials == cred
-    assert is_same is True, "The credentials are not corresponding"
+    assert (
+        planet_model.auth._key == planet_model.credentials
+    ), "The credentials are not corresponding"
 
     with pytest.raises(Exception):
         planet_model.init_session("wrongkey")
 
+    with pytest.raises(Exception):
+        planet_model.init_session(["wrongkey", "credentials"])
+
+    return
+
+
+@pytest.mark.skipif("PLANET_API_KEY" not in os.environ, reason="requires Planet")
+def test_init_with_file(planet_key) -> None:
+    """Check init the session from a file."""
+    planet_secret_file = Path.home() / ".planet.json"
+    existing = False
+
+    # This test will overwrite and remove the file, let's backup it
+    if planet_secret_file.exists():
+        existing = True
+        planet_secret_file.rename(planet_secret_file.with_suffix(".json.bak"))
+
+    # Create a file with the credentials
+    planet_model = PlanetModel()
+    # We assume that the credentials are valid
+    planet_model.init_session(planet_key, write_secrets=True)
+
+    assert planet_secret_file.exists()
+
+    # test init with the file
+    planet_model = PlanetModel()
+    planet_model.init_session(str(planet_secret_file))
+
+    assert planet_model.active is True
+
+    # Check that wrong credentials won't save the secrets file
+
+    # remove the file
+    planet_secret_file.unlink()
+
+    planet_model = PlanetModel()
+
+    with pytest.raises(Exception):
+        planet_model.init_session("wrong_key", write_secrets=True)
+
+        assert not planet_secret_file.exists()
+
+    # Check no save with good credentials
+
+    planet_model = PlanetModel()
+    planet_model.init_session(planet_key, write_secrets=False)
+
+    assert not planet_secret_file.exists()
+
+    # restore the file
+    if existing:
+        planet_secret_file.with_suffix(".json.bak").rename(planet_secret_file)
+
     return
 
 
diff --git a/tests/test_planetapi/test_PlanetView.py b/tests/test_planetapi/test_PlanetView.py
index c700fe3f..1d0b39cc 100644
--- a/tests/test_planetapi/test_PlanetView.py
+++ b/tests/test_planetapi/test_PlanetView.py
@@ -2,11 +2,13 @@
 
 import json
 import os
+from pathlib import Path
 
 import pytest
 
 from sepal_ui import sepalwidgets as sw
-from sepal_ui.planetapi import PlanetView
+from sepal_ui.message import ms
+from sepal_ui.planetapi import PlanetModel, PlanetView
 
 
 @pytest.mark.skipif("PLANET_API_KEY" not in os.environ, reason="requires Planet")
@@ -40,16 +42,27 @@ def test_reset() -> None:
 
     # reset the view
     planet_view.reset()
-    assert planet_view.w_username.v_model is None
-    assert planet_view.w_password.v_model is None
-    assert planet_view.w_key.v_model is None
+    assert planet_view.w_username.v_model == ""
+    assert planet_view.w_password.v_model == ""
+    assert planet_view.w_key.v_model == ""
 
     # use a default method
-    default_method = "credentials"
-    assert planet_view.w_method.v_model == default_method
-    assert planet_view.w_username.viz is True
-    assert planet_view.w_password.viz is True
-    assert planet_view.w_key.viz is False
+    # Default method will be from_file if the secrets file exists
+    default_method = (
+        "from_file" if (Path.home() / ".planet.json").exists() else "credentials"
+    )
+    if default_method == "credentials":
+        assert planet_view.w_method.v_model == default_method
+        assert planet_view.w_username.viz is True
+        assert planet_view.w_password.viz is True
+        assert planet_view.w_key.viz is False
+        assert planet_view.w_secret_file.viz is False
+    else:
+        assert planet_view.w_method.v_model == default_method
+        assert planet_view.w_username.viz is False
+        assert planet_view.w_password.viz is False
+        assert planet_view.w_key.viz is False
+        assert planet_view.w_secret_file.viz is True
 
     # change the method
     planet_view.w_method.v_model = "api_key"
@@ -57,6 +70,7 @@ def test_reset() -> None:
     assert planet_view.w_username.viz is False
     assert planet_view.w_password.viz is False
     assert planet_view.w_key.viz is True
+    assert planet_view.w_secret_file.viz is False
 
     return
 
@@ -83,3 +97,66 @@ def test_validate() -> None:
     assert planet_view.planet_model.active is True
 
     return
+
+
+@pytest.mark.skipif("PLANET_API_KEY" not in os.environ, reason="requires Planet")
+def test_validate_secret_file(planet_key) -> None:
+    """Test validation view method of the secret file."""
+    # Arrange
+    planet_secret_file = Path.home() / ".planet.json"
+
+    # Test with existing file
+    # Create a secrets file
+    planet_model = PlanetModel()
+    planet_model.init_session(planet_key, write_secrets=True)
+
+    planet_view = PlanetView()
+    planet_view.validate_secret_file()
+
+    assert planet_view.w_secret_file.error_messages == []
+
+    # Also validate with the event
+    planet_view.btn.fire_event("click", None)
+
+    # Test with non-existing file
+
+    # Create a backup of the file
+    planet_secret_file.rename(planet_secret_file.with_suffix(".json.bak"))
+
+    planet_view.validate_secret_file()
+
+    assert planet_view.w_secret_file.error_messages == [
+        ms.planet.exception.no_secret_file
+    ]
+
+    # Restore the file
+    planet_secret_file.with_suffix(".json.bak").rename(planet_secret_file)
+
+
+def test_validate_event() -> None:
+    """Test validation button event."""
+    # Arrange
+    planet_secret_file = Path.home() / ".planet.json"
+    exists = False
+
+    # if the file exists, rename it
+    if planet_secret_file.exists():
+        exists = True
+        planet_secret_file.rename(planet_secret_file.with_suffix(".json.bak"))
+
+    # Arrange
+    planet_view = PlanetView()
+
+    planet_view.w_method.v_model = "from_file"
+
+    # Act
+    planet_view.btn.fire_event("click", None)
+
+    # Assert
+    assert planet_view.alert.children[0].children == [
+        ms.planet.exception.no_secret_file
+    ]
+
+    # Restore if there was a file
+    if exists:
+        planet_secret_file.with_suffix(".json.bak").rename(planet_secret_file)