Fix dynamic sign-in options: remove stale default and improve validation

[cwperks]

Description

This PR improves the dynamic sign-in options feature by fixing validation logic and removing a stale default that prevented the frontend from correctly determining available authentication methods.

Changes

1. Remove hardcoded sign_in_options default (ConfigV7.java)

Changed the default from [BASIC] to an empty list and added @JsonInclude(NON_EMPTY) so the field is not serialized when empty. Previously, [BASIC] was persisted to the security index on every cluster bootstrap, even if no admin explicitly configured sign-in options. This caused the frontend to always see ["BASIC"] and override the opensearch_dashboards.yml auth type configuration.

With this change:

• New clusters will not have sign_in_options persisted at init time
• The field is only written to the index when explicitly set via the tenancy config API
• Existing clusters are unaffected — their stored ["BASIC"] continues to be read back as before

2. Fix getNewSignInOptions validation (MultiTenancyConfigApiAction.java)

The previous validation had two issues:

• It validated by checking if the auth domain name contained the option string (e.g., "basic_internal_auth_domain".contains("basicauth")), which was brittle and dependent on naming conventions
• It used DashboardSignInOption.valueOf(option) which requires the exact Java enum constant name, rejecting valid inputs like "basic" or "basicauth"

The new validation:

• Validates against actual authenticator types (http_authenticator.type) from enabled auth domains
• Resolves options flexibly — accepts the enum name (BASIC), the option value (basic), or the frontend identifier (basicauth)
• Provides distinct error messages for unrecognized options vs. unavailable providers

3. Updated tests

• ConfigV7Test — Updated to handle absent sign_in_options field when empty
• MultiTenancyConfigApiTest — Updated initial assertion to expect empty array; updated error message assertion for invalid options
• SignInOptionsPersistenceTest (new) — Integration test verifying that sign_in_options is not persisted on bootstrap or unrelated config updates, and is only persisted after explicit PUT

Issues Resolved

opensearch-project/security-dashboards-plugin#1573

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.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Possible Issue The validation logic throws an exception inside the map operation when an invalid option is encountered, but the stream processing continues. If multiple invalid options are present, only the first will be reported. Additionally, the exception is thrown during stream processing which may not be the clearest approach for validation errors. return IntStream.range(0, newOptions.size()).mapToObj(newOptions::get).map(JsonNode::asText).map(option -> { DashboardSignInOption resolved = resolveSignInOption(option); if (resolved == null) { throw new IllegalArgumentException("Validation failure: " + option + " is not a recognized sign-in option."); } if (resolved == DashboardSignInOption.ANONYMOUS) { return resolved; } if (!authenticatorTypes.contains(resolved.getOption())) { throw new IllegalArgumentException( "Validation failure: " + resolved + " authentication provider is not available for this cluster." ); } return resolved; }).collect(Collectors.toList()); Logic Issue The code checks if resolved == DashboardSignInOption.ANONYMOUS and returns it immediately without validating against authenticatorTypes. However, ANONYMOUS may still need validation depending on whether anonymous authentication is actually enabled in the cluster configuration. If anonymous auth is disabled but ANONYMOUS is in the sign-in options, it could lead to a non-functional configuration. if (resolved == DashboardSignInOption.ANONYMOUS) { return resolved; }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add null check for http_authenticator Add null safety check for domain.http_authenticator before accessing its type field. If http_authenticator is null, this will throw a NullPointerException at runtime. src/main/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiAction.java [272-277] Set<String> authenticatorTypes = authc.getDomains() .values() .stream() - .filter(domain -> domain.http_enabled) + .filter(domain -> domain.http_enabled && domain.http_authenticator != null) .map(domain -> domain.http_authenticator.type.toLowerCase()) .collect(Collectors.toSet()); Suggestion importance[1-10]: 7 __ Why: Adding a null check for http_authenticator prevents potential NullPointerException at runtime. This is a valid defensive programming practice, though the actual risk depends on whether http_authenticator can be null in the codebase.	Medium
General	Use Collections.emptyList() for initialization Use Collections.emptyList() instead of Arrays.asList() for initializing an empty list. Arrays.asList() creates a fixed-size list backed by an array, while Collections.emptyList() returns an immutable empty list which is more efficient and semantically correct. src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java [137-138] @JsonInclude(JsonInclude.Include.NON_EMPTY) -public List<DashboardSignInOption> sign_in_options = Arrays.asList(); +public List<DashboardSignInOption> sign_in_options = Collections.emptyList(); Suggestion importance[1-10]: 5 __ Why: Using Collections.emptyList() is more semantically correct and efficient than Arrays.asList() for an empty list. However, this is a minor optimization that doesn't significantly impact functionality, and the field appears to be reassignable so immutability may not be the primary concern.	Low