feat: remove credentials from urls (#841)

Author: mohammad-alisafaee

renku/cli/dataset.py

@@ -410,7 +410,10 @@ def edit(dataset_id):
     '--force', is_flag=True, help='Allow adding otherwise ignored files.'
 )
 @click.option(
-    '--create', is_flag=True, help='Create dataset if it does not exist.'
+    '-c',
+    '--create',
+    is_flag=True,
+    help='Create dataset if it does not exist.'
 )
 @click.option(
     '-s',

renku/core/commands/dataset.py

@@ -47,6 +47,7 @@
 from renku.core.models.refs import LinkReference
 from renku.core.models.tabulate import tabulate
 from renku.core.utils.doi import extract_doi
+from renku.core.utils.urls import remove_credentials
 
 from .client import pass_local_client
 from .echo import WARNING
@@ -206,6 +207,7 @@ def add_to_dataset(
                                 file_.url = file_.url.geturl()
 
                             file_.path = added_.path
+                            file_.url = remove_credentials(file_.url)
                             file_.creator = with_metadata.creator
                             file_._label = added_._label
                             file_.commit = added_.commit
@@ -505,6 +507,7 @@ def import_dataset(
         pool.close()
 
         dataset_name = name or dataset.display_name
+        dataset.url = remove_credentials(dataset.url)
         add_to_dataset(
             client,
             urls=[str(p) for p in Path(data_folder).glob('*')],

renku/core/management/datasets.py

@@ -41,6 +41,7 @@
 from renku.core.models.locals import with_reference
 from renku.core.models.provenance.agents import Person
 from renku.core.models.refs import LinkReference
+from renku.core.utils.urls import remove_credentials
 
 
 @attr.s
@@ -301,7 +302,7 @@ def _add_from_local(self, dataset, path, link, destination):
             else:
                 return [{
                     'path': path_in_repo,
-                    'url': src.as_uri(),
+                    'url': path_in_repo,
                     'creator': dataset.creator,
                     'dataset': dataset.name,
                     'parent': self
@@ -323,7 +324,7 @@ def _add_from_local(self, dataset, path, link, destination):
 
         return [{
             'path': destination.relative_to(self.path),
-            'url': src.as_uri(),
+            'url': 'file://' + os.path.relpath(str(src), str(self.path)),
             'creator': dataset.creator,
             'dataset': dataset.name,
             'parent': self
@@ -349,7 +350,7 @@ def _add_from_url(self, dataset, url, destination):
 
         return [{
             'path': destination.relative_to(self.path),
-            'url': url,
+            'url': remove_credentials(url),
             'creator': dataset.creator,
             'dataset': dataset.name,
             'parent': self
@@ -426,7 +427,7 @@ def _add_from_git(self, dataset, url, sources, destination, ref):
 
                 results.append({
                     'path': path_in_dst_repo,
-                    'url': url,
+                    'url': remove_credentials(url),
                     'creator': creators,
                     'dataset': dataset.name,
                     'parent': self,

renku/core/management/git.py

@@ -33,6 +33,7 @@
 import gitdb
 
 from renku.core import errors
+from renku.core.utils.urls import remove_credentials
 
 COMMIT_DIFF_STRATEGY = 'DIFF'
 STARTED_AT = int(time.time() * 1e3)
@@ -301,7 +302,8 @@ def commit(
                 raise errors.NothingToCommit()
             return
 
-        argv = [os.path.basename(sys.argv[0])] + sys.argv[1:]
+        argv = [os.path.basename(sys.argv[0])
+                ] + [remove_credentials(arg) for arg in sys.argv[1:]]
 
         # Ignore pre-commit hooks since we have already done everything.
         self.repo.index.commit(

renku/core/utils/urls.py

@@ -17,6 +17,7 @@
 # limitations under the License.
 """Helpers utils for handling URLs."""
 
+import urllib
 from urllib.parse import ParseResult
 
 
@@ -39,3 +40,11 @@ def url_to_string(url):
         return url
 
     raise ValueError('url value not recognized')
+
+
+def remove_credentials(url):
+    """Remove username and password from a URL."""
+    if url is None:
+        return ''
+    parsed = urllib.parse.urlparse(url)
+    return parsed._replace(netloc=parsed.hostname).geturl()

tests/cli/test_datasets.py

@@ -1170,8 +1170,8 @@ def test_dataset_clean_up_when_add_fails(runner, client):
     assert not ref.is_symlink() and not ref.exists()
 
 
-def test_add_removes_local_path_information(runner, client, directory_tree):
-    """Test local paths are removed when adding to a dataset."""
+def test_avoid_empty_commits(runner, client, directory_tree):
+    """Test no empty commit is created when adding existing data."""
     runner.invoke(cli, ['dataset', 'create', 'my-dataset'])
 
     commit_sha_before = client.repo.head.object.hexsha
@@ -1192,3 +1192,27 @@ def test_add_removes_local_path_information(runner, client, directory_tree):
     commit_sha_after = client.repo.head.object.hexsha
     assert commit_sha_before == commit_sha_after
     assert 'Error: There is nothing to commit.' in result.output
+
+
+def test_add_removes_credentials(runner, client):
+    """Test credentials are removed when adding to a dataset."""
+    URL = 'https://username:password@example.com/index.html'
+    result = runner.invoke(cli, ['dataset', 'add', '-c', 'my-dataset', URL])
+    assert 0 == result.exit_code
+
+    with client.with_dataset('my-dataset') as dataset:
+        file_ = dataset.files[0]
+        assert file_.url == 'https://example.com/index.html'
+
+
+def test_add_removes_local_path_information(runner, client, directory_tree):
+    """Test local paths are removed when adding to a dataset."""
+    result = runner.invoke(
+        cli, ['dataset', 'add', '-c', 'my-dataset', directory_tree.strpath]
+    )
+    assert 0 == result.exit_code
+
+    with client.with_dataset('my-dataset') as dataset:
+        for file_ in dataset.files:
+            assert file_.url.startswith('file://../')
+            assert file_.url.endswith(file_.name)