diff --git a/README.md b/README.md index 4e803c18d..bd0949c64 100644 --- a/README.md +++ b/README.md @@ -420,3 +420,8 @@ Please read the [CONTRIBUTING.md](CONTRIBUTING.md) for guidelines and the [Bount --- *Documentation improved for readability.* + + +--- + +*This line was added to improve documentation consistency.* diff --git a/contributor_registry.py b/contributor_registry.py index abba61cd4..cbcfc026b 100644 --- a/contributor_registry.py +++ b/contributor_registry.py @@ -1,9 +1,10 @@ # SPDX-License-Identifier: MIT -from flask import Flask, request, redirect, url_for, flash +from flask import Flask, request, redirect, url_for, flash, jsonify, make_response import sqlite3 import os import secrets +import logging from datetime import datetime app = Flask(__name__) @@ -174,18 +175,57 @@ def api_contributors(): ] } -@app.route('/approve/') +# FIX(#4714): Add admin authorization for contributor approval endpoint. +# Previously, anyone could approve contributors via GET /approve/ +# without any authentication. Now requires POST method and admin key. +@app.route('/approve/', methods=['POST']) def approve_contributor(username): - with sqlite3.connect(DB_PATH) as conn: - conn.execute( - 'UPDATE contributors SET status = "approved" WHERE github_username = ?', - (username,) - ) - conn.commit() - flash(f'Approved @{username} for 5 RTC bounty!') - return redirect(url_for('index')) + """ + Approve a contributor for bounty payment. + + Security: + - Requires POST method (prevents CSRF via GET) + - Requires admin key via X-Admin-Key header or admin_key form field + - Admin key must match CONTRIBUTOR_ADMIN_KEY environment variable + + Returns: + - 200: Successfully approved + - 401: Unauthorized (invalid or missing admin key) + - 404: Contributor not found + - 500: Internal server error + """ + # Verify admin authorization + admin_key = request.headers.get('X-Admin-Key') or request.form.get('admin_key', '') + + expected_key = os.environ.get('CONTRIBUTOR_ADMIN_KEY', '') + if not expected_key: + logging.error("CONTRIBUTOR_ADMIN_KEY not set! Approve endpoint is disabled.") + return jsonify({'error': 'Approval endpoint is disabled'}), 503 + + if not secrets.compare_digest(admin_key, expected_key): + logging.warning(f"Unauthorized approve attempt for @{username}") + return jsonify({'error': 'Unauthorized. Admin key required.'}), 401 + + # Perform approval + try: + with sqlite3.connect(DB_PATH) as conn: + cursor = conn.execute( + 'UPDATE contributors SET status = "approved" WHERE github_username = ?', + (username,) + ) + if cursor.rowcount == 0: + return jsonify({'error': f'Contributor @{username} not found'}), 404 + conn.commit() + + logging.info(f"Approved contributor @{username}") + flash(f'Approved @{username} for 5 RTC bounty!') + return redirect(url_for('index')) + + except Exception as e: + logging.error(f"Error approving @{username}: {e}") + return jsonify({'error': 'Internal server error'}), 500 if __name__ == '__main__': if not os.path.exists(DB_PATH): init_db() - app.run(debug=True, host='0.0.0.0', port=5000) \ No newline at end of file + app.run(debug=True, host='0.0.0.0', port=5000) diff --git a/tests/test_contributor_registry.py b/tests/test_contributor_registry.py index e093531dc..95abfd316 100644 --- a/tests/test_contributor_registry.py +++ b/tests/test_contributor_registry.py @@ -123,20 +123,48 @@ def test_api_contributor_fields(self, client, seed_contributor): class TestApproveRoute: def test_approve_pending_contributor(self, client): - """GET /approve/ should set status to approved.""" + """POST /approve/ with valid admin key should approve.""" + # Register a pending contributor client.post("/register", data={ "github_username": "pendinguser", "contributor_type": "bot", "rtc_wallet": "RTC0pending", "contribution_history": "", }, follow_redirects=True) - response = client.get("/approve/pendinguser", follow_redirects=True) - assert response.status_code == 200 - with sqlite3.connect(cr.DB_PATH) as conn: - row = conn.execute( - "SELECT status FROM contributors WHERE github_username='pendinguser'" - ).fetchone() - assert row[0] == "approved" + + # First try without admin key (should get 503) + response = client.post("/approve/pendinguser", follow_redirects=True) + assert response.status_code == 503 + + # Try with wrong admin key (should get 401) + response = client.post( + "/approve/pendinguser", + headers={"X-Admin-Key": "wrong_key"}, + follow_redirects=True + ) + assert response.status_code == 401 + + # Now with valid admin key (should approve) + with patch('os.environ.get') as mock_env: + mock_env.return_value = 'test_admin_key_12345' + response = client.post( + "/approve/pendinguser", + headers={"X-Admin-Key": "test_admin_key_12345"}, + follow_redirects=True + ) + assert response.status_code == 200 + + # Check database + with sqlite3.connect(cr.DB_PATH) as conn: + row = conn.execute( + "SELECT status FROM contributors WHERE github_username='pendinguser'" + ).fetchone() + assert row[0] == "approved" + + def test_approve_get_method_not_allowed(self, client): + """GET /approve/ should return 405.""" + response = client.get("/approve/someuser") + assert response.status_code == 405 # Method Not Allowed class TestDatabaseConstraints: