Merge pull request #98 from britive/develop

v1.5.0rc1

Author: twratl

CHANGELOG.md

@@ -2,6 +2,25 @@
 
 * As of v1.4.0 release candidates will be published in an effort to get new features out faster while still allowing time for full QA testing before moving the release candidate to a full release.
 
+## v1.5.0rc1 [2023-09-18]
+#### What's New
+* None
+
+#### Enhancements
+* Enrich shell completion results for the `api` command
+
+#### Bug Fixes
+* Fixes an issue with interactive login when randomly generated tokens include `--` which the WAF sometimes sees as a SQL injection attack
+* Fixes an issue with `ssh-add` and temporary keys filling up the `ssh-agent` due to the order of command flags
+* Fixes and issue with `checkin` checking in the wrong profile type (programmatic vs console)
+
+#### Dependencies
+* `britive>=2.21.0`
+
+#### Other
+* None
+
+
 ## v1.4.0 [2023-07-25]
 #### What's New
 * `pybritive ssh gcp identity-aware-proxy` command - supports OS Login and SSH Instance Metadata

requirements.txt

@@ -1,4 +1,4 @@
-britive>=2.20.1
+britive>=2.21.0
 certifi>=2022.12.7
 charset-normalizer==2.1.0
 click==8.1.3

setup.cfg

@@ -1,6 +1,6 @@
 [metadata]
 name = pybritive
-version = 1.4.0
+version = 1.5.0rc1
 author = Britive Inc.
 author_email = support@britive.com
 description = A pure Python CLI for Britive
@@ -26,7 +26,7 @@ install_requires =
     toml
     cryptography>=41.0.0
     python-dateutil
-    britive>=2.20.1
+    britive>=2.21.0
     jmespath
     pyjwt
 

src/pybritive/britive_cli.py

@@ -377,7 +377,7 @@ def __get_cloud_credential_printer(self, app_type, console, mode, profile, silen
                 cli=self
             )
 
-    def checkin(self, profile):
+    def checkin(self, profile, console):
         self.login()
         self._set_available_profiles()
         parts = self._split_profile_into_parts(profile)
@@ -388,18 +388,21 @@ def checkin(self, profile):
             application_name=parts['app']
         )
 
+        access_type = 'CONSOLE' if console else 'PROGRAMMATIC'
+
         transaction_id = None
         application_type = None
         for checked_out_profile in self.b.my_access.list_checked_out_profiles():
             same_env = checked_out_profile['environmentId'] == ids['environment_id']
             same_profile = checked_out_profile['papId'] == ids['profile_id']
-            if all([same_env, same_profile]):
+            same_access_type = checked_out_profile['accessType'] == access_type
+            if all([same_env, same_profile, same_access_type]):
                 transaction_id = checked_out_profile['transactionId']
 
                 for available_profile in self.available_profiles:
                     same_env_2 = checked_out_profile['environmentId'] == available_profile['env_id']
                     same_profile_2 = checked_out_profile['papId'] == available_profile['profile_id']
-                    if all([same_env_2, same_profile_2]):
+                    if all([same_env_2, same_profile_2, access_type == 'PROGRAMMATIC']):
                         application_type = available_profile['app_type'].lower()
                         break
                 break
@@ -937,7 +940,7 @@ def _ssh_generate_key(self, username, hostname, key_source):
 
         # and if we are using ssh-agent we need to add the private key via ssh-add
         if key_source == 'ssh-agent':
