Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Reviewer's GuideThe PR migrates the nginx Shibboleth login proxy from PHP-FPM to a Python CGI script served by fcgiwrap, preserving the existing SHIB_ATTR_* backend payload while updating routing, container dependencies, supervision, and login endpoint configuration. Basic script behavior was tested, but nginx image-build and staging Shibboleth-login validation remain outstanding. Sequence diagram for the Python Shibboleth login proxysequenceDiagram
participant Browser
participant Nginx
participant LoginPy as login.py
participant Backend as WEKO backend
Browser->>Nginx: GET /secure/login.py?next=...
Nginx->>LoginPy: Execute via fcgiwrap
LoginPy->>LoginPy: Check HTTP_WEKOSOCIETYAFFILIATION
LoginPy->>Backend: POST /weko/shib/login with SHIB_ATTR_* fields
Backend-->>LoginPy: Redirect path or login page
alt redirect path
LoginPy-->>Nginx: Status: 302 Found and Location
Nginx-->>Browser: 302 redirect
else login page
LoginPy-->>Nginx: Status: 200 OK and HTML
Nginx-->>Browser: Login page
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 security issue, and 1 other issue
Security issues:
- 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)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="nginx/login.py" line_range="39" />
<code_context>
+ }
+
+ # Request to the Shibboleth login
+ response = requests.post(url, data=data, verify=False)
+ response.raise_for_status()
+ redirect = response.text
</code_context>
<issue_to_address>
**Backend TLS can be impersonated**
When traffic between nginx and the backend is intercepted, `requests.post(..., verify=False)` accepts an untrusted certificate, letting the interceptor read or alter the identity attributes and session ID and affect the login response; users can be impersonated or receive attacker-controlled content.
Enable certificate verification and configure the trusted CA for the backend.
</issue_to_address>
### Comment 2
<location path="nginx/login.py" line_range="39" />
<code_context>
response = requests.post(url, data=data, verify=False)
</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. 2 findings to address first, and the new login proxy posts Shibboleth identity and affiliation attributes with TLS certificate verification disabled, so an interceptor could read them or alter the login response. Reverting restores the previous login path, but any attributes disclosed or access granted while this is live cannot be undone.
Blocking findings: nginx/login.py:39, nginx/login.py:39
| } | ||
|
|
||
| # Request to the Shibboleth login | ||
| response = requests.post(url, data=data, verify=False) |
There was a problem hiding this comment.
🟠 High · Backend TLS can be impersonated
When traffic between nginx and the backend is intercepted, requests.post(..., verify=False) accepts an untrusted certificate, letting the interceptor read or alter the identity attributes and session ID and affect the login response; users can be impersonated or receive attacker-controlled content.
Enable certificate verification and configure the trusted CA for the backend.
Prompt for AI agents
In `nginx/login.py` at line 39:
**Backend TLS can be impersonated**
When traffic between nginx and the backend is intercepted, `requests.post(..., verify=False)` accepts an untrusted certificate, letting the interceptor read or alter the identity attributes and session ID and affect the login response; users can be impersonated or receive attacker-controlled content.
Enable certificate verification and configure the trusted CA for the backend.| } | ||
|
|
||
| # Request to the Shibboleth login | ||
| response = requests.post(url, data=data, verify=False) |
There was a problem hiding this comment.
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
upstream (RCOSDP/weko main) の nginx/login.py をベースに、現行バックエンドの /weko/shib/login が受け付ける SHIB_ATTR_* 形式のフォームデータを送るよう調整した。 - nginx/login.py を追加し、nginx/login.php を削除 - php-fpm を廃止し fcgiwrap + python3-requests で /secure を実行 - /secure/login.py/<path> 形式に対応するため fastcgi_split_path_info を追加 - WEKO_ACCOUNTS_SHIB_IDP_LOGIN_URL を secure/login.py に変更 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
概要
nginx の Shibboleth ログイン用スクリプト
login.phpを、upstream（main）のnginx/login.pyをベースにしたlogin.pyに置き換えます。mainのlogin.pyは fastcgi のパラメータ名（Shib-Session-ID,eppnなど）をそのまま POST し、それに対応するバックエンド側の変更（Shib-Session-ID受け取り、attribute_mapping設定）を前提としています。本ブランチのバックエンドはSHIB_ATTR_*形式を受け付けるため、login.pyはlogin.phpと同じSHIB_ATTR_*形式で送るよう調整しています。バックエンドの変更はありません。変更内容
nginx/login.pyを追加、nginx/login.phpを削除nginx/Dockerfile:php-fpm/php-curlをfcgiwrap/python3-requestsに置き換え、PHP 用の設定（sed）を削除nginx/supervisord.conf:php-fpmをfcgiwrap（www-data,/tmp/fcgiwrap.sock）に置き換えnginx/weko.conf:/secureの転送先をfcgiwrapに変更し、/secure/login.py/<path>形式に対応するためfastcgi_split_path_infoを追加WEKO_ACCOUNTS_SHIB_IDP_LOGIN_URLをsecure/login.pyに変更（config.py,scripts/instance.cfg）upstream の login.py との差分
Status: 302 Foundを使用nextを URL エンコード、エラー時の JavaScript 内 URL をエスケープlogin.phpと同じくSERVER_NAMEを使用テスト
login.pyを実行し、login.phpと同じ 5 項目（SHIB_ATTR_USER_NAME,SHIB_ATTR_EPPN,SHIB_ATTR_MAIL,SHIB_ATTR_SESSION_ID,SHIB_ATTR_ROLE_AUTHORITY_NAME）を POST し、返された URL へ 302 することを確認HTTP_WEKOSOCIETYAFFILIATIONがない場合に "Permission is invalid" のアラートを返すことを確認注意
/secureで PHP を使うのはlogin.phpだけだったため、イメージから PHP を削除しています。🤖 Generated with Claude Code
Summary by Sourcery
Replace the PHP-based Shibboleth login proxy with a Python CGI implementation and update nginx and application configuration to use it.
New Features:
Bug Fixes:
Enhancements:
Build: