Skip to content

Conversation

@ywangd
Copy link
Member

@ywangd ywangd commented Jun 24, 2022

All enabled realms must have unique names. This PR tightened the logic
for ensuring realm name uniqueness. Previously the unique name check
does not cover realms that are automatically enabled.

Relates: #69096

All enabled realms must have unique names. This PR tightened the logic
for ensuring realm name uniqueness. Previously the unique name check
does not cover realms that are automatically enabled.

Relates: elastic#69096
@ywangd ywangd added >bug :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.4.0 labels Jun 24, 2022
@ywangd ywangd requested a review from tvernum June 24, 2022 02:51
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Jun 24, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@elasticsearchmachine
Copy link
Collaborator

Hi @ywangd, I've created a changelog YAML for you.

)
);

// It is OK if the explicitly configured realm is disabled
Copy link
Member Author

Choose a reason for hiding this comment

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

This and a few other test scenarios below can be seen as a leniency. Ideally these names should not be used at all. We may want to do something about it, e.g. log warnings. But it is a separate issue from what we are trying to fix here.

@ywangd
Copy link
Member Author

ywangd commented Jun 24, 2022

@elasticmachine update branch

Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@n1v0lg n1v0lg left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the fix.

…ecurity/authc/Realms.java

Co-authored-by: Nikolaj Volgushev <[email protected]>
@ywangd ywangd added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jun 24, 2022
@ywangd
Copy link
Member Author

ywangd commented Jun 24, 2022

@elasticmachine run elasticsearch-ci/part-2-fips

@elasticsearchmachine elasticsearchmachine merged commit a298645 into elastic:master Jun 24, 2022
@ywangd ywangd deleted the actually-no-duplicate-realm-names branch June 24, 2022 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team v8.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants