8000 Password history policy - add time based policy by MaciejMierzwa · Pull Request #30215 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Password history policy - add time based policy - #30215

Merged
pedroigor merged 2 commits into
keycloak:mainfrom
MaciejMierzwa:feature_30211-password-age-policy
Jul 24, 2024
Merged

Password history policy - add time based policy#30215
pedroigor merged 2 commits into
keycloak:mainfrom
MaciejMierzwa:feature_30211-password-age-policy

Conversation

@MaciejMierzwa
@MaciejMierzwa MaciejMierzwa commented Jun 6, 2024 •
edited
Loading
Copy link
Copy Markdown
Contributor

Issue: #30211
Discussion: #30210

Closes #30211

@MaciejMierzwa
MaciejMierzwa force-pushed the feature_30211-password-age-policy branch 3 times, most recently from 89c6420 to 3383ec5 Compare June 6, 2024 13:25
@MaciejMierzwa
MaciejMierzwa marked this pull request as ready for review June 6, 2024 13:26
@MaciejMierzwa
MaciejMierzwa requested review from a team as code owners June 6, 2024 13:26
@MaciejMierzwa
MaciejMierzwa force-pushed the feature_30211-password-age-policy branch from 3383ec5 to c7b8445 Compare June 6, 2024 15:28
@mposolda mposolda self-assigned this Jun 7, 2024
@MaciejMierzwa
MaciejMierzwa force-pushed the feature_30211-password-age-policy branch from c7b8445 to 12713ef Compare June 7, 2024 10:12
@MaciejMierzwa
MaciejMierzwa commented Jun 7, 2024
Copy link
Copy Markdown
Contributor Author

Implementation is based on other password history based policy - HistoryPasswordPolicyProvider.java

@keycloak-github-bot keycloak-github-bot Bot mentioned this pull request Jun 7, 2024
Closed
thomasdarimont
thomasdarimont suggested changes Jun 7, 2024
View reviewed changes
@thomasdarimont thomasdarimont left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a few nitpicks - otherwise LGTM

Comment thread common/src/main/java/org/keycloak/common/util/Time.java Resolved
Comment thread server-spi-private/src/main/java/org/keycloak/policy/AgePasswordPolicyProvider.java Resolved
Comment thread server-spi-private/src/main/java/org/keycloak/policy/AgePasswordPolicyProvider.java Resolved
Comment thread server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java Resolved
8000 @MaciejMierzwa
MaciejMierzwa force-pushed the feature_30211-password-age-policy branch 3 times, most recently from 9a93123 to 5f58de0 Compare June 12, 2024 10:15
@MaciejMierzwa
MaciejMierzwa commented Jun 12, 2024
Copy link
Copy Markdown
Contributor Author

Updated branch with suggested changes. I also increased the time offset in tests, it was set to a couple of seconds, but now it is a couple of minutes. It should help with test flakiness in case of longer runs.

@MaciejMierzwa
MaciejMierzwa force-pushed the feature_30211-password-age-policy branch 4 times, most recently from 6b903ab to 5d1d62a Compare June 12, 2024 13:17
@MaciejMierzwa
MaciejMierzwa force-pushed the feature_30211-password-age-policy branch 4 times, most recently from 21f7e9d to 8c66df9 Compare June 26, 2024 09:52
@MaciejMierzwa
MaciejMierzwa commented Jun 27, 2024
Copy link
Copy Markdown
Contributor Author

Hi everybody, imo the feature is ready to go. It looks like all tests are green. One issue I have is that I changed documentation which triggers the documentation validation job. There are some expired links, but it's not in caused by my changes. Looks like it's the scope of a different task.
Could you please advise on that?
@sguilhen
@pedroigor
@thomasdarimont
@mabartos

@MaciejMierzwa
MaciejMierzwa force-pushed the feature_30211-password-age-policy branch from 8c66df9 to 0ab59b9 Compare June 27, 2024 09:20
This was referenced Jun 28, 2024
Closed
Closed
@MaciejMierzwa
MaciejMierzwa commented Jun 28, 2024
Copy link
Copy Markdown
Contributor Author

Hi, looks like the pipeline is green. My tests are run in job: Keycloak CI / Base IT (6)

@MaciejMierzwa
MaciejMierzwa force-pushed the feature_30211-password-age-policy branch 2 times, most recently from 047270c to 0ffcd47 Compare July 3, 2024 08:20
@MaciejMierzwa
MaciejMierzwa force-pushed the feature_30211-password-age-policy branch from 0ffcd47 to 259ef3e Compare July 8, 2024 09:08
@MaciejMierzwa
MaciejMierzwa commented Jul 8, 2024
Copy link
Copy Markdown
Contributor Author

Hi, are there any additional steps I have to take to get this merged/released? No additional activity from me, except rebasing.
@sguilhen
@pedroigor
@thomasdarimont
@mabartos

@MaciejMierzwa
MaciejMierzwa force-pushed the feature_30211-password-age-policy branch 2 times, most recently from 2bbc368 to f10f835 Compare July 17, 2024 13:38
@MaciejMierzwa
MaciejMierzwa commented Jul 18, 2024
Copy link
Copy Markdown
Contributor Author

Hi, looks like the feature works, all tests are passing, there is documentation ready in PR, and the only issue is the amount of changes coming to the main branch and rebasing it.
What should I do to get this merged/released?
@sguilhen
@pedroigor
@thomasdarimont
@mabartos

@MaciejMierzwa
MaciejMierzwa commented Jul 22, 2024
Copy link
Copy Markdown
Contributor Author

Hi @sguilhen pinging for visibility, I saw you assigned this task to yourself. I implemented the changes you suggested: #30215 (comment)

@MaciejMierzwa
MaciejMierzwa requested a review from sguilhen July 22, 2024 09:07
@sguilhen
sguilhen commented Jul 22, 2024
Copy link
Copy Markdown
Contributor

Hi @MaciejMierzwa - yeah, I assigned it to myself to ensure I can help drive this to completion. I've already asked for maintainer approval for the PR, I'm hoping to get this in shortly

MaciejMierzwa and others added 2 commits July 24, 2024 11:53
@MaciejMierzwa @pedroigor
feature: password age in days policy
79ca57f 
Closes keycloak#30210

Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
@pedroigor
Updating docs
223b5d8 
Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
@pedroigor
pedroigor force-pushed the feature_30211-password-age-policy branch from f10f835 to 223b5d8 Compare July 24, 2024 15:23
pedroigor
pedroigor approved these changes Jul 24, 2024
View reviewed changes
@pedroigor pedroigor left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@MaciejMierzwa Thanks for your time. I have just updated the docs to make it a bit more clear about the intention of the new policy.

Please, let me know what you think. LGTM.

@keycloak-github-bot keycloak-github-bot Bot mentioned this pull request Jul 24, 2024
Closed
@pedroigor
pedroigor merged commit f4b1a5c into keycloak:main Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pedroigor pedroi 7FFE gor pedroigor approved these changes
@thomasdarimont thomasdarimont Awaiting requested review from thomasdarimont
@mabartos mabartos Awaiting requested review from mabartos
@sguilhen sguilhen Awaiting requested review from sguilhen

Assignees

@mposolda mposolda

Labels

flaky-test team/cloud-native team/core-clients team/core-iam

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Password history policy - add time based policy.

7 participants

@MaciejMierzwa @sguilhen @codespearhead @thomasdarimont @pedroigor @mabartos @mposolda
0