#61 Session header authentication #63
Conversation
…nded IQS Connection Widget to accommodate
| password_label='Password:', | ||
| oidc_checkbox_label='Use OpenID Connect', | ||
| token_label='OIDC Token:', |
There was a problem hiding this comment.
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).
| default_use_oidc: bool = False | ||
| default_use_password: bool = True | ||
| default_use_auth_header: bool = False |
There was a problem hiding this comment.
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.
| 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] |
There was a problem hiding this comment.
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?
|
|
||
| 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: |
There was a problem hiding this comment.
??? 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
|
@bergmanngabor Since your change requests are widget specific and cleaning-up the connection widget is not in the scope of this issue I created a follow-up: #64 |
Fixes #61
Added ability to connect to IQS instance with authentication header (name & value).
Extended IQS Connection Widget with multiple auth options.