Skip to content

Conversation

@Nayana-R-Gowda
Copy link
Collaborator

Signed-off-by: NAYANAR nayana.r5@ibm.com

🐛 Bug-fix PR

💡 Fix Description

How did you solve it? Key design points.
Implement password-policy visibility toggle in Change Password screen based on environment variables

🧪 Verification

Check Command Status
Lint suite make lint pass
Unit tests make test pass

📐 MCP Compliance (if relevant)

  • Matches current MCP spec
  • No breaking change to MCP clients

✅ Checklist

  • Code formatted (make black isort pre-commit)
  • No secrets/credentials committed

Copy link
Collaborator

@rakdutta rakdutta left a comment

Choose a reason for hiding this comment

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

The PR is not behaving as expected when PASSWORD_REQUIRE_UPPERCASE=True is enabled. During the server setup, the default admin@example.com user is still being created even though the default password does not contain any uppercase character, which violates the configured password policy. This indicates that the uppercase validation is not being enforced correctly at the time of default user creation.

…te context for the change-password page

Signed-off-by: NAYANAR <nayana.r5@ibm.com>
Signed-off-by: NAYANAR <nayana.r5@ibm.com>
…IRE_MIN_LENGTH is false

Signed-off-by: NAYANAR <nayana.r5@ibm.com>
…me of default user creation

Signed-off-by: NAYANAR <nayana.r5@ibm.com>
Signed-off-by: NAYANAR <nayana.r5@ibm.com>
Copy link
Collaborator

@rakdutta rakdutta left a comment

Choose a reason for hiding this comment

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

Tested the PR by modifying the following environment variables with all possible value combinations, and everything is working as expected.
Therefore, the PR is ready to be merged.

  • PASSWORD_MIN_LENGTH
  • PASSWORD_REQUIRE_UPPERCASE
  • PASSWORD_REQUIRE_LOWERCASE
  • PASSWORD_REQUIRE_NUMBERS
  • PASSWORD_REQUIRE_SPECIAL

@crivetimihai
Copy link
Member

  • PR user mgmt password policy #1569 already implemented this feature more completely
  • The password_require_numbers support is included in main but missing from this PR
  • No additional value would be gained from merging this PR

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.

5 participants