rusackas commented on code in PR #39469:
URL: https://github.com/apache/superset/pull/39469#discussion_r3616477637
##########
superset/cli/reset.py:
##########
@@ -68,7 +69,11 @@ def factory_reset(
# Validate the user
password = click.prompt("Admin Password", hide_input=True)
user = security_manager.find_user(username)
- if not user or not check_password_hash(user.password, password):
+ if (
+ not user
+ or not user.is_active
+ or not verify_auth_db_password(user.password, password)
Review Comment:
The CLI runs with shell access on the host, inside the operator trust
boundary per SECURITY.md, so login-failure bookkeeping doesn't buy us anything
there (that shell can read the metadata DB outright). Web logins keep the full
security-manager flow. Resolving.
##########
superset-frontend/src/utils/generateAuthDbPassword.test.ts:
##########
@@ -0,0 +1,143 @@
+/**
+ * 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 {
+ AUTH_DB_DEFAULT_PASSWORD_POLICY,
+ AUTH_DB_PASSWORD_MIN_LENGTH,
+ generateAuthDbPassword,
+ getAuthDbPasswordPolicyChecks,
+ getAuthDbPasswordPolicyError,
+ satisfiesDefaultAuthDbPasswordPolicy,
+} from './generateAuthDbPassword';
+
+test('generateAuthDbPassword returns policy-compliant passwords', () => {
+ for (let i = 0; i < 20; i += 1) {
+ const pwd = generateAuthDbPassword();
+ expect(Array.from(pwd).length).toBeGreaterThanOrEqual(
+ AUTH_DB_PASSWORD_MIN_LENGTH,
+ );
+ expect(satisfiesDefaultAuthDbPasswordPolicy(pwd)).toBe(true);
+ }
+});
+
+test('generateAuthDbPassword honors a custom minimum length policy', () => {
+ const policy = {
+ ...AUTH_DB_DEFAULT_PASSWORD_POLICY,
+ password_min_length: 20,
+ };
+ for (let i = 0; i < 10; i += 1) {
+ const pwd = generateAuthDbPassword(policy);
+ expect(Array.from(pwd).length).toBeGreaterThanOrEqual(20);
+ expect(getAuthDbPasswordPolicyError(pwd, policy)).toBeNull();
+ }
+});
+
+test('getAuthDbPasswordPolicyChecks counts Unicode code points for min
length', () => {
+ const policy = {
+ ...AUTH_DB_DEFAULT_PASSWORD_POLICY,
+ password_min_length: 2,
+ password_require_uppercase: false,
+ password_require_lowercase: false,
+ password_require_digit: false,
+ password_require_special: false,
+ password_common_list_check: false,
+ };
+ expect(getAuthDbPasswordPolicyChecks('a😀', policy).minLength).toBe(true);
+ expect(getAuthDbPasswordPolicyChecks('😀', policy).minLength).toBe(false);
+});
+
+test('getAuthDbPasswordPolicyError respects disabled uppercase requirement',
() => {
+ const policy = {
+ ...AUTH_DB_DEFAULT_PASSWORD_POLICY,
+ password_require_uppercase: false,
+ };
+ const password = 'abcdefghijklm1!';
+ expect(getAuthDbPasswordPolicyError(password, policy)).toBeNull();
+});
+
+test('getAuthDbPasswordPolicyError rejects bcrypt passwords over the byte
limit', () => {
+ const password = `Aa1!${'x'.repeat(69)}`;
+ expect(getAuthDbPasswordPolicyError(password)).toMatch(/72 bytes/);
+});
+
+test('getAuthDbPasswordPolicyError skips bcrypt byte limit for argon2', () => {
+ const policy = {
+ ...AUTH_DB_DEFAULT_PASSWORD_POLICY,
+ password_hash_algorithm: 'argon2' as const,
+ password_require_uppercase: false,
+ password_require_lowercase: false,
+ password_require_digit: false,
+ password_require_special: false,
+ password_common_list_check: false,
+ password_min_length: 1,
Review Comment:
Looks like this landed as the shared `POLICY_WITH_ONLY_MIN_LENGTH` fixture,
resolving.
##########
superset/security/manager.py:
##########
@@ -1334,22 +1349,50 @@ def reset_password(self, userid: Union[int, str],
password: str) -> None:
bypassed. We distinguish the two by comparing the acting user
(``g.user``) against the target ``userid``: they match for a
self-service reset and differ for an admin reset.
+
+ On ``AUTH_DB``, the per-user session auth stamp is rotated so every
+ other session for the target account is invalidated — including when an
+ administrator resets a compromised password.
"""
+ # pylint: disable=import-outside-toplevel
+ from flask_appbuilder.const import AUTH_DB
+
super().reset_password(userid, password)
+ try:
+ target_user_id = int(userid)
+ except (TypeError, ValueError):
+ target_user_id = None
+
acting_user = getattr(g, "user", None)
acting_user_id = getattr(acting_user, "id", None)
# ``userid`` arrives as a string (the ``pk`` request arg) on the admin
# path, so coerce both sides before comparing.
- is_self_service = acting_user_id is not None and self._same_user(
- acting_user_id, userid
+ is_self_service = (
+ acting_user_id is not None
+ and target_user_id is not None
+ and self._same_user(acting_user_id, userid)
)
- if is_self_service:
+
+ if (
+ current_app.config.get("AUTH_TYPE") == AUTH_DB
+ and target_user_id is not None
+ ):
+ from superset.utils.auth_session_stamp import (
+ bump_user_session_auth_stamp,
+ sync_session_auth_stamp_on_login,
+ )
+
+ bump_user_session_auth_stamp(target_user_id)
+ if is_self_service and acting_user is not None:
+ sync_session_auth_stamp_on_login(acting_user)
+
+ if is_self_service and target_user_id is not None:
from superset.security.password_change import (
clear_password_must_change,
)
- clear_password_must_change(int(userid))
+ clear_password_must_change(target_user_id)
Review Comment:
This landed... `UserPasswordChanged` now fires through the configured event
logger for both the self-service and admin flows. Resolving.
--
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]
