Skip to content

fix widget permission issue (No. 290, 303, 304) #1930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ivis-miyachi merged 2 commits into from
Sep 29, 2026
Merged

fix widget permission issue (No. 290, 303, 304) #1930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
facd5ab
fix widget permission issue (No. 290, 303, 304)
wei-kuochen Sep 29, 2026
894f70f
add test code
wei-kuochen Sep 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions modules/weko-gridlayout/tests/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,13 +463,21 @@ def test_delete_widget_design_page(client, users):


# def load_widget_type():
def test_load_widget_type(client, users):
login_user_via_session(client=client, email=users[2]['obj'].email)
user_results2 = [
(0, 403),
(1, 200),
(2, 200),
(3, 200),
(4, 403),
]
@pytest.mark.parametrize('id, status_code', user_results2)
def test_load_widget_type(client, users, id, status_code):
login_user_via_session(client=client, email=users[id]['obj'].email)

@sourcery-ai sourcery-ai Bot Sep 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (testing): The test_load_widget_type test logs in with users[id]['obj'].email, but the fixture's entry at index 4 has email set to generaluser.email while obj is incorrectly set to sysadmin; the case expecting a 403 therefore logs in as a system administrator and does not test the general user's permission.

Triggers: When the parameterized case uses id == 4.

Suggested fix: Log in with users[id]['email'], as the uploaded-file test does, or correct the fixture's obj value before using it.

Suggested change
login_user_via_session(client=client, email=users[id]['obj'].email)
login_user_via_session(client=client, email=users[id]['email'])

res = client.get(
url_for("weko_gridlayout_api.load_widget_type"),
headers={"Content-Type": "application/json"}
)
assert res.status_code == 200
assert res.status_code == status_code


# def save_widget_item():
Expand Down Expand Up @@ -838,6 +846,24 @@ def test_upload_file(client, users, communities):


# def uploaded_file(filename, community_id=0):
user_results2 = [
(0, 403),
(1, 200),
(2, 200),
(3, 403),
(4, 403),
]
@pytest.mark.parametrize('id, status_code', user_results2)
def test_uploaded_file(client, users, id, status_code):
login_user_via_session(client=client, email=users[id]["email"])
with patch('weko_gridlayout.views.WidgetBucket.get_file', return_value="test"):
res = client.get(
url_for("weko_gridlayout.uploaded_file", community_id="Root Index", filename="file")
)
assert res.status_code == status_code
assert res.get_data(as_text=True) == "test"


def test_uploaded_file(client, communities):
# The view returns whatever get_file() gives it, so the stand-in has to be
# something Flask can turn into a response - a function is not.
Expand All @@ -848,7 +874,6 @@ def test_uploaded_file(client, communities):
assert res.status_code == 200
assert res.get_data(as_text=True) == "test"


# def unlocked_widget():
def test_unlocked_widget(client, users):
login_user_via_session(client=client, email=users[2]["email"])
Expand Down
9 changes: 9 additions & 0 deletions modules/weko-gridlayout/weko_gridlayout/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
from sqlalchemy.orm.exc import NoResultFound
from werkzeug.exceptions import NotFound
from invenio_db import db
from weko_accounts.utils import roles_required
from weko_admin.config import WEKO_ADMIN_PERMISSION_ROLE_REPO, \
WEKO_ADMIN_PERMISSION_ROLE_SYSTEM, WEKO_ADMIN_PERMISSION_ROLE_COMMUNITY

from .api import WidgetItems
from .config import WEKO_GRIDLAYOUT_ACCESS_COUNTER_TYPE
Expand Down Expand Up @@ -296,6 +299,9 @@ def delete_widget_design_page():

@blueprint_api.route('/load_widget_type', methods=['GET'])
@login_required
@roles_required([WEKO_ADMIN_PERMISSION_ROLE_SYSTEM,

@ivis-miyachi ivis-miyachi Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix no.290

WEKO_ADMIN_PERMISSION_ROLE_REPO,
WEKO_ADMIN_PERMISSION_ROLE_COMMUNITY])
Comment on lines +302 to +304

@sourcery-ai sourcery-ai Bot Sep 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): roles_required immediately calls the view for every method in Flask-Login's EXEMPT_METHODS, which includes GET, so these newly protected GET endpoints do not check authentication or roles. load_widget_type remains reachable by any authenticated user, and uploaded_file is reachable anonymously because it has no separate login_required decorator.

Triggers: When either widget endpoint is accessed with GET.

Suggested fix: Use an authorization decorator that does not exempt GET requests, or explicitly enforce authentication and the allowed roles inside these views.

def load_widget_type():
"""Get Widget Type List."""
results = get_widget_type_list()
Expand Down Expand Up @@ -592,6 +598,9 @@ def upload_file(community_id):
@blueprint.route('/widget/uploaded/<string:filename>/<string:community_id>',
methods=["GET"]
)
@roles_required([WEKO_ADMIN_PERMISSION_ROLE_SYSTEM,

@ivis-miyachi ivis-miyachi Sep 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix no.303, 304

WEKO_ADMIN_PERMISSION_ROLE_REPO,
WEKO_ADMIN_PERMISSION_ROLE_COMMUNITY])
def uploaded_file(filename, community_id=0):
"""Get widget static file.

Expand Down
Loading