Skip to content

Commit 1bb2424

Browse files
committed
fix(api): redact login-persistence error log + cover missing-sub skip (#586)
Address PR review: - Log only the exception class on login-persistence failure; the raw message can carry SQL parameters (email, OIDC sub). - Add callback test: login enabled but token missing 'sub' creates no user row and leaves session['user_id'] unset.
1 parent 9041061 commit 1bb2424

2 files changed

Lines changed: 35 additions & 1 deletion

File tree

application/tests/web_main_test.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,6 +1217,38 @@ def test_callback_does_not_persist_when_login_flag_off(
12171217

12181218
self.assertEqual(sqla.session.query(db.User).count(), 0)
12191219

1220+
@patch("application.web.web_main.id_token")
1221+
@patch("application.web.web_main.CREFlow")
1222+
def test_callback_skips_persistence_when_sub_missing(
1223+
self, cre_flow_mock, id_token_mock
1224+
) -> None:
1225+
# Login enabled, but the OIDC token has no 'sub' claim: must not create
1226+
# a user (google_sub is NOT NULL) and must not set session['user_id'].
1227+
id_token_mock.verify_oauth2_token.return_value = {
1228+
"name": "Test User",
1229+
"email": "test@example.com",
1230+
}
1231+
flow_instance = cre_flow_mock.instance.return_value
1232+
flow_instance.flow.credentials._id_token = "tok"
1233+
self.app.secret_key = "test-secret"
1234+
with patch.dict(
1235+
os.environ,
1236+
{
1237+
"CRE_ENABLE_LOGIN": "1",
1238+
"LOGIN_ALLOWED_DOMAINS": "*",
1239+
"NO_LOAD_GRAPH_DB": "1",
1240+
"INSECURE_REQUESTS": "1",
1241+
},
1242+
):
1243+
with self.app.test_client() as client:
1244+
with client.session_transaction() as sess:
1245+
sess["state"] = "xyz"
1246+
client.get("/rest/v1/callback?state=xyz")
1247+
with client.session_transaction() as sess:
1248+
self.assertNotIn("user_id", sess)
1249+
1250+
self.assertEqual(sqla.session.query(db.User).count(), 0)
1251+
12201252
def test_deeplink(self) -> None:
12211253
self.maxDiff = None
12221254
collection = db.Node_collection().with_graph()

application/web/web_main.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1179,7 +1179,9 @@ def callback():
11791179
)
11801180
session["user_id"] = user.id
11811181
except Exception as e:
1182-
logger.error(f"failed to persist user on login: {e}")
1182+
# Avoid logging the raw exception: it can carry SQL parameters
1183+
# such as the user's email or OIDC subject. Log only the class.
1184+
logger.error("failed to persist user on login: %s", type(e).__name__)
11831185
return redirect("/chatbot")
11841186

11851187

0 commit comments

Comments
 (0)