From 3310ff103b6729e8eae7af38507c67ab27162dbd Mon Sep 17 00:00:00 2001 From: enieuwy Date: Mon, 29 Jun 2026 23:18:52 +0800 Subject: [PATCH] fix: yield to the user on federated login pages instead of thrashing During the manual-login wait, _complete_login_from_current_page re-clicks the SSO/OpenAthens entries every 3s unless _is_human_login_page() returns True. That detector only recognized Chinese-academia hosts (.edu.cn etc.), so on login.openathens.net it kept returning False -> the loop navigated the page on a 3s cycle and the user could never enter their institution/credentials. Recognize federated-access hubs (OpenAthens, SeamlessAccess) and common IdP providers (Azure AD, Okta, Auth0, Duo, Ping) as human-login pages, and add international academic host suffixes (.edu.au, .ac.uk, .ac.nz, .edu.sg, ...). The automation never fills credentials, so treating any real login/IdP page as 'wait' is safe; publisher pages are unaffected and still get the SSO-entry click. Adds a unit test covering OpenAthens/SeamlessAccess/Azure/Duo/AU+UK IdPs as human-login and publisher pages as not. --- instsci/publisher_batch.py | 27 +++++++++++++++ tests/test_human_login_page.py | 63 ++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 tests/test_human_login_page.py diff --git a/instsci/publisher_batch.py b/instsci/publisher_batch.py index 8186141..56202cc 100644 --- a/instsci/publisher_batch.py +++ b/instsci/publisher_batch.py @@ -1079,6 +1079,23 @@ def _is_human_login_page(self, page: Any) -> bool: host = (parsed.hostname or parsed.netloc or "").lower() if host.endswith("id.tsinghua.edu.cn") or host.endswith("idp.tsinghua.edu.cn"): return True + # Federated-access hubs and identity providers where a human must act + # (choose institution, enter credentials, complete 2FA). The login loop + # MUST yield here; otherwise it keeps re-clicking SSO entries and + # thrashes the page so the user can never type. The automation never + # fills credentials, so treating any real login/IdP page as "wait" is + # always safe. + federation_host_suffixes = ( + "openathens.net", + "seamlessaccess.org", + "microsoftonline.com", + "okta.com", + "auth0.com", + "duosecurity.com", + "pingidentity.com", + ) + if any(host == s or host.endswith("." + s) for s in federation_host_suffixes): + return True institution_host_suffixes = ( ".edu.cn", ".edu", @@ -1086,6 +1103,16 @@ def _is_human_login_page(self, page: Any) -> bool: ".edu.hk", ".edu.tw", ".edu.mo", + ".edu.au", + ".ac.uk", + ".ac.nz", + ".edu.sg", + ".ac.jp", + ".ac.za", + ".ac.in", + ".edu.in", + ".ac.kr", + ".edu.my", ) if not any(host.endswith(suffix) for suffix in institution_host_suffixes): return False diff --git a/tests/test_human_login_page.py b/tests/test_human_login_page.py new file mode 100644 index 0000000..02e5f98 --- /dev/null +++ b/tests/test_human_login_page.py @@ -0,0 +1,63 @@ +import types +import unittest + +from instsci.publisher_batch import PublisherBatchDownloader + + +def _page(url): + return types.SimpleNamespace(url=url) + + +def _shim(): + # _is_human_login_page only calls self._title/self._body_text as a last + # resort; the cases below resolve from the URL/host before that. + s = types.SimpleNamespace() + s._title = lambda page: "" + s._body_text = lambda page, n=0: "" + return s + + +def _is_human(url): + return PublisherBatchDownloader._is_human_login_page(_shim(), _page(url)) + + +class HumanLoginPageTests(unittest.TestCase): + """The login loop must YIELD on real login/IdP pages, not re-click them. + + Regression: login.openathens.net was not recognized, so the loop thrashed + the page and the user could never enter institution/credentials. + """ + + def test_openathens_waits(self): + self.assertTrue( + _is_human("https://login.openathens.net/auth/gen?t=%2Fsaml%2F2%2Fsso") + ) + + def test_seamlessaccess_waits(self): + self.assertTrue(_is_human("https://seamlessaccess.org/ds/")) + + def test_azure_ad_waits(self): + self.assertTrue( + _is_human("https://login.microsoftonline.com/common/oauth2/authorize") + ) + + def test_duo_2fa_waits(self): + self.assertTrue(_is_human("https://api-abc.duosecurity.com/frame/web/v1/auth")) + + def test_au_university_idp_waits(self): + self.assertTrue( + _is_human("https://idp.une.edu.au/idp/profile/SAML2/Redirect/SSO") + ) + + def test_uk_university_idp_waits(self): + self.assertTrue(_is_human("https://sso.cam.ac.uk/login")) + + def test_publisher_pages_do_not_wait(self): + # Publisher article / SSO-entry pages must NOT be treated as human-login, + # so the loop still clicks the institutional-access entry button. + self.assertFalse(_is_human("https://onlinelibrary.wiley.com/doi/10.1002/bdm.2118")) + self.assertFalse(_is_human("https://www.tandfonline.com/doi/full/10.1080/x")) + + +if __name__ == "__main__": + unittest.main()