-
Notifications
You must be signed in to change notification settings - Fork 12
29 new service also access with ssl certificate in lieu of usernamepassword #550
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
base: integration/main
Are you sure you want to change the base?
Conversation
Signed-off-by: bdhivya <[email protected]>
Signed-off-by: bdhivya <[email protected]>
Signed-off-by: bdhivya <[email protected]>
Signed-off-by: bdhivya <[email protected]>
suhasbshekar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good, please fix the lint issues
|
|
||
| // setAuthMethod determines the authentication method based on available credentials | ||
| func (c *HTTPClient) setAuthMethod() (AuthMethod, error) { | ||
| // Defaults to cert authentication if both basic and client certificate authentication parameters are given |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@suhasbshekar Is it a good idea to allow user to input both basic and cert auth? Setting default seems not explicit enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is actually not Defaulted, we should change the wording in the comment, users are allowed to input both basic and cert auth, if they use it together, then cert auth is considered, @DhivyaBharathi215 you can change the wording to "cert auth is prioritized if f both basic and client certificate authentication parameters are given
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as of now, ansible is having the same logic as well, it seems right, let user be free to configure, if they provide both, lets have a priority of which needs to be considered
Signed-off-by: bdhivya <[email protected]>
|
Do we need the documentation regarding this? |
Added ssl certificate based authentication in addition to basic username and password authentication