Dynamic sign in options

[davidosorno]

Description

New Feature
Allow admins to define the sign-in options that will be displayed on OpenSearch Dashboard login page. There are couple of sign-in options defined in Security documentation, and theses options must be available in security config.yml file to be able to change them dynamically in Security Dashboard.
Furthermore, if anonymous_auth_enabled is true it will be available in Security Dashboard sign-in options to allow admins enable or disable it.

Old Behavior
Admins have to update opensearch_dashboards.yml adding or removing sign-in options, and then restart Dashboards to be able to log in using other sign-in option.

New Behavior
Admins can change sign-in options dynamically without having to restart the Dashboards, and the changes are applied immediately. Users just need to logout in order to see the sign-in options available.

Issues Resolved

• Related [FEATURE] Dynamic Sign in options for OpenSearch Dashboards security-dashboards-plugin#1573

Testing

Unit Testing, Integration Testing, and Manual Testing.

Check List

• New functionality includes testing
• New functionality has been documented
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[cwperks]

Thank you @davidosorno and @EduardoCorazon. This PR looks good to me. Left a couple of suggestions.

[peternied]

Nice job plumbing this through, I think we need a couple more test cases around different positive scenarios.

Before we merge lets make the validation clearer:

• Is the response is when Enum::valueOf doesn't have a match actionable? I hope their is a clear error message
• What should happen in the authc configuration only saml is supported? I would think the other sign in types be invalid, how do you think this should be handled?

[DarshitChanpura]

LGTM. Thanks @davidosorno for addressing all comments!

[DarshitChanpura]

@peternied This PR is currently merge-blocked by your requested change. Would you please re-review this?

[peternied]

This PR uses the security's backend for data storage with no reasoning about the correctness of the values set. Until this is addressed I don't think this change should be merged. If we don't want to perform this reasoning I would advocate that this scenario is supported by using .kibana index to dynamically config this setting post boot.

@DarshitChanpura What do you think?

• Outstanding comment with requested test cases Dynamic sign in options #3869 (comment)

[DarshitChanpura]

My understanding is that this is a security config related update like multi-tenancy toggle which we are trying to make dynamically change-able, and all security config related updates have been part of security backend. I agree with adding couple more test cases to ensure that correct data is being displayed, and I think that using security backend along with tests that cover all bases should suffice to add this change. @davidosorno Would you mind addressing the outstanding comment about adding tests for all scenarios?

waiting for more comments to be addressed

[stephen-crawford]

@cwperks what do you want to do with this? I know @davidosorno is not going to be able to finish this so should we close it or do you have plans to take this over?

Edit: Thanks @davidosorno, sorry I misunderstood from our previous chat.

[davidosorno]

Hi @scrawfor99, I can fix anything to this PR if needed, I'm just waiting @peternied unblock it and the PR gets more reviews.

[peternied]

@davidosorno The data storage is being used to save a value for use on the front end - it isn't usable by the backend or other clients. With the value being specific to the front end, it seems stored it in the .kibana index would be an easier alternative with less coupling, no?

[cwperks]

The data storage is being used to save a value for use on the front end - it isn't usable by the backend or other clients. With the value being specific to the front end, it seems stored it in the .kibana index would be an easier alternative with less coupling, no?

@peternied The convention for dynamic dashboards settings has been to store them in the security index. Putting the sign-in options in the same place is consistent with the other dynamic dashboards settings for the security dashboards plugin.

[DarshitChanpura]

+1 to Craig's comments. This should be stored in security index since it is about the ability to update sign-in options and from what i understand .kibana index stores information about dashboard objects and not about security related configuration. Maybe I'm misunderstanding the question here?

[davidosorno]

Finally, it was merged.

I want to Thank @cwperks, @peternied, and @DarshitChanpura for your guidance, and helping me to develop this feature. It was really nice working with you all.