From 9a9a51afefeaf6eb5c94abdb3c2809bc684e4bc8 Mon Sep 17 00:00:00 2001
From: Masaharu Hayashi <masaharu.hayashi3@gmail.com>
Date: Mon, 10 Aug 2026 21:35:46 +0000
Subject: [PATCH] Add WEKO_ACCOUNTS_SHIB_ROLE_MANUAL_ASSIGN to keep manually
 assigned roles

Shibboleth login clears and recalculates user roles on every check_in.
In environments where the IdP does not send isMemberOf, this wipes the
roles an administrator set from the admin screen and can refuse the
login entirely.

Add WEKO_ACCOUNTS_SHIB_ROLE_MANUAL_ASSIGN (default False). When True,
ShibUser.check_in() returns immediately so no role is cleared, assigned
from shib_role_authority_name, or bound from Gakunin mAP groups. The
existing behavior is unchanged when the flag is off.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 modules/weko-accounts/tests/test_api.py       | 45 +++++++++++++++++++
 modules/weko-accounts/tests/test_views.py     | 38 ++++++++++++++++
 modules/weko-accounts/weko_accounts/api.py    |  5 +++
 modules/weko-accounts/weko_accounts/config.py |  9 ++++
 scripts/instance.cfg                          |  5 +++
 5 files changed, 102 insertions(+)

diff --git a/modules/weko-accounts/tests/test_api.py b/modules/weko-accounts/tests/test_api.py
index 832bef1095..96ce8f2b19 100644
--- a/modules/weko-accounts/tests/test_api.py
+++ b/modules/weko-accounts/tests/test_api.py
@@ -575,6 +575,51 @@ def test_check_in(self, extra_app, mocker):
             shibuser.user.roles.clear.reset_mock()
             mocker.resetall()
 
