Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 1 addition & 5 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,11 @@ jobs:
with:
fetch-depth: 1

- name: Setup GitHub App Token
uses: anthropics/claude-code-action@v1
with:
github_token: ${{ secrets.GITHUB_TOKEN }}

- name: Run Claude Code Review
id: claude-review
uses: anthropics/claude-code-action@v1
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
prompt: |
Please review this pull request and provide feedback on:
Expand Down
82 changes: 48 additions & 34 deletions techblog_cms/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@
# Set up logging
logger = logging.getLogger(__name__)

# Detect testing mode early so dependent settings can branch consistently.
IS_TESTING = (
os.environ.get('TESTING') == 'True'
or 'PYTEST_CURRENT_TEST' in os.environ
or any(name.endswith('pytest') for name in sys.modules)
)

# Build paths inside the project like this: BASE_DIR / 'subdir'.
BASE_DIR = Path(__file__).resolve().parent.parent

Expand Down Expand Up @@ -76,46 +83,53 @@

WSGI_APPLICATION = 'techblog_cms.wsgi.application'

# Database
# Database
# Prefer DATABASE_URL when provided (12factor style)
db_url = os.environ.get('DATABASE_URL')
if db_url:
parsed = urlparse(db_url)
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.postgresql',
'NAME': parsed.path.lstrip('/') or os.environ.get('APP_DB_NAME') or os.environ.get('POSTGRES_DB', 'techblogdb'),
'USER': unquote(parsed.username or os.environ.get('APP_DB_USER') or os.environ.get('POSTGRES_USER', 'techblog')),
'PASSWORD': unquote(parsed.password or os.environ.get('APP_DB_PASSWORD') or os.environ.get('POSTGRES_PASSWORD', 'techblogpass')),
'HOST': parsed.hostname or os.environ.get('POSTGRES_HOST', 'db'),
'PORT': str(parsed.port or os.environ.get('POSTGRES_PORT', '5432')),
}
}
elif os.environ.get('APP_DB_USER') or os.environ.get('APP_DB_NAME') or os.environ.get('APP_DB_PASSWORD'):
# Next preference: dedicated app credentials if provided
if IS_TESTING:
# Unit tests use an in-memory SQLite database to avoid external service dependencies.
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.postgresql',
'NAME': os.environ.get('APP_DB_NAME', os.environ.get('POSTGRES_DB', 'techblogdb')),
'USER': os.environ.get('APP_DB_USER', os.environ.get('POSTGRES_USER', 'techblog')),
'PASSWORD': os.environ.get('APP_DB_PASSWORD', os.environ.get('POSTGRES_PASSWORD', 'techblogpass')),
'HOST': os.environ.get('POSTGRES_HOST', 'db'),
'PORT': os.environ.get('POSTGRES_PORT', '5432'),
'ENGINE': 'django.db.backends.sqlite3',
'NAME': ':memory:',
}
}
else:
# Fallback to generic POSTGRES_* variables
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.postgresql',
'NAME': os.environ.get('POSTGRES_DB', 'techblogdb'),
'USER': os.environ.get('POSTGRES_USER', 'techblog'),
'PASSWORD': os.environ.get('POSTGRES_PASSWORD', 'techblogpass'),
'HOST': os.environ.get('POSTGRES_HOST', 'db'),
'PORT': os.environ.get('POSTGRES_PORT', '5432'),
# Prefer DATABASE_URL when provided (12factor style)
db_url = os.environ.get('DATABASE_URL')
if db_url:
parsed = urlparse(db_url)
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.postgresql',
'NAME': parsed.path.lstrip('/') or os.environ.get('APP_DB_NAME') or os.environ.get('POSTGRES_DB', 'techblogdb'),
'USER': unquote(parsed.username or os.environ.get('APP_DB_USER') or os.environ.get('POSTGRES_USER', 'techblog')),
'PASSWORD': unquote(parsed.password or os.environ.get('APP_DB_PASSWORD') or os.environ.get('POSTGRES_PASSWORD', 'techblogpass')),
'HOST': parsed.hostname or os.environ.get('POSTGRES_HOST', 'db'),
'PORT': str(parsed.port or os.environ.get('POSTGRES_PORT', '5432')),
}
}
elif os.environ.get('APP_DB_USER') or os.environ.get('APP_DB_NAME') or os.environ.get('APP_DB_PASSWORD'):
# Next preference: dedicated app credentials if provided
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.postgresql',
'NAME': os.environ.get('APP_DB_NAME', os.environ.get('POSTGRES_DB', 'techblogdb')),
'USER': os.environ.get('APP_DB_USER', os.environ.get('POSTGRES_USER', 'techblog')),
'PASSWORD': os.environ.get('APP_DB_PASSWORD', os.environ.get('POSTGRES_PASSWORD', 'techblogpass')),
'HOST': os.environ.get('POSTGRES_HOST', 'db'),
'PORT': os.environ.get('POSTGRES_PORT', '5432'),
}
}
else:
# Fallback to generic POSTGRES_* variables
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.postgresql',
'NAME': os.environ.get('POSTGRES_DB', 'techblogdb'),
'USER': os.environ.get('POSTGRES_USER', 'techblog'),
'PASSWORD': os.environ.get('POSTGRES_PASSWORD', 'techblogpass'),
'HOST': os.environ.get('POSTGRES_HOST', 'db'),
'PORT': os.environ.get('POSTGRES_PORT', '5432'),
}
}
}

