[EPMDEDP-12204]: fix: Allow non-interactive login with set password for KeycloakRealmUser

BronzeDeer commented 1 year ago

Description

Fixes https://github.com/epam/edp-keycloak-operator/issues/6

When configuring a User with a desired password, either via spec.password or .spec.passwordSecret, the password cannot be used for login since the UPDATE_PASSWORD required action is always set, regardless of what was specified in spec.requiredUserActions. This makes the User CRD unusable for non-interactive logins by machine users (e.g. a User automatically created for a test job) until an admin manually removes the required Action. Logging in interactively via a browser will also clear the login blocking action, however since this will force a password change, the configured password can no longer be used for automatic logins.

This commit sets the "temporary" attribute of the password to false, the previous value of "true" was forcing the UPDATE_PASSWORD user action. If this behaviour is specifically desired (for example as part of an automated flow that provides initial passwords to human users), one should add "UPDATE_PASSWORD" to .spec.requiredUserActions explicitly.

Type of change

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Ran all available tests including make e2e against kind with k8s v1.27.3
Deployed build with fix in Dev at work where Bug was intially noticed, bug confirmed gone

Checklist:

[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[x] My changes generate no new warnings
[ ] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes

Screenshots (if appropriate):

Additional context

Add any other context or screenshots about the feature request here.

BronzeDeer commented 1 year ago

Replaced commit with signed version

SergK commented 1 year ago

Hi @BronzeDeer

Thank you for the fix. We are currently in the process of migration (exposing our CI to the external world), meanwhile, can I kindly ask you to align with our existing requirements (https://epam.github.io/edp-install/developer-guide/edp-workflow/), so the commit message should be in the expected format. This is how it should look in your case (I've created ticket in our internal system to properly address it):

[EPMDEDP-12204]: fix: Allow non-interactive login with set password for KeycloakRealmUser

Fixes https://github.com/epam/edp-keycloak-operator/issues/6

When configuring a User with a desired password, either via
`spec.password` or `.spec.passwordSecret`, the password cannot be used
for login since the UPDATE_PASSWORD required action is always set,
regardless of what was specified in `spec.requiredUserActions`. This
makes the User CRD unusable for non-interactive logins by machine users
(e.g. a User automatically created for a test job) until an admin
manually removes the required Action. Logging in interactively via a
browser will also clear the login blocking action, however since this
will force a password change, the configured password can no longer be
used for automatic logins.

This commit sets the "temporary" attribute of the password to false,
the previous value of "true" was forcing the UPDATE_PASSWORD user
action. If this behavior is specifically desired (for example, as part
of an automated flow that provides initial passwords to human users),
one should add "UPDATE_PASSWORD" to `.spec.requiredUserActions`
explicitly.

This is how our changelog tool is configured and expects a commit message in this format

Please use squash into a single commit as well. Thank you in advance

PS. We are expecting to move our CI to the public in a couple of weeks, so the process will be more visible and simple

Thank you

BronzeDeer commented 1 year ago

Changed commit message to required format. Thank you for the quick response!

SergK commented 1 year ago

@BronzeDeer merged, thank you for your contribution

epam / edp-keycloak-operator