Skip to content
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

[WFCORE-5532] Proposal for realm readiness checking with Elytron #469

Closed
wants to merge 1 commit into from

Conversation

Skyllarr
Copy link
Contributor

@Skyllarr Skyllarr force-pushed the wfcore-5532 branch 2 times, most recently from 255e2d0 to 4ddece1 Compare March 29, 2022 14:21

=== Related Issues

* https://issues.redhat.com/browse/EAP7-1823[EAP7-1823] - EAP issue tracker
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be EAP7-1098.

LDAP realm, JAAS realm and others that do not provide possibility to check how many users are presentwill default to true, and the error page advising user to add management user will never be shown. But filesystem realm will check if the realm's folder is empty, properties realm will check properties file for users. This method will be used in the `createReadyFunction(Builder builder)` method of `ManagementHttpServer`. The current security domain can be obtained in `createReadyFunction(Builder builder)` method from builder's httpAuthenticationFactory, or the realm will default to "ManagementRealm". The security domain cwill internally check the map of its realms to check for existence of users in management domain.

The error page that prompts to add a user and the redirection is already implemented, so wildfly-elytron changes and updating of the `createReadyFunction(Builder builder)` method of `ManagementHttpServer` in wildfly-core should suffice.

Copy link
Contributor

Choose a reason for hiding this comment

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

The legacy realm readiness check also attempted to check if client cert auth was possible. Can we do something similar to that as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can check if the mechanism names obtained from the builder's httpAuthenticationFactory contain CLIENT_CERT. I have added it to the proposal and it will become more clear during implementation if that works well and is sufficient. I think it should work, thanks!

Add new method to SecurityRealm interface, similar to legacy method `isReadyForHttpChallenge()`, that can be used to find out if at least 1 user was added to the realm. New method will be added to security domain that will check its map of realms to find out whether the domain contains users.

LDAP realm, JAAS realm and others that do not provide possibility to check how many users are presentwill default to true, and the error page advising user to add management user will never be shown. But filesystem realm will check if the realm's folder is empty, properties realm will check properties file for users. This method will be used in the `createReadyFunction(Builder builder)` method of `ManagementHttpServer`. The current security domain can be obtained in `createReadyFunction(Builder builder)` method from builder's httpAuthenticationFactory, or the realm will default to "ManagementRealm". The security domain cwill internally check the map of its realms to check for existence of users in management domain.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just something to keep in mind. Currently, the add-user.sh script is used to add users to a properties realm.

When the default out of the box configuration makes use of a filesystem realm instead, a different approach (i.e., add-user 2.0 / CLI commands) would need to be used to add users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this info to the proposal to not forget, thanks!


Add new method to SecurityRealm interface, similar to legacy method `isReadyForHttpChallenge()`, that can be used to find out if at least 1 user was added to the realm. New method will be added to security domain that will check its map of realms to find out whether the domain contains users.

LDAP realm, JAAS realm and others that do not provide possibility to check how many users are presentwill default to true, and the error page advising user to add management user will never be shown. But filesystem realm will check if the realm's folder is empty, properties realm will check properties file for users. This method will be used in the `createReadyFunction(Builder builder)` method of `ManagementHttpServer`. The current security domain can be obtained in `createReadyFunction(Builder builder)` method from builder's httpAuthenticationFactory, or the realm will default to "ManagementRealm". The security domain cwill internally check the map of its realms to check for existence of users in management domain.
Copy link
Contributor

@fjuma fjuma Apr 4, 2022

Choose a reason for hiding this comment

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

Just a note here. For the filesystem realm, I think it's possible for the realm's folder to contain subdirectories but no files (e.g., try adding a user, then removing it, and checking the resulting directory structure). So when checking if the realm's directory is empty, this is something that needs to be taken into account too.

One option that could be considered is simply calling the FilesystemRealm#getRealmIdentityIterator method and checking what hasNext returns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fjuma added this info, thanks


== Implementation Plan

Add new method to SecurityRealm interface, similar to legacy method `isReadyForHttpChallenge()`, that can be used to find out if at least 1 user was added to the realm. New method will be added to security domain that will check its map of realms to find out whether the domain contains users.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should go for a different method name than what was used in the legacy security approach to make it more clear what the method does, e.g., isNonEmpty, containsUsers, isReadyForAuthentication or something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I agree. The containUsers might be misleading for the CLIENT_CERT mechanism since the realm does not have to contain the users IIUC, only roles? So that leaves isNonEmpty and isReadyForAuthentication or something like canResultInSuccessfulAuthentication.

@darranl
Copy link
Contributor

darranl commented Jan 26, 2023

Superseded by #511

@darranl darranl closed this Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants