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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
