Skip to content
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.

Sign up for GitHub

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

#61 Session header authentication #63

Merged
merged 1 commit into from
Oct 21, 2021
+54 −10
Merged

#61 Session header authentication #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions source/incqueryserver-jupyter/iqs_jupyter/api_composition.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ def decorate_iqs_client(iqs_client_object, root_configuration, endpoint_class):
endpoint_specific_config.host = "{}/{}".format(root_configuration.host, api_client_module.endpoint_path)
endpoint_specific_client = endpoint_class(endpoint_specific_config)

# If session header is set in configuration, we set a default header in ApiClient
if hasattr(root_configuration, 'auth_header_name') and hasattr(root_configuration, 'auth_header_value'):
endpoint_specific_client.set_default_header(root_configuration.auth_header_name, root_configuration.auth_header_value)


for api_field_name, api_class_name in api_client_module.api_names_to_class.items():
api_module_name = "{}.api.{}_api".format(api_client_module.root_module_name, api_field_name)
try:
Expand Down
56 changes: 46 additions & 10 deletions source/incqueryserver-jupyter/iqs_jupyter/authentication.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,34 @@ def connect(
user=defaults.default_IQS_username,
password=defaults.default_IQS_password,
token=defaults.default_IQS_token,
auth_header_name=defaults.default_IQS_auth_header_name,
auth_header_value=defaults.default_IQS_auth_header_value,
auth_with_user_pw=defaults.default_use_password,
use_oidc=defaults.default_use_oidc
use_oidc=defaults.default_use_oidc,
use_auth_header=defaults.default_use_auth_header
):
configuration = iqs_client.Configuration()
configuration.host = address
configuration.access_token = None
if auth_with_user_pw:

# Cases: Auth with password, auth with token, Session header/value
# First priority: Session header

if use_auth_header:
# Disabling username, password and token, so ApiClient does not try to authenticate
configuration.username = None
configuration.password = None
configuration.access_token = None

setattr(configuration, 'auth_header_name', auth_header_name)
setattr(configuration, 'auth_header_value', auth_header_value)

elif auth_with_user_pw:
configuration.username = user
configuration.password = password
else:
configuration.access_token = token

return IQSClient(configuration, use_oidc)


Expand All @@ -64,26 +81,42 @@ def __init__(
initial_password=defaults.default_IQS_password,
use_oidc=defaults.default_use_oidc,
token=defaults.default_IQS_token,
initial_auth_header_name=defaults.default_IQS_auth_header_name,
initial_auth_header_value=defaults.default_IQS_auth_header_value,
use_auth_header=defaults.default_use_auth_header,
address_label='Address:',
use_user_pw_label='Login with Username & Password',
user_label='User:',
oicd_checkbox_label='Use OpenID Connect',
password_label='Password:',
oidc_checkbox_label='Use OpenID Connect',
token_label='OIDC Token:',
Copy link
Member

Choose a reason for hiding this comment

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

Do we always want to display this field? I think we should at least add some label to say that this is optional to provide. Same for the Auth header name/value (which already has a checkbox).

label_text="IQS API Access Point",
auth_header_checkbox_label='Use Authentication Header',
auth_header_name_label='Header Name:',
auth_header_value_label='Header Value:',
login_button=True,
auto_display=defaults.default_auto_display
):
self.ask_for_user_pw = ask_for_user_pw
self.token = token

self.ask_for_user_pw_checkbox = widgets.Checkbox(value=ask_for_user_pw, description=use_user_pw_label)
self.address_field = widgets.Text(value=initial_address, description=address_label)
self.user_field = widgets.Text(value=initial_user, description=user_label)
self.password_field = widgets.Password(value=initial_password, description=password_label)
self.oicd_checkbox = widgets.Checkbox(value=use_oidc, description=oicd_checkbox_label)

self.oidc_checkbox = widgets.Checkbox(value=use_oidc, description=oidc_checkbox_label)
self.token_field = widgets.Text(value=token, description=token_label)

self.auth_header_checkbox = widgets.Checkbox(value=use_auth_header, description=auth_header_checkbox_label)
self.auth_header_name_field = widgets.Text(value=initial_auth_header_name, description=auth_header_name_label)
self.auth_header_value_field = widgets.Text(value=initial_auth_header_value, description=auth_header_value_label)

self.iqs_client = None

if ask_for_user_pw:
Copy link
Member

Choose a reason for hiding this comment

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

??? if there is a ask_for_user_pw_checkbox why do you use its (possibly obsolete) initial value, as opposed to its actual value? will the set of displayed widgets change if I check/uncheck the checkbox?

Same with auth_header_checkbox

fields = [self.address_field, self.user_field, self.password_field, self.oicd_checkbox]
fields = [self.address_field,
self.ask_for_user_pw_checkbox, self.user_field, self.password_field,
self.oidc_checkbox, self.token_field,
self.auth_header_checkbox, self.auth_header_name_field, self.auth_header_value_field]
Copy link
Member

Choose a reason for hiding this comment

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

why are these in the ask_for_user_pw branch? I though we do not ask for user/pass when we ask for auth header fields; also, should we not branch on the value of self.auth_header_checkbox to determine whether we should display these fields?

else:
fields = [self.address_field]

Expand Down Expand Up @@ -137,7 +170,10 @@ def connect(self):
address=self.address_field.value,
user=self.user_field.value,
password=self.password_field.value,
token=self.token,
auth_with_user_pw=self.ask_for_user_pw,
use_oidc=self.oicd_checkbox.value
token=self.token_field.value,
auth_with_user_pw=self.ask_for_user_pw_checkbox.value,
use_oidc=self.oidc_checkbox.value,
use_auth_header=self.auth_header_checkbox.value,
auth_header_name=self.auth_header_name_field.value,
auth_header_value=self.auth_header_value_field.value
)
3 changes: 3 additions & 0 deletions source/incqueryserver-jupyter/iqs_jupyter/config_defaults.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@
default_auto_display: bool = True
default_use_oidc: bool = False
default_use_password: bool = True
default_use_auth_header: bool = False
Comment on lines 24 to +26
Copy link
Member

Choose a reason for hiding this comment

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

These 3 should be modified to use the default_IQS_ prefix - after all, they relate to the connection to the IQS, as opposed to e.g. MMS. (probably we can keep the old names in deprecated form, to retain compatibility, and use their value if the newer environment variable name is not present)

Sorry for not catching this in earlier reviews.


default_IQS_address : str = None
default_IQS_username : str = None
default_IQS_password : str = None
default_IQS_token: str = None
default_IQS_auth_header_name: str = None
default_IQS_auth_header_value: str = None

default_twc_workspace : str = None
default_twc_resource : str = None
Expand Down