diff --git a/application/api/oidc/routes.py b/application/api/oidc/routes.py index e349ebfb..5bf8e4fe 100644 --- a/application/api/oidc/routes.py +++ b/application/api/oidc/routes.py @@ -79,10 +79,7 @@ def _state_cookie_secure() -> bool: def _frontend_redirect(fragment: str): base = (settings.OIDC_FRONTEND_URL or "").rstrip("/") - response = redirect(f"{base}/#{fragment}", code=302) - # Every callback exit consumes the one-shot state cookie. - response.delete_cookie(STATE_COOKIE_NAME, path=STATE_COOKIE_PATH) - return response + return redirect(f"{base}/#{fragment}", code=302) def _no_store(payload, status: int = 200) -> Response: @@ -453,6 +450,22 @@ def oidc_refresh(): if effective.get("sid"): identity["oidc_sid"] = effective["sid"] + # Re-check revocation right before minting, against the (possibly remapped) + # identity but with the ORIGINAL session's iat: a back-channel logout or + # SCIM deny that landed during the IdP grant — or one targeting the + # refreshed sub/sid — sets a watermark newer than this iat and must block + # renewal. The renewed token's own (fresh) iat would post-date the + # watermark and slip past the per-request check, so anchor on the old iat. + if denylist.is_denied( + { + "sub": identity["sub"], + "oidc_sub": identity.get("oidc_sub"), + "oidc_sid": identity.get("oidc_sid"), + "iat": decoded.get("iat"), + } + ): + return make_response(jsonify({"error": "token_revoked"}), 401) + new_token, new_jti = _mint_session_token(identity) new_refresh = tokens.get("refresh_token") or refresh_token try: diff --git a/application/api/scim/routes.py b/application/api/scim/routes.py index eac8c4d8..4018646b 100644 --- a/application/api/scim/routes.py +++ b/application/api/scim/routes.py @@ -331,7 +331,10 @@ def replace_user(user_pk: str): row = UsersRepository(conn).get_by_pk(user_pk) if not row: return _scim_error(404, "User not found") - if "userName" in body and body["userName"] != row["user_id"]: + if "userName" in body and str(body["userName"]).lower() != str(row["user_id"]).lower(): + # userName is case-insensitive (caseExact=false), so a PUT that + # echoes the stored name under different casing is NOT a change — + # rejecting it would block deprovision for case-divergent IdPs. return _scim_error(400, "userName is immutable", "mutability") if desired is not None: row = _apply_active(conn, row, desired) diff --git a/tests/test_oidc.py b/tests/test_oidc.py index 3c00e684..b60e3a53 100644 --- a/tests/test_oidc.py +++ b/tests/test_oidc.py @@ -1412,6 +1412,28 @@ class TestRefreshRoute: assert response.get_json() == {"error": "token_revoked"} assert "oidc:refresh:jti-1" in fake_redis.store + def test_revocation_during_grant_blocks_renewal(self, client, fake_redis): + # is_denied passes the pre-grant check, but a back-channel logout lands + # during the IdP grant; the pre-mint re-check (anchored on the old iat) + # must catch it instead of minting a session that escapes the watermark. + self.denylist.is_denied.side_effect = [False, True] + token = make_session_token() + fake_redis.store["oidc:refresh:jti-1"] = b"rt-old" + id_claims = id_token_claims(sid="sess-1") + del id_claims["nonce"] + response, _ = self._refresh( + client, + token, + idp_response={ + "access_token": "at-2", + "refresh_token": "rt-new", + "id_token": sign_id_token(id_claims), + }, + ) + + assert response.status_code == 401 + assert response.get_json() == {"error": "token_revoked"} + def test_disabled_user_rejected(self, client, fake_redis): self.db.users.get.return_value = {"user_id": "oidc-user-1", "active": False} token = make_session_token() diff --git a/tests/test_scim.py b/tests/test_scim.py index 0fa2578f..851950a6 100644 --- a/tests/test_scim.py +++ b/tests/test_scim.py @@ -393,6 +393,22 @@ class TestReplaceUser: "alice@example.com", "scim_deactivated", metadata={"via": "scim"} ) + def test_put_differently_cased_username_deprovisions(self, client, scim_settings, scim_mocks): + # userName is case-insensitive: a PUT echoing the stored name under + # different casing must deactivate, not 400 "userName is immutable". + row = _user_row(active=True) # user_id == "alice@example.com" + scim_mocks.users.get_by_pk.return_value = row + scim_mocks.users.set_active.return_value = {**row, "active": False} + response = client.put( + f"/scim/v2/Users/{USER_PK}", + headers=AUTH, + json={"userName": "Alice@Example.com", "active": False}, + ) + assert response.status_code == 200 + assert response.get_json()["active"] is False + scim_mocks.users.set_active.assert_called_once_with(USER_PK, False) + scim_mocks.deny_user.assert_called_once_with("alice@example.com") + def test_put_unchanged_active_has_no_side_effects(self, client, scim_settings, scim_mocks): scim_mocks.users.get_by_pk.return_value = _user_row(active=True) response = client.put(