sadpandajoe commented on code in PR #40695: URL: https://github.com/apache/superset/pull/40695#discussion_r3358358194
########## tests/integration_tests/security/session_invalidation_tests.py: ########## @@ -0,0 +1,78 @@ +# 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. + +from superset import db +from superset.models.user_attributes import UserAttribute +from tests.integration_tests.base_tests import SupersetTestCase + +USERNAME = "session_invalidation_user" +PASSWORD = "general" # noqa: S105 + + +class TestSessionInvalidation(SupersetTestCase): + def setUp(self) -> None: + self.create_user( + USERNAME, + PASSWORD, + "Admin", + email="[email protected]", + ) + db.session.commit() + + def tearDown(self) -> None: + if user := self.get_user(USERNAME): + db.session.query(UserAttribute).filter_by(user_id=user.id).delete() + db.session.delete(user) + db.session.commit() + + def _set_active(self, active: bool) -> None: + # Update via the ORM so the after_update event fires (mirrors the + # FAB admin UI / REST API path that flips ``active``). + user = self.get_user(USERNAME) + user.active = active + db.session.commit() + + def test_disabling_user_forces_logout_of_active_session(self) -> None: + self.login(USERNAME, PASSWORD) + + # Authenticated request works before the account is disabled. + assert self.client.get("/api/v1/me/").status_code == 200 + + # Disabling the account stamps the per-user invalidation epoch... + self._set_active(False) + user = self.get_user(USERNAME) + attribute = ( + db.session.query(UserAttribute).filter_by(user_id=user.id).one_or_none() + ) + assert attribute is not None + assert attribute.sessions_invalidated_at is not None + + # ...so the previously-authenticated session is now forced out. + assert self.client.get("/api/v1/me/").status_code != 200 Review Comment: Should we use `== 401` here? `!= 200` means the test would pass with 500 or 403. If we wanted to have more status codes we accept, we could maybe compare the `status_code` to an array of status codes we accept? -- 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]