-            subprocess.run(['ssh-add', str(pem_file), '-t', '60', '-q'])
+            subprocess.run(['ssh-add', '-t', '60', '-q', str(pem_file)])
 
         return {
             'private_key_filename': pem_file,

src/pybritive/commands/checkin.py

@@ -6,16 +6,17 @@
 
 @click.command()
 @build_britive
-@britive_options(names='tenant,token,silent,passphrase,federation_provider')
+@britive_options(names='console,tenant,token,silent,passphrase,federation_provider')
 @click_smart_profile_argument
-def checkin(ctx, tenant, token, silent, passphrase, federation_provider, profile):
+def checkin(ctx, console, tenant, token, silent, passphrase, federation_provider, profile):
     """Checkin a profile.
 
     This command takes 1 required argument `PROFILE`. This should be a string representation of the profile
     that should be checked in. Format is `application name/environment name/profile name`.
     """
     ctx.obj.britive.checkin(
-        profile=profile
+        profile=profile,
+        console=console
     )
 
 

src/pybritive/completers/api.py

@@ -1,5 +1,6 @@
 from britive.britive import Britive
-import json
+from click.shell_completion import CompletionItem
+import inspect
 
 
 def api_completer(ctx, param, incomplete):
@@ -16,17 +17,46 @@ def api_completer(ctx, param, incomplete):
     for part in parts:
         b = getattr(b, part)
 
+    existing = '.'.join(parts)
+
     options = []
 
     # vars happen at all levels
-    options += [var for var, value in vars(b).items() if str(value).startswith('<britive.') and var != 'britive']
+    for var, value in vars(b).items():
+        # filter out things which should not show as completion items
+        if not str(value).startswith('<britive.') or var == 'britive':
+            continue
+
+        method = f'{existing}.{var}' if not_base_level else var
+
+        if method.lower().startswith(incomplete.lower()):
+            doc_line = f'methods related to {var}'
+            try:
+                doc_line = inspect.getdoc(getattr(b, var)).split('\n')[0]
+            except:
+                pass
+            options.append(CompletionItem(method, help=doc_line))
 
     # dir only happens at non "base" levels
     if not_base_level:
-        options += [func for func in dir(b) if callable(getattr(b, func)) and not func.startswith("_")]
+        # for each method in the class
+        for func in dir(b):
+            # filter out methods which are not callable and methods which are not "public"
+            if not callable(getattr(b, func)) or func.startswith("_"):
+                continue
 
-    # pull it all back together and make it look nice
-    existing = '.'.join(parts)
-    options = [f'{existing}.{o}' if not_base_level else o for o in options]
+            method = f'{existing}.{func}'
+
+            # if this method is a potential match add it to the completion list
+            if method.lower().startswith(incomplete.lower()):
+                # grab the doc string if present
+                doc_line = f'no docs found for {method}'
+
+                try:
+                    doc_line = inspect.getdoc(getattr(b, func)).split('\n')[0]
+                except:
+                    pass
+
+                options.append(CompletionItem(method, help=doc_line))
+    return options
 
-    return [o for o in options if o.lower().startswith(incomplete.lower())]

src/pybritive/completers/api_command.py

@@ -14,28 +14,27 @@ def get_dynamic_method_parameters(method):
 
         # parse the method, so we can determine where in the "hierarchy" we are
         # and what commands/subcommands the user should be presented with
+
         for part in method.split('.'):
             b = getattr(b, part)
 
         params = {}
         spec = inspect.getfullargspec(b)
-
         # reformat parameters into a more consumable dict while holds all the required details
         helper = spec[6]
         helper.pop('return', None)
+
         for param, param_type in helper.items():
-            params[param] = {
-                'type': str(param_type).split("'")[1]
-            }
+            params[param] = {}
 
-        defaults = list(spec[3])
-        names = list(spec[0])
+        defaults = [] if spec[3] is None else list(spec[3])
+        names = [] if spec[0] is None else list(spec[0])
 
         if len(defaults) > 0:
             for i in range(1, len(defaults) + 1):
                 name = names[-1 * i]
                 default = defaults[-1 * i]
-                params[name]['default'] = default
+                params[name]['default'] = '<empty string>' if default == '' else default
 
         try:  # we don't REALLY need the doc string so if there are errors just eat them and move on
             doc_lines = inspect.getdoc(b)
@@ -70,6 +69,11 @@ def get_dynamic_method_parameters(method):
 
             param_list.append(param)
 
+        param_list.append({
+            'flag': '---------------------',
+            'help': 'separator between sdk parameters and cli parameters'
+        })
+
         return param_list
     except Exception as e:
         return []

src/pybritive/helpers/api_method_argument_dectorator.py

@@ -1,12 +1,13 @@
 import click
 import pkg_resources
-from ..completers.api import api_completer
+
 
 click_major_version = int(pkg_resources.get_distribution('click').version.split('.')[0])
 
 
 def click_smart_api_method_argument(func):
     if click_major_version >= 8:
+        from ..completers.api import api_completer
         dec = click.argument('method', shell_complete=api_completer)
     else:
         dec = click.argument('method')

src/pybritive/helpers/credentials.py

@@ -57,8 +57,14 @@ def __init__(self, tenant_name: str, tenant_alias: str, cli: any, federation_pro
         # not sure if we really need 32 random bytes or if any random string would work
         # but the current britive-cli in node.js does it this way so it will be done the same
         # way in python
-        self.verifier = b64_encode_url_safe(bytes([random.getrandbits(8) for _ in range(0, 32)]))
-        self.auth_token = b64_encode_url_safe(bytes(hashlib.sha512(self.verifier.encode('utf-8')).digest()))
+        while True:  # will break eventually when we get values that do not include --
+            self.verifier = b64_encode_url_safe(bytes([random.getrandbits(8) for _ in range(0, 32)]))
+            self.auth_token = b64_encode_url_safe(bytes(hashlib.sha512(self.verifier.encode('utf-8')).digest()))
+
+            # WAF doesn't like to see `--` as it thinks it is a sql injection attempt
+            if '--' not in self.verifier and '--' not in self.auth_token:
+                break
+
         self.credentials = self.load() or {}
 
     def _setup_requests_session(self):

src/pybritive/options/console.py

@@ -6,6 +6,6 @@
     default=False,
     is_flag=True,
     show_default=True,
-    help='Checkout the console access for the profile instead of programmatic access.'
+    help='Checkout/checkin the console access for the profile instead of programmatic access.'
 )