Skip to content

Fix v0.9.26 hiroba w2026 64 hiroba - #1946

Merged
kanon-yoneji merged 16 commits into
RCOSDP:v0.9.26_hiroba_developfrom
ivis-weko3-dev:fix-v0.9.26_hiroba_W2026-64_hiroba
Oct 23, 2026
Merged

kanon-yoneji merged 16 commits into
RCOSDP:v0.9.26_hiroba_developfrom
ivis-weko3-dev:fix-v0.9.26_hiroba_W2026-64_hiroba

Conversation

@kanon-yoneji

@kanon-yoneji kanon-yoneji commented Oct 23, 2026 •

Copy link
Copy Markdown

概要 (Summary)

  • #63646
  • #63830
  • #63219

Summary by Sourcery

Harden authentication, search-data access, and billing workflows while migrating the Shibboleth login proxy from PHP to Python.

New Features:

  • Replace the Shibboleth login proxy with a Python CGI implementation and update its container integration.
  • Add server-side validation of billable file status and price before initiating payments, including handling price changes in the confirmation flow.

Bug Fixes:

  • Correct Shibboleth login proxy URLs from login.php to login.py.
  • Restrict search-data API access to authenticated users with the required item permissions.
  • Align Babel Jinja extraction settings across modules, including correcting silent configuration and supported extensions.

Enhancements:

  • Improve billing confirmation behavior by safely handling changed prices and centralizing charge-item session cleanup.

Build:

  • Update the Nginx image to use fcgiwrap and Python requests instead of PHP-FPM.

Deployment:

  • Update Nginx and instance configuration to serve the Python Shibboleth login proxy through fcgiwrap.

Tests:

  • Update account and search-data tests for the Python login proxy and permission-based access behavior.