# Static files (CSS, JavaScript, Images)
# NOTE: Must start with a leading slash to avoid relative URLs like
Expand Down
2 changes: 2 additions & 0 deletions techblog_cms/templates/login.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ <h1>ログイン</h1>
<div class="alert alert-danger">{{ error }}</div>
{% endif %}
<form method="post">
{% if not IS_TESTING %}
{% csrf_token %}
Comment on lines 10 to 12

Choose a reason for hiding this comment

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

[P1] Skipping CSRF token when TESTING=True breaks login flows

When the TESTING flag is set (either via environment variable or the pytest detection in settings.py), the login template stops rendering {% csrf_token %} but the view continues to be protected by CsrfViewMiddleware. Any browser-based login against a server started with TESTING=True will therefore submit no CSRF token and receive a 403 before the view executes, effectively disabling sign‑in for staging/test environments. The tests still pass because Django’s test client doesn’t enforce CSRF, so this regression is silent.

Useful? React with 👍 / 👎.

{% endif %}
<div class="form-group">
<label for="username">ユーザー名</label>
<input id="username" name="username" type="text" class="form-control" required />
Expand Down
93 changes: 93 additions & 0 deletions techblog_cms/tests/test_login_security.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
from django.test import TestCase, Client
from django.contrib.auth.models import User
from django.urls import reverse
import time


class LoginSecurityTests(TestCase):
"""Test cases for login security to prevent username enumeration attacks."""

def setUp(self):
self.client = Client()
# Create a test user
self.test_user = User.objects.create_user(
username='testuser',
password='testpass123'
)
self.login_url = reverse('login') # assuming the login view is named 'login'

def test_login_error_message_consistency(self):
"""Test that error messages are generic for both existing and non-existing users."""
# Test with non-existing user
response_nonexistent = self.client.post(self.login_url, {
'username': 'nonexistentuser',
'password': 'wrongpassword'
})

# Test with existing user but wrong password
response_wrong_password = self.client.post(self.login_url, {
'username': 'testuser',
'password': 'wrongpassword'
})

# Both should return the same generic error message
self.assertEqual(response_nonexistent.status_code, 401)
self.assertEqual(response_wrong_password.status_code, 401)

# Check that error messages are the same and generic
error_msg_nonexistent = response_nonexistent.context.get('error', '')
error_msg_wrong_password = response_wrong_password.context.get('error', '')

# Both should have the same generic error message
self.assertEqual(error_msg_nonexistent, error_msg_wrong_password)

# Error message should be generic (not revealing username existence)
self.assertEqual(error_msg_nonexistent, 'Invalid credentials')

def test_successful_login(self):
"""Test that successful login still works correctly."""
response = self.client.post(self.login_url, {
'username': 'testuser',
'password': 'testpass123'
})

# Should redirect to dashboard on successful login
self.assertEqual(response.status_code, 302)
self.assertRedirects(response, reverse('dashboard'))

def test_login_timing_consistency(self):
"""Test that login attempts take similar time regardless of username existence."""
# This is a basic timing test - in a real security audit, more sophisticated timing analysis would be needed

times_nonexistent = []
times_wrong_password = []

# Run multiple attempts to get average timing
for _ in range(5):
# Time non-existent user login
start_time = time.time()
self.client.post(self.login_url, {
'username': 'nonexistentuser',
'password': 'wrongpassword'
})
times_nonexistent.append(time.time() - start_time)

# Time existing user with wrong password
start_time = time.time()
self.client.post(self.login_url, {
'username': 'testuser',
'password': 'wrongpassword'
})
times_wrong_password.append(time.time() - start_time)

# Calculate averages
avg_nonexistent = sum(times_nonexistent) / len(times_nonexistent)
avg_wrong_password = sum(times_wrong_password) / len(times_wrong_password)

# The difference should be minimal (less than 50ms)
# This is a very basic test - sophisticated timing attacks require more precise measurements
time_difference = abs(avg_nonexistent - avg_wrong_password)
self.assertLess(time_difference, 0.05,
f"Timing difference too large: {time_difference:.3f}s. "
f"Avg nonexistent: {avg_nonexistent:.3f}s, "
f"Avg wrong password: {avg_wrong_password:.3f}s")
3 changes: 2 additions & 1 deletion techblog_cms/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ def login_view(request):
login(request, user)
return redirect('dashboard')
else:
return render(request, 'login.html', {"error": "ユーザー名またはパスワードが違います。"}, status=401)
# Share the same generic message regardless of which field was incorrect to avoid enumeration.
return render(request, 'login.html', {"error": "Invalid credentials"}, status=401)
return render(request, 'login.html')


Expand Down