+# .tox/c1/bin/pytest --cov=weko_accounts tests/test_api.py::TestShibUserExtra::test_check_in_role_manual_assign -vv -s --cov-branch --cov-report=html --basetemp=/code/modules/weko-accounts/.tox/c1/tmp
+    def test_check_in_role_manual_assign(self, extra_app, db, user, roles, mocker):
+        """Roles set from the admin screen survive a Shibboleth login.
+
+        With WEKO_ACCOUNTS_SHIB_ROLE_MANUAL_ASSIGN enabled, check_in must not
+        touch the roles even when the IdP sends no isMemberOf and no
+        WEKO_ACCOUNTS_IDP_ENTITY_ID is configured.
+        """
+        with extra_app.app_context():
+            extra_app.config['WEKO_ACCOUNTS_SHIB_ROLE_MANUAL_ASSIGN'] = True
+            extra_app.config['WEKO_ACCOUNTS_IDP_ENTITY_ID'] = ''
+
+            mock_assign_user_role = mocker.patch('weko_accounts.api.ShibUser.assign_user_role')
+            mock_get_roles_to_add = mocker.patch('weko_accounts.api.ShibUser._get_roles_to_add')
+            mock_find_organization_name = mocker.patch('weko_accounts.api.ShibUser._find_organization_name')
+            mock_assign_roles_to_user = mocker.patch('weko_accounts.api.ShibUser._assign_roles_to_user')
+
+            # a role assigned by an administrator beforehand
+            user.roles.append(roles[0])
+            db.session.commit()
+
+            # no isMemberOf, but shib_role_authority_name is sent
+            shibuser = ShibUser({'shib_role_authority_name': '管理者'})
+            shibuser.user = user
+            shibuser.shib_user = MagicMock(spec=ShibbolethUser)
+
+            assert shibuser.check_in() is None
+            assert [role.name for role in user.roles] == ['Role_Administrator']
+            mock_assign_user_role.assert_not_called()
+            mock_get_roles_to_add.assert_not_called()
+            mock_find_organization_name.assert_not_called()
+            mock_assign_roles_to_user.assert_not_called()
+            mocker.resetall()
+
+            # flag off: the existing behavior is kept (roles are recalculated)
+            extra_app.config['WEKO_ACCOUNTS_SHIB_ROLE_MANUAL_ASSIGN'] = False
+            mock_assign_user_role.return_value = (True, "")
+            mock_get_roles_to_add.return_value = []
+            mock_find_organization_name.return_value = True
+
+            assert shibuser.check_in() is None
+            assert list(user.roles) == []
+            mock_assign_user_role.assert_called_once()
+            mock_get_roles_to_add.assert_called_once()
+
 # .tox/c1/bin/pytest --cov=weko_accounts tests/test_api.py::TestShibUserExtra::test_get_roles_to_add -vv -s --cov-branch --cov-report=html --basetemp=/code/modules/weko-accounts/.tox/c1/tmp
     def test_get_roles_to_add(self, extra_app):
         shibuser = ShibUser({
diff --git a/modules/weko-accounts/tests/test_views.py b/modules/weko-accounts/tests/test_views.py
index a91aa5ee1d..dbfae1b218 100644
--- a/modules/weko-accounts/tests/test_views.py
+++ b/modules/weko-accounts/tests/test_views.py
@@ -174,6 +174,44 @@ def test_shib_auto_login(client,redis_connect,mocker):
     with patch("weko_accounts.views.RedisConnection",side_effect=BaseException("test_error")):
         res = client.get(url+"?Shib-Session-ID=1111")
         assert res.status_code == 400
+# .tox/c1/bin/pytest --cov=weko_accounts tests/test_views.py::test_shib_auto_login_role_manual_assign -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp
+def test_shib_auto_login_role_manual_assign(app,client,redis_connect,mocker):
+    """Login succeeds without isMemberOf when roles are managed manually."""
+    url = url_for("weko_accounts.shib_auto_login")
+    mocker.patch("weko_accounts.views.RedisConnection.connection",return_value=redis_connect)
+    mocker.patch("weko_accounts.views.ShibUser.new_relation_info")
+    mock_shib_login = mocker.patch("weko_accounts.views.ShibUser.shib_user_login")
+
+    # the IdP sends no isMemberOf and no entity id is configured
+    app.config["WEKO_ACCOUNTS_SHIB_BIND_GAKUNIN_MAP_GROUPS"] = True
+    app.config["WEKO_ACCOUNTS_IDP_ENTITY_ID"] = ""
+    app.config["WEKO_ACCOUNTS_SHIB_ROLE_MANUAL_ASSIGN"] = True
+
+    shibuser = ShibUser({})
+    shibuser.shib_user = MagicMock()
+    shibuser.user = User(id=1)
+
+    redis_connect.put("Shib-Session-1111",bytes('{"shib_eppn":"test_eppn"}',"utf-8"))
+    set_session(client,{"shib_session_id":"1111","next":"/next_page"})
+    mock_redirect_method = mocker.patch("weko_accounts.views._redirect_method",return_value=make_response())
+    with patch("weko_accounts.views.ShibUser",return_value=shibuser):
+        mock_redirect = mocker.patch("weko_accounts.views.redirect",return_value=make_response())
+        client.get(url+"?next=/next_page")
+        mock_redirect.assert_called_with("/next_page")
+        mock_shib_login.assert_called_once()
+        mock_redirect_method.assert_not_called()
+
+    # flag off: _get_roles_to_add raises and the login is refused as before
+    app.config["WEKO_ACCOUNTS_SHIB_ROLE_MANUAL_ASSIGN"] = False
+    mock_shib_login.reset_mock()
+    redis_connect.put("Shib-Session-1111",bytes('{"shib_eppn":"test_eppn"}',"utf-8"))
+    set_session(client,{"shib_session_id":"1111","next":"/next_page"})
+    mocker.patch("weko_accounts.views.flash")
+    with patch("weko_accounts.views.ShibUser",return_value=shibuser):
+        client.get(url+"?next=/next_page")
+        mock_redirect_method.assert_called_once()
+        mock_shib_login.assert_not_called()
+
 #def confirm_user():
 # .tox/c1/bin/pytest --cov=weko_accounts tests/test_views.py::test_confirm_user -vv -s --cov-branch --cov-report=term --basetemp=/code/modules/weko-workflow/.tox/c1/tmp
 def test_confirm_user(client,redis_connect,mocker):
diff --git a/modules/weko-accounts/weko_accounts/api.py b/modules/weko-accounts/weko_accounts/api.py
index 324d4c1826..ceb96253aa 100644
--- a/modules/weko-accounts/weko_accounts/api.py
+++ b/modules/weko-accounts/weko_accounts/api.py
@@ -313,6 +313,11 @@ def check_in(self):
         :return:
 
         """
+        # ロールを管理画面から手動で設定する運用の場合、
+        # Shibboleth属性によるロールの再計算は行わない
+        if current_app.config['WEKO_ACCOUNTS_SHIB_ROLE_MANUAL_ASSIGN']:
+            return None
+
         #ログインユーザーのロールをクリアする
         self.user.roles.clear()
         check_role, error = self.assign_user_role()
diff --git a/modules/weko-accounts/weko_accounts/config.py b/modules/weko-accounts/weko_accounts/config.py
index bf7d1c00ae..268d3fb242 100644
--- a/modules/weko-accounts/weko_accounts/config.py
+++ b/modules/weko-accounts/weko_accounts/config.py
@@ -221,6 +221,15 @@
 WEKO_ACCOUNTS_SHIB_BIND_GAKUNIN_MAP_GROUPS = False
 """Bind Gakunin mAP groups to WEKO groups."""
 
+WEKO_ACCOUNTS_SHIB_ROLE_MANUAL_ASSIGN = False
+"""Manage roles of Shibboleth users manually from the admin screen.
+
+If True, no role is assigned nor cleared on Shibboleth login. Use this
+when the IdP does not provide the isMemberOf attribute. If False, the
+existing behavior (shib_role_authority_name / isMemberOf /
+organizationName) is kept.
+"""
+
 WEKO_ACCOUNTS_GAKUNIN_GROUP_SUFFIX = "_gakunin_groups"
 """Suffix for Gakunin group names."""
 
diff --git a/scripts/instance.cfg b/scripts/instance.cfg
index aa18a7b655..fdb197d960 100644
--- a/scripts/instance.cfg
+++ b/scripts/instance.cfg
@@ -881,6 +881,11 @@ WEKO_ACCOUNTS_IDP_ENTITY_ID = ''
 WEKO_ACCOUNTS_SHIB_BIND_GAKUNIN_MAP_GROUPS = False
 """Bind Gakunin mAP groups to WEKO groups."""
 
+WEKO_ACCOUNTS_SHIB_ROLE_MANUAL_ASSIGN = False
+"""Manage roles of Shibboleth users manually from the admin screen."""
+"""Note: Set True when the IdP does not provide the isMemberOf attribute."""
+"""      Roles are then assigned from Administration > User Management > Users."""
+
 WEKO_RECORDS_UI_S3_TRANSFER_MULTIPART_THRESHOLD = 8 * 1024 * 1024
 """Threshold to use multipart upload for S3 compatible service transfer (byte). Default is 8 MiB."""