Skip to content

feat(icp4d): Add support for icp4d #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 6, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@ or
easy_install --upgrade ibm-cloud-sdk-core
```

## Authentication Types
There are several flavors of authentication supported in this package. To specify the intended authentication pattern to use, the user can pass in the parameter `authentication_type`. This parameter is optional, but it may become required in a future major release. The options for this parameter are `basic`, `iam`, and `icp4d`.

### basic
This indicates Basic Auth is to be used. Users will pass in a `username` and `password` and the SDK will generate a Basic Auth header to send with requests to the service.

### iam
This indicates that IAM token authentication is to be used. Users can pass in an `iam_apikey` or an `iam_access_token`. If an API key is used, the SDK will manage the token for the user. In either case, the SDK will generate a Bearer Auth header to send with requests to the service.

### icp4d
This indicates that the service is an instance of ICP4D, which has its own version of token authentication. Users can pass in a `username` and `password`, or an `icp4d_access_token`. If a username and password is given, the SDK will manage the token for the user.
A `icp4d_url` is **required** for this type. In order to use an SDK-managed token with ICP4D authentication, this option **must** be passed in.

## Issues

If you encounter an issue with this project, you are welcome to submit a [bug report](https://github.com/IBM/python-sdk-core/issues).
Expand Down
2 changes: 2 additions & 0 deletions ibm_cloud_sdk_core/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,7 @@
from .base_service import BaseService
from .detailed_response import DetailedResponse
from .iam_token_manager import IAMTokenManager
from .jwt_token_manager import JWTTokenManager
from .icp4d_token_manager import ICP4DTokenManager
from .api_exception import ApiException
from .utils import datetime_to_string, string_to_datetime
145 changes: 100 additions & 45 deletions ibm_cloud_sdk_core/base_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from .version import __version__
from .utils import has_bad_first_or_last_char, remove_null_values, cleanup_values
from .iam_token_manager import IAMTokenManager
from .icp4d_token_manager import ICP4DTokenManager
from .detailed_response import DetailedResponse
from .api_exception import ApiException

Expand All @@ -45,19 +46,21 @@ class BaseService(object):
ICP_PREFIX = 'icp-'
APIKEY = 'apikey'
IAM_ACCESS_TOKEN = 'iam_access_token'
ICP4D_ACCESS_TOKEN = 'icp4d_access_token'
URL = 'url'
USERNAME = 'username'
PASSWORD = 'password'
IAM_APIKEY = 'iam_apikey'
IAM_URL = 'iam_url'
ICP4D_URL = 'icp4d_url'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about the reasoning behind giving ICP4D it's own URL. If I understand correctly, ICP4D has one URL and the tokens are retrieved from a certain endpoint on the same cluster as the service requests. Therefore, only one URL should needed (unlike IAM, which has a separate service entirely).

Is that correct @mediumTaj? Let me know if I'm missing something or if you have a reason for doing this in Python!

Copy link
Contributor Author

@ehdsouza ehdsouza Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The url for token exchange looks different from the endpoint for API calls. This was based on the test instance provided:

assistant = AssistantV1(
    version='2018-07-10',
    username='<username>',
    password='<pwd>',
    url='https://mycluster.icp:31843/assistant/cfg-default/instances/1559584393/api',
    icp4d_url='https://mycluster.icp:31843',
    authentication_type='icp4d')
assistant.disable_SSL_verification()

@dpopp07 @mediumTaj please let me know if I understood it wrong

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, interesting. I hadn't seen that yet. Still, I wonder if there is a concrete way to parse the base URL from the service URL to use in the token manager. It seems redundant to pass in both of those when the base is the same. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My 2 cents... while it might be the case that the service endpoint and icp4d token service share a common part of their respective URLs, what happens when the architecture or configuration changes and that is no longer the case? It's probably best to let the user specify the service endpoint URL separate from the ICP4D token service url.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I too agree with @padamstx to have a separate url for the two.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, after talking with Phil a bit, I am convinced on accepting two separate URLs. That said, how we specifically handle that might be worth some additional discussion. Let's follow up on this tomorrow

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss this on the 6/5 standup call and make sure all SDK cores (and generator) are on the same page with this.

APIKEY_DEPRECATION_MESSAGE = 'Authenticating with apikey is deprecated. Move to using Identity and Access Management (IAM) authentication.'
DEFAULT_CREDENTIALS_FILE_NAME = 'ibm-credentials.env'
SDK_NAME = 'ibm-python-sdk-core'

def __init__(self, vcap_services_name, url, username=None, password=None,
use_vcap_services=True, api_key=None,
iam_apikey=None, iam_access_token=None, iam_url=None, iam_client_id=None, iam_client_secret=None,
display_name=None):
use_vcap_services=True, api_key=None, iam_apikey=None, iam_url=None,
iam_access_token=None, iam_client_id=None, iam_client_secret=None,
display_name=None, icp4d_access_token=None, icp4d_url=None, authentication_type=None):
"""
It loads credentials with the following preference:
1) Credentials explicitly set in the request
Expand All @@ -66,41 +69,56 @@ def __init__(self, vcap_services_name, url, username=None, password=None,
"""
self.url = url
self.http_config = {}
self.authentication_type = authentication_type.lower() if authentication_type else None
self.jar = None
self.api_key = None
self.username = None
self.password = None
self.default_headers = None
self.iam_apikey = None
self.iam_access_token = None
self.iam_url = None
self.iam_client_id = None
self.iam_client_secret = None
self.api_key = api_key
self.username = username
self.password = password
self.iam_apikey = iam_apikey
self.iam_access_token = iam_access_token
self.iam_url = iam_url
self.iam_client_id = iam_client_id
self.iam_client_secret = iam_client_secret
self.icp4d_access_token = icp4d_access_token
self.icp4d_url = icp4d_url
self.token_manager = None
self.default_headers = None
self.verify = None # Indicates whether to ignore verifying the SSL certification

if has_bad_first_or_last_char(self.url):
raise ValueError('The URL shouldn\'t start or end with curly brackets or quotes. '
'Be sure to remove any {} and \" characters surrounding your URL')
self._check_credentials()

self.set_user_agent_header(self.build_user_agent())

# 1. Credentials are passed in constructor
if api_key is not None:
if api_key.startswith(self.ICP_PREFIX):
self.set_username_and_password(self.APIKEY, api_key)
else:
self.set_token_manager(api_key, iam_access_token, iam_url, iam_client_id, iam_client_secret)
elif username is not None and password is not None:
if username is self.APIKEY and not password.startswith(self.ICP_PREFIX):
self.set_token_manager(password, iam_access_token, iam_url, iam_client_id, iam_client_secret)
else:
self.set_username_and_password(username, password)
elif iam_access_token is not None or iam_apikey is not None:
if iam_apikey and iam_apikey.startswith(self.ICP_PREFIX):
self.set_username_and_password(self.APIKEY, iam_apikey)
else:
self.set_token_manager(iam_apikey, iam_access_token, iam_url, iam_client_id, iam_client_secret)
if self.authentication_type == 'iam' or self._has_iam_credentials(self.iam_apikey, self.iam_access_token) or self._has_iam_credentials(self.api_key, self.iam_access_token):
self.token_manager = IAMTokenManager(self.iam_apikey or self.api_key or self.password,
self.iam_access_token,
self.iam_url,
self.iam_client_id,
self.iam_client_secret)
self.iam_apikey = self.iam_apikey or self.api_key or self.password
elif self._uses_basic_for_iam(self.username, self.password):
self.token_manager = IAMTokenManager(self.password,
self.iam_access_token,
self.iam_url,
self.iam_client_id,
self.iam_client_secret)
self.iam_apikey = self.password
self.username = None
self.password = None
elif self._is_for_icp4d(self.authentication_type, self.icp4d_access_token):
if self.icp4d_url is None:
raise Exception('The icp4d_url is mandatory for ICP4D.')
self.token_manager = ICP4DTokenManager(self.icp4d_url,
self.username,
self.password,
self.icp4d_access_token)
elif self._is_for_icp(self.api_key) or self._is_for_icp(self.iam_apikey):
self.username = self.APIKEY
self.password = self.api_key or self.iam_apikey
elif self.token_manager is None and self._has_basic_credentials(username, password):
self.username = username
self.password = password

# 2. Credentials from credential file
if display_name and not self.username and not self.token_manager:
Expand All @@ -124,6 +142,10 @@ def __init__(self, vcap_services_name, url, username=None, password=None,
self.set_iam_apikey(self.vcap_service_credentials.get(self.IAM_APIKEY))
if self.IAM_ACCESS_TOKEN in self.vcap_service_credentials:
self.set_iam_access_token(self.vcap_service_credentials.get(self.IAM_ACCESS_TOKEN))
if self.ICP4D_URL in self.vcap_service_credentials:
self.icp4d_url = self.vcap_service_credentials.get(self.ICP4D_URL)
if self.ICP4D_ACCESS_TOKEN in self.vcap_service_credentials:
self.set_icp4d_access_token(self.vcap_service_credentials.get(self.ICP4D_ACCESS_TOKEN))

if (self.username is None or self.password is None) and self.token_manager is None:
raise ValueError(
Expand Down Expand Up @@ -183,8 +205,45 @@ def _load_from_vcap_services(self, service_name):
else:
return None

def _is_for_icp(self, credential=None):
return credential and credential.startswith(self.ICP_PREFIX)

def _is_for_icp4d(self, authentication_type, icp4d_access_token=None):
return authentication_type == 'icp4d' or icp4d_access_token

def _has_basic_credentials(self, username, password):
return username and password and not self._uses_basic_for_iam(username, password)

def _has_iam_credentials(self, iam_apikey, iam_access_token):
return (iam_apikey or iam_access_token) and not self._is_for_icp(iam_apikey)

def _uses_basic_for_iam(self, username, password):
"""
Returns true if the user provides basic auth creds with the intention
of using IAM auth
"""
return username and password and username == self.APIKEY and not self._is_for_icp(password)

def _has_bad_first_or_last_char(self, str):
return str is not None and (str.startswith('{') or str.startswith('"') or str.endswith('}') or str.endswith('"'))

def _check_credentials(self):
credentials_to_check = {
'URL': self.url,
'username': self.username,
'password': self.password,
'credentials': self.iam_apikey
}

for key in credentials_to_check:
if self._has_bad_first_or_last_char(credentials_to_check.get(key)):
raise ValueError('The ' + key + ' shouldn\'t start or end with curly brackets or quotes. '
'Be sure to remove any {} and \" characters surrounding your ' + key)

def disable_SSL_verification(self):
self.verify = False
if self.token_manager is not None:
self.token_manager.disable_SSL_verification(True)

def set_username_and_password(self, username, password):
if has_bad_first_or_last_char(username):
Expand All @@ -198,20 +257,6 @@ def set_username_and_password(self, username, password):
self.password = password
self.jar = CookieJar()

def set_token_manager(self, iam_apikey=None, iam_access_token=None, iam_url=None,
iam_client_id=None, iam_client_secret=None):
if has_bad_first_or_last_char(iam_apikey):
raise ValueError('The credentials shouldn\'t start or end with curly brackets or quotes. '
'Be sure to remove any {} and \" characters surrounding your credentials')

self.token_manager = IAMTokenManager(iam_apikey, iam_access_token, iam_url, iam_client_id, iam_client_secret)
self.iam_apikey = iam_apikey
self.iam_access_token = iam_access_token
self.iam_url = iam_url
self.iam_client_id = iam_client_id
self.iam_client_secret = iam_client_secret
self.jar = CookieJar()

def set_iam_access_token(self, iam_access_token):
if self.token_manager:
self.token_manager.set_access_token(iam_access_token)
Expand All @@ -220,6 +265,16 @@ def set_iam_access_token(self, iam_access_token):
self.iam_access_token = iam_access_token
self.jar = CookieJar()

def set_icp4d_access_token(self, icp4d_access_token):
if self.token_manager:
self.token_manager.set_access_token(icp4d_access_token)
else:
if self.icp4d_url is None:
raise Exception('The icp4d_url is mandatory for ICP4D.')
self.token_manager = ICP4DTokenManager(self.icp4d_url, access_token=icp4d_access_token)
self.icp4d_access_token = icp4d_access_token
self.jar = CookieJar()

def set_iam_url(self, iam_url):
if self.token_manager:
self.token_manager.set_iam_url(iam_url)
Expand Down Expand Up @@ -313,7 +368,7 @@ def request(self, method, url, accept_json=False, headers=None,
if self.token_manager:
access_token = self.token_manager.get_token()
headers['Authorization'] = '{0} {1}'.format(self.BEARER, access_token)
if self.username and self.password:
elif self.username and self.password:
auth = (self.username, self.password)

# Use a one minute timeout when our caller doesn't give a timeout.
Expand Down
Loading