kanon-yoneji and others added 16 commits May 8, 2026 15:32
W2025-33_early(release: 2026/03/30) from V0.9.26 hiroba develop
Update get_search_data API access control and tests(#63219)
…rom_W2026-64_hiroba

fix charge process to use server-calculated price and handle price changes(#63646)
…vert_php_to_py

Convert PHP files to Python and update related configuration(#63830)
@sourcery-ai

sourcery-ai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

This PR consolidates Jinja/Babel extraction settings, migrates the Shibboleth login proxy from PHP-FPM to a Python CGI served through fcgiwrap, adds permission enforcement for search-data APIs, and hardens file charging by recalculating prices server-side with client handling for price changes.

Sequence diagram for server-validated file charging

sequenceDiagram
    actor User
    participant Browser
    participant Charge as charge()
    participant Permission as get_file_charge_info()
    participant Payment as create_charge()

    User->>Browser: Click charge confirmation
    Browser->>Charge: GET /charge with item_id, file_name, price
    Charge->>Permission: get_file_charge_info(item_id, file_name)
    Permission-->>Charge: server_price, unit, status
    alt price differs
        Charge-->>Browser: price_changed with new price and unit
        Browser->>Browser: showChargeConfirmation()
    else price matches
        Charge->>Payment: create_charge(user_id, item_id, server_price, title, file_url, ret_url)
        Payment-->>Charge: redirect URL or payment result
        Charge-->>Browser: charge status
    end
Loading

Flow diagram for price-change charge recovery

flowchart TD
    Start[Charge confirmation accepted] --> Request[Browser calls /charge]
    Request --> Validate["get_file_charge_info()"]
    Validate --> Match{Requested price matches server price?}
    Match -->|Yes| Create["create_charge()"]
    Create --> Result[Show payment result]
    Match -->|No| Remove["removeChargeItem()"]
    Remove --> Update[Update button price and unit]
    Update --> Confirm["showChargeConfirmation()"]
    Confirm --> Request
Loading

File-Level Changes

Change Details Files
Standardize Babel Jinja extraction configuration across modules and enable the do extension.
  • Correct the misspelled silent option where present.
  • Add the asset, loop-control, i18n, and do extensions consistently to template extraction settings.
modules/*/babel.ini
Replace the Shibboleth PHP login proxy with a Python CGI implementation and update its container/runtime wiring.
  • Install fcgiwrap and Python requests instead of PHP-FPM dependencies.
  • Add a Python login proxy that validates affiliation, forwards Shibboleth attributes to the application, and returns redirects or rendered responses.
  • Update Nginx, Supervisor, configuration, and tests to use secure/login.py.
  • Remove obsolete PHP scripts and PHP-FPM configuration.
nginx/Dockerfile
nginx/Dockerfile.arm64
nginx/login.py
nginx/login.php
nginx/index.php
nginx/phpinfo.php
nginx/supervisord.conf
nginx/weko.conf
modules/weko-accounts/weko_accounts/config.py
modules/weko-accounts/tests/test_views.py
scripts/instance.cfg
Restrict search-data access to authenticated users with the required item permission.
  • Apply login_required and item_permission enforcement to the search-data endpoint.
  • Expand tests to cover unauthenticated requests and role-based 200/403 responses.
  • Update expected search results for the permitted user set.
modules/weko-items-ui/weko_items_ui/views.py
modules/weko-items-ui/tests/test_views.py
Make file charging server-authoritative and handle price changes during confirmation.
  • Resolve and validate the billable file and current charge status on the server.
  • Reject invalid, already purchased, or non-chargeable requests and use the server-calculated price for payment creation.
  • Return a price_changed response when the client price is stale or tampered with.
  • Update the confirmation UI and session-storage bookkeeping to redisplay the current price safely.
  • Use encoded query serialization and add translated price-change messaging.
modules/weko-records-ui/weko_records_ui/permissions.py
modules/weko-records-ui/weko_records_ui/views.py
modules/weko-records-ui/weko_records_ui/static/js/weko_records_ui/app.js
modules/weko-records-ui/weko_records_ui/templates/weko_records_ui/modal_charge_confirmation.html
modules/weko-records-ui/weko_records_ui/translations/en/LC_MESSAGES/messages.po
modules/weko-records-ui/weko_records_ui/translations/ja/LC_MESSAGES/messages.po
modules/weko-records-ui/weko_records_ui/translations/messages.pot
modules/weko-records-ui/weko_records_ui/translations/en/LC_MESSAGES/messages.mo
modules/weko-records-ui/weko_records_ui/translations/ja/LC_MESSAGES/messages.mo

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Oct 23, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4add921e-4902-4ea6-8418-6c25fac854d2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kanon-yoneji
kanon-yoneji merged commit d089676 into RCOSDP:v0.9.26_hiroba_develop Oct 23, 2026
1 of 84 checks passed

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 3 security issues

Security issues:

  • User controlled data in methods like innerHTML, outerHTML or document.write is an anti-pattern that can lead to XSS vulnerabilities (link)
  • User controlled data in a textarea.innerHTML is an anti-pattern that can lead to XSS vulnerabilities (link)
  • Certificate verification has been explicitly disabled. This permits insecure connections to insecure servers. Re-enable certification validation. (link)

Fixed security issues:

  • SSL verification is disabled but should not be (currently CURLOPT_SSL_VERIFYPEER= false) (link)
  • The 'phpinfo' function may reveal sensitive information about your environment. (link)
Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="modules/weko-records-ui/weko_records_ui/static/js/weko_records_ui/app.js" line_range="390" />
<code_context>
        textarea.innerHTML = String(value);
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-document-method):** User controlled data in methods like `innerHTML`, `outerHTML` or `document.write` is an anti-pattern that can lead to XSS vulnerabilities

*Source: opengrep*
</issue_to_address>

### Comment 2
<location path="modules/weko-records-ui/weko_records_ui/static/js/weko_records_ui/app.js" line_range="390" />
<code_context>
        textarea.innerHTML = String(value);
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `textarea.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities

*Source: opengrep*
</issue_to_address>

### Comment 3
<location path="nginx/login.py" line_range="50" />
<code_context>
    response = requests.post(url, data=data, verify=False, cookies=cookie_jar)
</code_context>
<issue_to_address>
**security (python.requests.security.disabled-cert-validation):** Certificate verification has been explicitly disabled. This permits insecure connections to insecure servers. Re-enable certification validation.

*Source: opengrep*
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 3 findings to address first, and the charge endpoint now uses the server-resolved price to create an external payment, so a defect in resolving the file’s billing metadata or price can result in an incorrect charge. Reverting would stop future charges but would not undo payments already processed.

Blocking findings: modules/weko-records-ui/weko_records_ui/static/js/weko_records_ui/app.js:390, modules/weko-records-ui/weko_records_ui/static/js/weko_records_ui/app.js:390, nginx/login.py:50


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

function showChargeConfirmation(price, unit, priceChanged, previousPrice, previousUnit) {
const getDisplayUnit = function (value) {
const textarea = document.createElement('textarea');
textarea.innerHTML = String(value);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security (javascript.browser.security.insecure-document-method): User controlled data in methods like innerHTML, outerHTML or document.write is an anti-pattern that can lead to XSS vulnerabilities

Source: opengrep

function showChargeConfirmation(price, unit, priceChanged, previousPrice, previousUnit) {
const getDisplayUnit = function (value) {
const textarea = document.createElement('textarea');
textarea.innerHTML = String(value);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security (javascript.browser.security.insecure-innerhtml): User controlled data in a textarea.innerHTML is an anti-pattern that can lead to XSS vulnerabilities

Source: opengrep

Comment thread nginx/login.py
cookie_jar = cookiejar.LWPCookieJar(temp_path)

# Request to the Shibboleth login
response = requests.post(url, data=data, verify=False, cookies=cookie_jar)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security (python.requests.security.disabled-cert-validation): Certificate verification has been explicitly disabled. This permits insecure connections to insecure servers. Re-enable certification validation.

Source: opengrep

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants