You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/06/02 04:04:19 UTC
[PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
rusackas opened a new pull request, #40670:
URL: https://github.com/apache/superset/pull/40670
> **Draft / `hold:testing`** β enables a password policy by default (behavior change on password-setting flows). Needs end-to-end validation before merge.
### SUMMARY
`SupersetSecurityManager` enforced **no** password policy, so DB-auth self-registration and password changes accepted trivially short or common passwords (FINDING-014 / 015 β ASVS 6.2.1, 6.2.4, CWE-521).
This adds `superset.security.password_complexity.validate_password_complexity`:
- **minimum length** via `AUTH_PASSWORD_MIN_LENGTH` (default **8**), and
- a **common-password blocklist** (built-in set of the most common passwords; extendable via `AUTH_PASSWORD_COMMON_BLOCKLIST`).
It's wired through Flask-AppBuilder's `FAB_PASSWORD_COMPLEXITY_ENABLED` + `FAB_PASSWORD_COMPLEXITY_VALIDATOR`. FAB runs this callable from **both** the WTForms password fields (self-registration, user edit, reset password) **and** the User REST API β so a single function covers every password-setting flow, with clean validation errors. The policy is intentionally less draconian than FAB's built-in `default_password_complexity` (which requires 2 uppercase + 1 special + 2 digits + 3 lowercase + length 10).
### WHY DRAFT (`hold:testing`)
Enabling the policy by default changes behavior: short/common passwords are now rejected at registration, password change, and API-driven user provisioning. Needs validation that the form flows and the User API surface the error cleanly and that no existing automated provisioning relies on weak passwords.
### TESTING INSTRUCTIONS
```
pytest tests/unit_tests/security/test_password_complexity.py
```
Tests: strong password accepted; short rejected; common rejected (case-insensitive); configurable min length; extra blocklist honored.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [x] Introduces new feature or API (config-driven policy)
- [ ] Removes existing feature or API
π€ Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #40670:
URL: https://github.com/apache/superset/pull/40670#discussion_r3402412377
##########
tests/unit_tests/security/test_password_complexity.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+
+from superset.security.password_complexity import validate_password_complexity
+
+
+def test_validate_password_complexity_accepts_strong_password() -> None:
Review Comment:
Done... added docstrings to all the new tests in this file.
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,121 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ raw_min_length = current_app.config.get(
+ "AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH
+ )
+ # Operators commonly wire config via env vars, so AUTH_PASSWORD_MIN_LENGTH can
+ # arrive as a string (or be left unset/None). Coerce defensively and fall back
+ # to the default rather than blowing up every password-setting flow with a
+ # TypeError on the length comparison.
+ try:
+ min_length = int(raw_min_length)
+ except (TypeError, ValueError):
+ min_length = DEFAULT_MIN_LENGTH
+
+ if len(password) < min_length:
Review Comment:
Good catch. Non-positive values fall back to the default min length, with a test covering 0 and -1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #40670:
URL: https://github.com/apache/superset/pull/40670#discussion_r3338881988
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,105 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ min_length = current_app.config.get("AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH)
+ if len(password) < min_length:
Review Comment:
`AUTH_PASSWORD_MIN_LENGTH` is pulled directly from config and assumed to be a positive `int`. If an operator sets it to a string/None (common when wiring via env vars), `len(password) < min_length` will raise a `TypeError` and break all password-setting flows. Consider validating/coercing the config value up-front and failing with a clear error when misconfigured.
##########
superset/config.py:
##########
@@ -343,6 +343,20 @@ def _try_json_readsha(filepath: str, length: int) -> str | None:
RATELIMIT_APPLICATION = "50 per second"
AUTH_RATE_LIMITED = True
AUTH_RATE_LIMIT = "5 per second"
+
+# Password complexity policy, enforced (via Flask-AppBuilder) across
+# self-registration, the user edit/reset forms, and the User REST API.
+# The Superset validator requires a minimum length and rejects common
+# passwords; tune via AUTH_PASSWORD_MIN_LENGTH / AUTH_PASSWORD_COMMON_BLOCKLIST,
+# or replace FAB_PASSWORD_COMPLEXITY_VALIDATOR with your own callable.
+from superset.security.password_complexity import ( # noqa: E402
+ validate_password_complexity as _validate_password_complexity,
+)
+
+FAB_PASSWORD_COMPLEXITY_ENABLED = True
+FAB_PASSWORD_COMPLEXITY_VALIDATOR = _validate_password_complexity
+AUTH_PASSWORD_MIN_LENGTH = 8
+AUTH_PASSWORD_COMMON_BLOCKLIST: list[str] = []
Review Comment:
This enables password complexity enforcement by default (`FAB_PASSWORD_COMPLEXITY_ENABLED = True`), which is a user-visible behavior change for registration/reset/user provisioning. Superset typically records βenabled by defaultβ security/behavior changes in `UPDATING.md`; please add a release-note entry so operators arenβt surprised during upgrades.
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,105 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ min_length = current_app.config.get("AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH)
+ if len(password) < min_length:
+ raise PasswordComplexityValidationError(
+ __(
+ "Password must be at least %(min_length)s characters long.",
+ min_length=min_length,
+ )
+ )
+
+ extra = current_app.config.get("AUTH_PASSWORD_COMMON_BLOCKLIST") or []
+ blocklist = COMMON_PASSWORDS | {str(item).lower() for item in extra}
+ if password.lower() in blocklist:
Review Comment:
`AUTH_PASSWORD_COMMON_BLOCKLIST` is treated as an iterable of items, but if itβs misconfigured as a string (or other non-sequence), the set comprehension will iterate characters and the policy will behave unexpectedly. Also, using `casefold()` is a more correct case-insensitive comparison for potential non-ASCII passwords.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #40670:
URL: https://github.com/apache/superset/pull/40670
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40670:
URL: https://github.com/apache/superset/pull/40670#discussion_r3393718237
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,121 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ raw_min_length = current_app.config.get(
+ "AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH
+ )
+ # Operators commonly wire config via env vars, so AUTH_PASSWORD_MIN_LENGTH can
+ # arrive as a string (or be left unset/None). Coerce defensively and fall back
+ # to the default rather than blowing up every password-setting flow with a
+ # TypeError on the length comparison.
+ try:
+ min_length = int(raw_min_length)
+ except (TypeError, ValueError):
+ min_length = DEFAULT_MIN_LENGTH
+
+ if len(password) < min_length:
+ raise PasswordComplexityValidationError(
+ __(
+ "Password must be at least %(min_length)s characters long.",
+ min_length=min_length,
+ )
+ )
+
+ extra = current_app.config.get("AUTH_PASSWORD_COMMON_BLOCKLIST") or []
+ # A bare string is iterable but would be split into characters, so treat a
+ # misconfigured string as a single entry. casefold() gives correct
+ # case-insensitive matching for non-ASCII passwords too.
+ if isinstance(extra, str):
+ extra = [extra]
+ blocklist = COMMON_PASSWORDS | {str(item).casefold() for item in extra}
+ if password.casefold() in blocklist:
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-20: Whitespace Bypass in Blocklist</b></div>
<div id="fix">
The blocklist check at line 118 does not strip whitespace, allowing passwords like ` password123 ` to bypass the common-password blocklist. An attacker can set a weak password with leading/trailing spaces that passes validation. Normalize with `password.strip().casefold()` before the blocklist lookup. (See also: [CWE-20](https://cwe.mitre.org/data/definitions/20.html))
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset/security/password_complexity.py
+++ b/superset/security/password_complexity.py
@@ -117,3 +117,5 @@ def validate_password_complexity(password: str) -> None:
blocklist = COMMON_PASSWORDS | {str(item).casefold() for item in extra}
- if password.casefold() in blocklist:
+ # Strip whitespace before blocklist check to prevent bypassing with padded
+ # common passwords (e.g., " password123 " should be rejected like "password123").
+ if password.strip().casefold() in blocklist:
raise PasswordComplexityValidationError(
__("This password is too common; please choose a less guessable one.")
)
```
</div>
</details>
</div>
<small><i>Code Review Run #5ad6c5</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #40670:
URL: https://github.com/apache/superset/pull/40670#discussion_r3393779506
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,121 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ raw_min_length = current_app.config.get(
+ "AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH
+ )
+ # Operators commonly wire config via env vars, so AUTH_PASSWORD_MIN_LENGTH can
+ # arrive as a string (or be left unset/None). Coerce defensively and fall back
+ # to the default rather than blowing up every password-setting flow with a
+ # TypeError on the length comparison.
+ try:
+ min_length = int(raw_min_length)
+ except (TypeError, ValueError):
+ min_length = DEFAULT_MIN_LENGTH
+
+ if len(password) < min_length:
+ raise PasswordComplexityValidationError(
+ __(
+ "Password must be at least %(min_length)s characters long.",
+ min_length=min_length,
+ )
+ )
+
+ extra = current_app.config.get("AUTH_PASSWORD_COMMON_BLOCKLIST") or []
+ # A bare string is iterable but would be split into characters, so treat a
+ # misconfigured string as a single entry. casefold() gives correct
+ # case-insensitive matching for non-ASCII passwords too.
+ if isinstance(extra, str):
+ extra = [extra]
+ blocklist = COMMON_PASSWORDS | {str(item).casefold() for item in extra}
+ if password.casefold() in blocklist:
Review Comment:
I don't think this is really a bypass. ` password123 ` is a different (and longer) credential the user has to type identically every time, not the common `password123` we're blocking. Stripping would just start rejecting passwords that aren't actually in the blocklist, so I'd rather leave the comparison literal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4666327842
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4665335040
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4692008404
@sha174n added integration tests that hit `POST /api/v1/security/users/` with short, common, and compliant passwords, so a future FAB bump that silently drops enforcement will fail in CI. Still mulling 8 vs. 12 for the default... will circle back on that one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4664275678
<!-- translation-regression-bot -->
## β οΈ Translation Regression Detected
A source change in this PR renamed or reworded strings, invalidating existing translations (they are now `#, fuzzy`) in `ar`, `ca`, `cs`, `de`, `fi`, `fr`, `it`, `ja`, `ko`, `lv`, `mi`, `nl`, `pt`, `ru`, `sk`, `sl`, `th`, `tr`, `uk`. Please resolve the affected `.po` files before merging.
_Note: intentionally **deleting** a translatable string is not a regression and is not flagged here β only translations invalidated by a renamed/reworded source string are._
| Language | Fuzzy before | Fuzzy after | New |
|----------|-------------:|------------:|----:|
| `ar` | 1136 | 1139 | +3 |
| `ca` | 815 | 818 | +3 |
| `cs` | 787 | 790 | +3 |
| `de` | 997 | 1000 | +3 |
| `fi` | 4823 | 4826 | +3 |
| `fr` | 1014 | 1017 | +3 |
| `it` | 1667 | 1670 | +3 |
| `ja` | 322 | 325 | +3 |
| `ko` | 1521 | 1524 | +3 |
| `lv` | 295 | 298 | +3 |
| `mi` | 807 | 810 | +3 |
| `nl` | 1143 | 1146 | +3 |
| `pt` | 1839 | 1842 | +3 |
| `ru` | 293 | 296 | +3 |
| `sk` | 774 | 777 | +3 |
| `sl` | 963 | 966 | +3 |
| `th` | 4823 | 4826 | +3 |
| `tr` | 786 | 788 | +2 |
| `uk` | 293 | 296 | +3 |
### How to fix
**1. Install dependencies** (if not already set up):
```bash
pip install -r superset/translations/requirements.txt
sudo apt-get install gettext # or: brew install gettext
```
**2. Re-extract strings and sync `.po` files:**
```bash
./scripts/translations/babel_update.sh
```
This rewrites `superset/translations/messages.pot` from the current source files and merges the changes into every `.po` file. Strings whose `msgid` changed will be marked `#, fuzzy`.
**3. Resolve the fuzzy entries** in the affected language files (`ar`, `ca`, `cs`, `de`, `fi`, `fr`, `it`, `ja`, `ko`, `lv`, `mi`, `nl`, `pt`, `ru`, `sk`, `sl`, `th`, `tr`, `uk`):
```bash
grep -n '#, fuzzy' superset/translations/<lang>/LC_MESSAGES/messages.po
```
For each fuzzy entry, either rewrite the `msgstr` to match the new string and remove the `#, fuzzy` line, or clear the `msgstr` to `""` if you cannot provide a translation.
**4. Commit your changes to the `.po` files.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4693472572
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4677665235
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4598635721
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40670?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `63.15789%` with `7 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 55.15%. Comparing base ([`a183582`](https://app.codecov.io/gh/apache/superset/commit/a183582291ea020bae95bc10969c73c0470459d3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5efd69d`](https://app.codecov.io/gh/apache/superset/commit/5efd69d77de93e4e15bf7d75bb798e4d0f11552b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 180 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40670?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/security/password\_complexity.py](https://app.codecov.io/gh/apache/superset/pull/40670?src=pr&el=tree&filepath=superset%2Fsecurity%2Fpassword_complexity.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvc2VjdXJpdHkvcGFzc3dvcmRfY29tcGxleGl0eS5weQ==) | 50.00% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40670?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (a183582) and HEAD (5efd69d). Click for more details.
>
> <details><summary>HEAD has 162 uploads less than BASE</summary>
>
>| Flag | BASE (a183582) | HEAD (5efd69d) |
>|------|------|------|
>|python|138|2|
>|presto|14|1|
>|hive|14|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #40670 +/- ##
==========================================
- Coverage 64.18% 55.15% -9.03%
==========================================
Files 2591 2652 +61
Lines 138471 142172 +3701
Branches 32120 32568 +448
==========================================
- Hits 88883 78421 -10462
- Misses 48056 63063 +15007
+ Partials 1532 688 -844
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/40670/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/40670/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.76% <63.15%> (+0.35%)` | :arrow_up: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/40670/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/40670/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/40670/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.37% <63.15%> (+0.27%)` | :arrow_up: |
| [python](https://app.codecov.io/gh/apache/superset/pull/40670/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.43% <63.15%> (-19.14%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/40670/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/40670/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/40670?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4598625878
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | 5efd69d77de93e4e15bf7d75bb798e4d0f11552b |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a1e5643234f82000800bf62 |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-40670--superset-docs-preview.netlify.app](https://deploy-preview-40670--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">π±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">π€</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpLCoAgEADQu8y6aDKT6AYdw89IguVgQkl099y9xXuBczq4wArQgcn6tHuzJ10G1td1p-x620akJ5Tac4rB1lYdcUx1cy0rPdKs5CQm6ReBiAui8UrA9wM) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40670--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #40670:
URL: https://github.com/apache/superset/pull/40670#discussion_r3384768418
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,105 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ min_length = current_app.config.get("AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH)
+ if len(password) < min_length:
Review Comment:
Good catch β `AUTH_PASSWORD_MIN_LENGTH` is now coerced with `int()` and falls back to the default (8) when it is `None` or non-numeric, so a string/env-var misconfiguration no longer raises a `TypeError` across the password-setting flows. Added a regression test.
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,105 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ min_length = current_app.config.get("AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH)
+ if len(password) < min_length:
+ raise PasswordComplexityValidationError(
+ __(
+ "Password must be at least %(min_length)s characters long.",
+ min_length=min_length,
+ )
+ )
+
+ extra = current_app.config.get("AUTH_PASSWORD_COMMON_BLOCKLIST") or []
+ blocklist = COMMON_PASSWORDS | {str(item).lower() for item in extra}
+ if password.lower() in blocklist:
Review Comment:
Done β a string-valued `AUTH_PASSWORD_COMMON_BLOCKLIST` is now wrapped into a single-element list so it is no longer split into characters, and both the blocklist and the password comparison use `casefold()` for correct case-insensitive matching of non-ASCII input. Added a test covering the string-misconfiguration case.
##########
superset/config.py:
##########
@@ -343,6 +343,20 @@ def _try_json_readsha(filepath: str, length: int) -> str | None:
RATELIMIT_APPLICATION = "50 per second"
AUTH_RATE_LIMITED = True
AUTH_RATE_LIMIT = "5 per second"
+
+# Password complexity policy, enforced (via Flask-AppBuilder) across
+# self-registration, the user edit/reset forms, and the User REST API.
+# The Superset validator requires a minimum length and rejects common
+# passwords; tune via AUTH_PASSWORD_MIN_LENGTH / AUTH_PASSWORD_COMMON_BLOCKLIST,
+# or replace FAB_PASSWORD_COMPLEXITY_VALIDATOR with your own callable.
+from superset.security.password_complexity import ( # noqa: E402
+ validate_password_complexity as _validate_password_complexity,
+)
+
+FAB_PASSWORD_COMPLEXITY_ENABLED = True
+FAB_PASSWORD_COMPLEXITY_VALIDATOR = _validate_password_complexity
+AUTH_PASSWORD_MIN_LENGTH = 8
+AUTH_PASSWORD_COMMON_BLOCKLIST: list[str] = []
Review Comment:
Agreed β added an `UPDATING.md` entry under "Next" documenting that the policy is enabled by default, the min-length/blocklist behavior change for new and reset passwords, and the config knobs (`AUTH_PASSWORD_MIN_LENGTH`, `AUTH_PASSWORD_COMMON_BLOCKLIST`, `FAB_PASSWORD_COMPLEXITY_VALIDATOR`, and `FAB_PASSWORD_COMPLEXITY_ENABLED = False` to disable).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40670:
URL: https://github.com/apache/superset/pull/40670#discussion_r3401926850
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,121 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ raw_min_length = current_app.config.get(
+ "AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH
+ )
+ # Operators commonly wire config via env vars, so AUTH_PASSWORD_MIN_LENGTH can
+ # arrive as a string (or be left unset/None). Coerce defensively and fall back
+ # to the default rather than blowing up every password-setting flow with a
+ # TypeError on the length comparison.
+ try:
+ min_length = int(raw_min_length)
+ except (TypeError, ValueError):
+ min_length = DEFAULT_MIN_LENGTH
+
+ if len(password) < min_length:
Review Comment:
**Suggestion:** `AUTH_PASSWORD_MIN_LENGTH` is coerced to `int` but never validated to be positive. If an operator sets `0` or a negative value (for example via env), the length check is effectively disabled and very weak passwords can be accepted. Clamp invalid/non-positive values back to the secure default (or reject startup config) before comparing password length. [security]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
- β Password length policy can be disabled via misconfiguration.
- β Self-registration form accepts single-character passwords when misconfigured.
- β User REST API allows trivially weak passwords when misconfigured.
- β Violates ASVS password minimum length requirements.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. In `superset/config.py:371-383`, observe that `FAB_PASSWORD_COMPLEXITY_VALIDATOR` is
set to `superset.security.password_complexity.validate_password_complexity` and that
`AUTH_PASSWORD_MIN_LENGTH = 8` configures the minimum password length used by the
validator for self-registration, user edit/reset forms, and the User REST API (as
documented in the surrounding comments).
2. Following the pattern in `tests/unit_tests/security/test_password_complexity.py:25-33`
(which modifies `current_app.config["AUTH_PASSWORD_MIN_LENGTH"]` before calling the
validator), start an app context and set `current_app.config["AUTH_PASSWORD_MIN_LENGTH"] =
0` (or `-1`).
3. Call `validate_password_complexity("a")` from `superset.security.password_complexity`
(see function definition at `superset/security/password_complexity.py:85-41`). In the body
at `superset/security/password_complexity.py:91-24`, `raw_min_length` is read from config,
`min_length = int(raw_min_length)` succeeds with value `0`, and the check `if
len(password) < min_length:` (line 103 in the PR hunk) evaluates to `1 < 0`, which is
false, so no `PasswordComplexityValidationError` is raised for a 1-character password.
4. In a running Superset instance where `AUTH_PASSWORD_MIN_LENGTH` is misconfigured to `0`
or a negative value (via config or environment overrides), every password-setting flow
that relies on FAB's validator (self-registration, user edit/reset forms, and the User
REST API configured in `superset/config.py:371-383`) will accept arbitrarily short
passwords, effectively disabling the minimum length policy and weakening authentication.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=81a03019845f4ea49e44c2d5fafda90a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=81a03019845f4ea49e44c2d5fafda90a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/security/password_complexity.py
**Line:** 98:103
**Comment:**
*Security: `AUTH_PASSWORD_MIN_LENGTH` is coerced to `int` but never validated to be positive. If an operator sets `0` or a negative value (for example via env), the length check is effectively disabled and very weak passwords can be accepted. Clamp invalid/non-positive values back to the secure default (or reject startup config) before comparing password length.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=f1564214c8b55aa5964e2652fdad09356a7137421089b644c0897df15a156e26&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=f1564214c8b55aa5964e2652fdad09356a7137421089b644c0897df15a156e26&reaction=dislike'>π</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40670:
URL: https://github.com/apache/superset/pull/40670#discussion_r3401896496
##########
tests/unit_tests/security/test_password_complexity.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+
+from superset.security.password_complexity import validate_password_complexity
+
+
+def test_validate_password_complexity_accepts_strong_password() -> None:
+ # No exception for a sufficiently long, uncommon password.
+ validate_password_complexity("a-Good-Long-Passphrase-42")
+
+
+def test_validate_password_complexity_rejects_short_password() -> None:
Review Comment:
**Suggestion:** Add a short docstring under this function definition to describe the short-password rejection scenario. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This newly introduced function is missing a docstring in the final file state. That matches the rule requiring newly added Python functions to be documented inline.
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1ff33295fd6540bdb67fd6ed3985e545&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1ff33295fd6540bdb67fd6ed3985e545&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/security/test_password_complexity.py
**Line:** 30:30
**Comment:**
*Custom Rule: Add a short docstring under this function definition to describe the short-password rejection scenario.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=70ccad604bd37b403f55480cf48f08b52b9af8f4f45fe92efc5fd653f2074de0&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=70ccad604bd37b403f55480cf48f08b52b9af8f4f45fe92efc5fd653f2074de0&reaction=dislike'>π</a>
##########
tests/unit_tests/security/test_password_complexity.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+
+from superset.security.password_complexity import validate_password_complexity
+
+
+def test_validate_password_complexity_accepts_strong_password() -> None:
Review Comment:
**Suggestion:** Add a concise docstring to this new test function describing the expected behavior it validates. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This is a newly added Python test function in a new file, and it has no docstring. The custom rule requires newly added Python functions and classes to include inline docstrings, so the suggestion identifies a real violation.
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=52693464bff247108bff94ba2bb56393&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=52693464bff247108bff94ba2bb56393&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/security/test_password_complexity.py
**Line:** 25:25
**Comment:**
*Custom Rule: Add a concise docstring to this new test function describing the expected behavior it validates.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=0f74fa265014181198886a18617da20a7f97d63e5b4b54c5957de15683110150&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=0f74fa265014181198886a18617da20a7f97d63e5b4b54c5957de15683110150&reaction=dislike'>π</a>
##########
tests/unit_tests/security/test_password_complexity.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+
+from superset.security.password_complexity import validate_password_complexity
+
+
+def test_validate_password_complexity_accepts_strong_password() -> None:
+ # No exception for a sufficiently long, uncommon password.
+ validate_password_complexity("a-Good-Long-Passphrase-42")
+
+
+def test_validate_password_complexity_rejects_short_password() -> None:
+ with pytest.raises(PasswordComplexityValidationError):
+ validate_password_complexity("short1") # < 8 chars
+
+
+def test_validate_password_complexity_rejects_common_password() -> None:
Review Comment:
**Suggestion:** Add a docstring to this test function to explain that it verifies common-password blocking and case-insensitive handling. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
The function is newly added and does not have a docstring. This is a direct match for the custom rule about documenting new Python functions and classes.
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=49e3ac420b5446979ff0ca75c5f9e655&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=49e3ac420b5446979ff0ca75c5f9e655&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/security/test_password_complexity.py
**Line:** 35:35
**Comment:**
*Custom Rule: Add a docstring to this test function to explain that it verifies common-password blocking and case-insensitive handling.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=855e331d3b09b35451e369e9656984cedc3d8fc4b1dc4bfae00ef8a415271c22&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=855e331d3b09b35451e369e9656984cedc3d8fc4b1dc4bfae00ef8a415271c22&reaction=dislike'>π</a>
##########
tests/unit_tests/security/test_password_complexity.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+
+from superset.security.password_complexity import validate_password_complexity
+
+
+def test_validate_password_complexity_accepts_strong_password() -> None:
+ # No exception for a sufficiently long, uncommon password.
+ validate_password_complexity("a-Good-Long-Passphrase-42")
+
+
+def test_validate_password_complexity_rejects_short_password() -> None:
+ with pytest.raises(PasswordComplexityValidationError):
+ validate_password_complexity("short1") # < 8 chars
+
+
+def test_validate_password_complexity_rejects_common_password() -> None:
+ # Common even though length >= 8.
+ with pytest.raises(PasswordComplexityValidationError):
+ validate_password_complexity("password123")
+ # Case-insensitive.
+ with pytest.raises(PasswordComplexityValidationError):
+ validate_password_complexity("PASSWORD123")
+
+
+def test_validate_password_complexity_honors_configured_min_length() -> None:
+ original = current_app.config.get("AUTH_PASSWORD_MIN_LENGTH")
+ current_app.config["AUTH_PASSWORD_MIN_LENGTH"] = 16
+ try:
+ with pytest.raises(PasswordComplexityValidationError):
+ validate_password_complexity("only12chars!") # 12 < 16
+ finally:
+ current_app.config["AUTH_PASSWORD_MIN_LENGTH"] = original
+
+
+def test_validate_password_complexity_honors_extra_blocklist() -> None:
Review Comment:
**Suggestion:** Add a short docstring to this test function indicating it checks custom blocklist enforcement. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This function is newly added and has no docstring. That is a valid instance of the custom rule violation requiring new Python functions to be documented inline.
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=83315805588f440b8e63076370689742&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=83315805588f440b8e63076370689742&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/security/test_password_complexity.py
**Line:** 54:54
**Comment:**
*Custom Rule: Add a short docstring to this test function indicating it checks custom blocklist enforcement.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=95beb52ad90f786d3bfdfd9d2786a6ddcccbffcc4cb31aef04e639dadcb7608d&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=95beb52ad90f786d3bfdfd9d2786a6ddcccbffcc4cb31aef04e639dadcb7608d&reaction=dislike'>π</a>
##########
tests/unit_tests/security/test_password_complexity.py:
##########
@@ -0,0 +1,97 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+
+from superset.security.password_complexity import validate_password_complexity
+
+
+def test_validate_password_complexity_accepts_strong_password() -> None:
+ # No exception for a sufficiently long, uncommon password.
+ validate_password_complexity("a-Good-Long-Passphrase-42")
+
+
+def test_validate_password_complexity_rejects_short_password() -> None:
+ with pytest.raises(PasswordComplexityValidationError):
+ validate_password_complexity("short1") # < 8 chars
+
+
+def test_validate_password_complexity_rejects_common_password() -> None:
+ # Common even though length >= 8.
+ with pytest.raises(PasswordComplexityValidationError):
+ validate_password_complexity("password123")
+ # Case-insensitive.
+ with pytest.raises(PasswordComplexityValidationError):
+ validate_password_complexity("PASSWORD123")
+
+
+def test_validate_password_complexity_honors_configured_min_length() -> None:
Review Comment:
**Suggestion:** Add an inline docstring to this function describing that it validates behavior with a configured minimum length. [custom_rule]
**Severity Level:** Minor β οΈ
<details>
<summary><b>Why it matters? π€ </b></summary>
This new test function lacks an inline docstring in the final file state. The suggestion correctly points out a real violation of the docstring rule.
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=fc9ed3404c3346b49c4be2e3260b5a4a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=fc9ed3404c3346b49c4be2e3260b5a4a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/security/test_password_complexity.py
**Line:** 44:44
**Comment:**
*Custom Rule: Add an inline docstring to this function describing that it validates behavior with a configured minimum length.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=2744325b10f08d77af7f93c42f41b17407b072e43e4b948ffbae6fb540729576&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=2744325b10f08d77af7f93c42f41b17407b072e43e4b948ffbae6fb540729576&reaction=dislike'>π</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40670:
URL: https://github.com/apache/superset/pull/40670#discussion_r3393780219
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,121 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ raw_min_length = current_app.config.get(
+ "AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH
+ )
+ # Operators commonly wire config via env vars, so AUTH_PASSWORD_MIN_LENGTH can
+ # arrive as a string (or be left unset/None). Coerce defensively and fall back
+ # to the default rather than blowing up every password-setting flow with a
+ # TypeError on the length comparison.
+ try:
+ min_length = int(raw_min_length)
+ except (TypeError, ValueError):
+ min_length = DEFAULT_MIN_LENGTH
+
+ if len(password) < min_length:
+ raise PasswordComplexityValidationError(
+ __(
+ "Password must be at least %(min_length)s characters long.",
+ min_length=min_length,
+ )
+ )
+
+ extra = current_app.config.get("AUTH_PASSWORD_COMMON_BLOCKLIST") or []
+ # A bare string is iterable but would be split into characters, so treat a
+ # misconfigured string as a single entry. casefold() gives correct
+ # case-insensitive matching for non-ASCII passwords too.
+ if isinstance(extra, str):
+ extra = [extra]
+ blocklist = COMMON_PASSWORDS | {str(item).casefold() for item in extra}
+ if password.casefold() in blocklist:
Review Comment:
<!-- Bito Reply -->
The concern regarding literal comparison is noted. If the intention is to block only exact matches of common passwords and avoid rejecting valid passwords that happen to contain common substrings, maintaining the current literal comparison is appropriate. The proposed change would indeed normalize input, which could lead to rejecting passwords that are not in the blocklist but contain common patterns as substrings.
**superset/security/password_complexity.py**
```
blocklist = COMMON_PASSWORDS | {str(item).casefold() for item in extra}
- if password.casefold() in blocklist:
+ # Strip whitespace before blocklist check to prevent bypassing with padded
+ # common passwords (e.g., " password123 " should be rejected like "password123").
+ if password.strip().casefold() in blocklist:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40670:
URL: https://github.com/apache/superset/pull/40670#discussion_r3384779952
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,105 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ min_length = current_app.config.get("AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH)
+ if len(password) < min_length:
+ raise PasswordComplexityValidationError(
+ __(
+ "Password must be at least %(min_length)s characters long.",
+ min_length=min_length,
+ )
+ )
+
+ extra = current_app.config.get("AUTH_PASSWORD_COMMON_BLOCKLIST") or []
+ blocklist = COMMON_PASSWORDS | {str(item).lower() for item in extra}
Review Comment:
**Suggestion:** The extra blocklist value is treated as a generic iterable, so if an operator provides a single string (for example from environment-driven config), it is split into characters and the intended password entries are never blocked. Normalize this setting to a list of full password strings (or explicitly reject plain strings) before building the blocklist. [security]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β Organization-specific weak passwords remain allowed despite blocklist.
- β οΈ Password policy weaker than operators expect or document.
- β οΈ Harder to enforce compliance with internal password standards.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Superset wires the password complexity validator in `superset/config.py:7-19`,
documenting that policy is tuned via `AUTH_PASSWORD_COMMON_BLOCKLIST` and setting
`FAB_PASSWORD_COMPLEXITY_VALIDATOR = _validate_password_complexity` so all
password-setting flows go through `validate_password_complexity()`.
2. The Docker config loader at `superset/docker/pythonpath_dev/superset_config.py:139-148`
imports `superset_config_docker`, which can override `AUTH_PASSWORD_COMMON_BLOCKLIST`; an
operator may define `AUTH_PASSWORD_COMMON_BLOCKLIST =
os.getenv("AUTH_PASSWORD_COMMON_BLOCKLIST", "AcmeCorp2024")`, leaving it as a single
string instead of a list.
3. At runtime, when a password is set via any FAB-backed flow (self-registration, user
edit/reset, or User REST API as described in `superset/config.py:7-11`), Flask-AppBuilder
calls `validate_password_complexity()` in
`superset/security/password_complexity.py:85-105`.
4. In `validate_password_complexity()`, line `100` assigns `extra =
current_app.config.get("AUTH_PASSWORD_COMMON_BLOCKLIST") or []`, so `extra` is the string
`"AcmeCorp2024"`, and line `101` builds `blocklist = COMMON_PASSWORDS | {str(item).lower()
for item in extra}`, iterating over characters instead of full passwords; the check at
line `102` (`if password.lower() in blocklist:`) does not match `"acmecorp2024"`, so the
weak, organization-specific password is accepted despite the operator's intent to block
it.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b4a4e858dfb54a26b0f1f86a55032c04&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b4a4e858dfb54a26b0f1f86a55032c04&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/security/password_complexity.py
**Line:** 100:101
**Comment:**
*Security: The extra blocklist value is treated as a generic iterable, so if an operator provides a single string (for example from environment-driven config), it is split into characters and the intended password entries are never blocked. Normalize this setting to a list of full password strings (or explicitly reject plain strings) before building the blocklist.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=4042fa7d6ebae86d3cc8b4a175ceb760e7ac5b67d413db3e28df141f7b82dab1&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=4042fa7d6ebae86d3cc8b4a175ceb760e7ac5b67d413db3e28df141f7b82dab1&reaction=dislike'>π</a>
##########
superset/security/password_complexity.py:
##########
@@ -0,0 +1,105 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Superset password-complexity validator.
+
+Wired in via ``FAB_PASSWORD_COMPLEXITY_VALIDATOR`` (with
+``FAB_PASSWORD_COMPLEXITY_ENABLED``). Flask-AppBuilder runs this callable from
+both the WTForms password fields (self-registration, user edit, reset password)
+and the User REST API, so a single function enforces the policy across all
+password-setting flows.
+
+The default policy is a minimum length plus a common-password blocklist β
+intentionally less draconian than FAB's built-in ``default_password_complexity``
+(which requires 2 uppercase, 1 special, 2 digits, 3 lowercase and length 10).
+"""
+
+from __future__ import annotations
+
+from flask import current_app
+from flask_appbuilder.exceptions import PasswordComplexityValidationError
+from flask_babel import gettext as __
+
+# A small built-in blocklist of the most common/guessable passwords. Operators
+# can extend it with AUTH_PASSWORD_COMMON_BLOCKLIST. (A fuller list or a
+# Have-I-Been-Pwned k-anonymity check is a possible follow-up.)
+COMMON_PASSWORDS: frozenset[str] = frozenset(
+ {
+ "123456",
+ "123456789",
+ "12345678",
+ "1234567890",
+ "12345",
+ "111111",
+ "123123",
+ "000000",
+ "password",
+ "password1",
+ "password123",
+ "passw0rd",
+ "qwerty",
+ "qwerty123",
+ "qwertyuiop",
+ "abc123",
+ "letmein",
+ "welcome",
+ "welcome1",
+ "admin",
+ "admin123",
+ "administrator",
+ "root",
+ "superset",
+ "changeme",
+ "iloveyou",
+ "monkey",
+ "dragon",
+ "sunshine",
+ "princess",
+ "football",
+ "baseball",
+ "trustno1",
+ "login",
+ "master",
+ "hello123",
+ "secret",
+ "default",
+ }
+)
+
+DEFAULT_MIN_LENGTH = 8
+
+
+def validate_password_complexity(password: str) -> None:
+ """Validate a plaintext password against the configured policy.
+
+ :raises PasswordComplexityValidationError: if the password is too short or
+ appears in the common-password blocklist.
+ """
+ min_length = current_app.config.get("AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH)
+ if len(password) < min_length:
Review Comment:
**Suggestion:** `AUTH_PASSWORD_MIN_LENGTH` is read from config without type coercion, so a common deployment pattern like `os.getenv("AUTH_PASSWORD_MIN_LENGTH", "8")` will pass a string and make `len(password) < min_length` raise a `TypeError` at runtime. Coerce and validate this setting to an integer before comparing so password-setting flows fail with a proper validation error instead of a server error. [type error]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β Self-registration fails with 500 when length config mis-typed.
- β Password reset API fails under misconfigured minimum length.
- β οΈ Operators get no clear feedback on configuration errors.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Superset loads default password configuration from `superset/config.py:7-19`, which
defines `AUTH_PASSWORD_MIN_LENGTH = 8` and wires `FAB_PASSWORD_COMPLEXITY_VALIDATOR =
_validate_password_complexity` so Flask-AppBuilder uses `validate_password_complexity()`
for all password-setting flows.
2. The Docker config loader at `superset/docker/pythonpath_dev/superset_config.py:139-148`
imports `superset_config_docker`, whose documented purpose is to override defaults; an
operator can define `AUTH_PASSWORD_MIN_LENGTH = os.getenv("AUTH_PASSWORD_MIN_LENGTH",
"16")` there, making `current_app.config["AUTH_PASSWORD_MIN_LENGTH"]` a string.
3. When a user hits any FAB-backed password flow (self-registration, user edit/reset form,
or User REST API as documented in `superset/config.py:7-11`), Flask-AppBuilder calls
`superset.security.password_complexity.validate_password_complexity()` at
`superset/security/password_complexity.py:85-92`.
4. Inside `validate_password_complexity()`, line `91` reads `min_length =
current_app.config.get("AUTH_PASSWORD_MIN_LENGTH", DEFAULT_MIN_LENGTH)` and line `92`
executes `if len(password) < min_length:` where `len(password)` is an `int` and
`min_length` is the string `"16"`, causing a `TypeError: '<' not supported between
instances of 'int' and 'str'` and turning the password-setting request into a 500 error
instead of a clean validation message.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c96a23a00db743b0a8ff7ef2d453d470&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c96a23a00db743b0a8ff7ef2d453d470&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/security/password_complexity.py
**Line:** 91:92
**Comment:**
*Type Error: `AUTH_PASSWORD_MIN_LENGTH` is read from config without type coercion, so a common deployment pattern like `os.getenv("AUTH_PASSWORD_MIN_LENGTH", "8")` will pass a string and make `len(password) < min_length` raise a `TypeError` at runtime. Coerce and validate this setting to an integer before comparing so password-setting flows fail with a proper validation error instead of a server error.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=fb6a1d582299e36ac9fd52e849f78d2567cc1417e0d961333b296ebe16bf9e68&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40670&comment_hash=fb6a1d582299e36ac9fd52e849f78d2567cc1417e0d961333b296ebe16bf9e68&reaction=dislike'>π</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "sha174n (via GitHub)" <gi...@apache.org>.
sha174n commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4691650141
Confirmed the usual paths are unaffected (`add_user()`/`fab create-admin` bypass the validator; only forms + the User REST API enforce), and the unit tests pass. Two suggestions:
1. **Default `AUTH_PASSWORD_MIN_LENGTH` to 12, not 8.** ASVS's length bar is 12; 8 is the older NIST floor, so the default is slightly weaker than the standard this targets. Still operator-configurable.
2. **Add one integration test through the real FAB path** (e.g. `POST /api/v1/security/users/` with a too-short password expecting a 4xx). The unit tests cover the validator function, but nothing asserts it's wired into the form/API layer β a future Flask-AppBuilder bump could drop enforcement with no test failing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4690942004
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(security): enforce password complexity policy (min length + common-password blocklist) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40670:
URL: https://github.com/apache/superset/pull/40670#issuecomment-4598617380
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org