Skip to content

Commit 375373a

Browse files
committed
Simplify MAAS key storage to SHA-256 hash only
Remove redundant dual key storage (Django's make_password + SHA-256). The 'key' field now stores only the SHA-256 hash used for lookups, eliminating the separate 'key_hash' field and the check_password step.
1 parent 44d5a00 commit 375373a

5 files changed

Lines changed: 52 additions & 57 deletions

File tree

maas/admin.py

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
from django import forms
22
from django.contrib import admin
3-
from django.contrib.auth.hashers import make_password
43
from django.utils.translation import gettext_lazy as _
54

65
from maas.models import MAASProvider, MAASApiKey, MAASUsageRecord, sha256_key
@@ -16,13 +15,13 @@ class MAASProviderAdminForm(forms.ModelForm):
1615
'placeholder': _('Enter a key or click Generate'),
1716
}
1817
),
19-
help_text=_('Enter a clear-text key, or click Generate. It will be hashed and stored securely. Leave blank when editing to keep the existing key.'),
18+
help_text=_('Enter a clear-text key, or click Generate. It will be hashed (SHA-256) and stored. Leave blank when editing to keep the existing key.'),
2019
)
2120

2221
class Meta:
2322
model = MAASProvider
2423
fields = '__all__'
25-
exclude = ('key', 'key_hash')
24+
exclude = ('key',)
2625

2726
def __init__(self, *args, **kwargs):
2827
super().__init__(*args, **kwargs)
@@ -32,8 +31,7 @@ def save(self, commit=True):
3231
instance = super().save(commit=False)
3332
raw_key = self.cleaned_data.get('clear_text_key', '').strip()
3433
if raw_key:
35-
instance.key = make_password(raw_key)
36-
instance.key_hash = sha256_key(raw_key)
34+
instance.key = sha256_key(raw_key)
3735
if commit:
3836
instance.save()
3937
return instance
@@ -49,13 +47,13 @@ class MAASApiKeyAdminForm(forms.ModelForm):
4947
'placeholder': _('Enter a key or click Generate'),
5048
}
5149
),
52-
help_text=_('Enter a clear-text key, or click Generate. It will be hashed and stored securely. Leave blank when editing to keep the existing key.'),
50+
help_text=_('Enter a clear-text key, or click Generate. It will be hashed (SHA-256) and stored. Leave blank when editing to keep the existing key.'),
5351
)
5452

5553
class Meta:
5654
model = MAASApiKey
5755
fields = '__all__'
58-
exclude = ('key', 'key_hash')
56+
exclude = ('key',)
5957

6058
def __init__(self, *args, **kwargs):
6159
super().__init__(*args, **kwargs)
@@ -65,8 +63,7 @@ def save(self, commit=True):
6563
instance = super().save(commit=False)
6664
raw_key = self.cleaned_data.get('clear_text_key', '').strip()
6765
if raw_key:
68-
instance.key = make_password(raw_key)
69-
instance.key_hash = sha256_key(raw_key)
66+
instance.key = sha256_key(raw_key)
7067
if commit:
7168
instance.save()
7269
return instance
@@ -78,7 +75,7 @@ class MAASProviderAdmin(admin.ModelAdmin):
7875
list_display = ('name', 'url', 'is_active', 'created_at', 'updated_at')
7976
list_filter = ('is_active',)
8077
search_fields = ('name',)
81-
readonly_fields = ('created_at', 'updated_at', 'key_hash')
78+
readonly_fields = ('created_at', 'updated_at', 'key')
8279

8380
change_form_template = 'admin/maas/change_form.html'
8481

@@ -89,7 +86,7 @@ class MAASApiKeyAdmin(admin.ModelAdmin):
8986
list_display = ('user', 'name', 'is_active', 'is_expired', 'created_at', 'expires_at', 'last_used_at')
9087
list_filter = ('is_active',)
9188
search_fields = ('user__username', 'name')
92-
readonly_fields = ('created_at', 'last_used_at', 'key_hash')
89+
readonly_fields = ('created_at', 'last_used_at', 'key')
9390

9491
change_form_template = 'admin/maas/change_form.html'
9592

maas/migrations/0001_initial.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
# Generated migration for maas module
2-
31
from django.conf import settings
42
from django.db import migrations, models
53
import django.db.models.deletion
@@ -20,8 +18,7 @@ class Migration(migrations.Migration):
2018
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
2119
('name', models.CharField(max_length=255, unique=True, verbose_name='name')),
2220
('url', models.URLField(max_length=500, verbose_name='base URL')),
23-
('key', models.CharField(max_length=255, verbose_name='API key (hashed)')),
24-
('key_hash', models.CharField(db_index=True, max_length=64, verbose_name='key hash (lookup)')),
21+
('key', models.CharField(db_index=True, max_length=64, verbose_name='key hash (lookup)')),
2522
('is_active', models.BooleanField(default=True, verbose_name='active')),
2623
('created_at', models.DateTimeField(auto_now_add=True, verbose_name='created')),
2724
('updated_at', models.DateTimeField(auto_now=True, verbose_name='updated')),
@@ -37,8 +34,7 @@ class Migration(migrations.Migration):
3734
fields=[
3835
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
3936
('name', models.CharField(max_length=255, verbose_name='name')),
40-
('key', models.CharField(max_length=255, verbose_name='API key (hashed)')),
41-
('key_hash', models.CharField(db_index=True, max_length=64, verbose_name='key hash (lookup)')),
37+
('key', models.CharField(db_index=True, max_length=64, verbose_name='key hash (lookup)')),
4238
('created_at', models.DateTimeField(auto_now_add=True, verbose_name='created')),
4339
('expires_at', models.DateTimeField(blank=True, null=True, verbose_name='expires')),
4440
('is_active', models.BooleanField(default=True, verbose_name='active')),

maas/models.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ def sha256_key(raw_key):
1111
class MAASProvider(models.Model):
1212
name = models.CharField(_('name'), max_length=255, unique=True)
1313
url = models.URLField(_('base URL'), max_length=500)
14-
key = models.CharField(_('API key (hashed)'), max_length=255)
15-
key_hash = models.CharField(_('key hash (lookup)'), max_length=64, db_index=True)
14+
key = models.CharField(_('key hash (lookup)'), max_length=64, db_index=True)
1615
is_active = models.BooleanField(_('active'), default=True)
1716
created_at = models.DateTimeField(_('created'), auto_now_add=True)
1817
updated_at = models.DateTimeField(_('updated'), auto_now=True)
@@ -29,8 +28,7 @@ def __str__(self):
2928
class MAASApiKey(models.Model):
3029
user = models.ForeignKey(User, on_delete=models.CASCADE, related_name='maas_keys', verbose_name=_('user'))
3130
name = models.CharField(_('name'), max_length=255)
32-
key = models.CharField(_('API key (hashed)'), max_length=255)
33-
key_hash = models.CharField(_('key hash (lookup)'), max_length=64, db_index=True)
31+
key = models.CharField(_('key hash (lookup)'), max_length=64, db_index=True)
3432
created_at = models.DateTimeField(_('created'), auto_now_add=True)
3533
expires_at = models.DateTimeField(_('expires'), null=True, blank=True)
3634
is_active = models.BooleanField(_('active'), default=True)

maas/tests.py

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
from datetime import datetime, timedelta
2-
from django.contrib.auth.hashers import make_password
32
from tests.tests import CustomTestCase
43
from maas.models import MAASProvider, MAASApiKey, MAASUsageRecord, sha256_key
54

@@ -9,17 +8,15 @@ def _create_provider(self, name='Test Provider', url='https://test.example.com',
98
return MAASProvider.objects.create(
109
name=name,
1110
url=url,
12-
key=make_password(key),
13-
key_hash=sha256_key(key),
11+
key=sha256_key(key),
1412
is_active=True,
1513
)
1614

1715
def _create_api_key(self, user, name='Test Key', key='test-user-key', expires_at=None):
1816
return MAASApiKey.objects.create(
1917
user=user,
2018
name=name,
21-
key=make_password(key),
22-
key_hash=sha256_key(key),
19+
key=sha256_key(key),
2320
expires_at=expires_at,
2421
is_active=True,
2522
)
@@ -290,6 +287,25 @@ def test_public_revoke_no_header(self):
290287
self.assertEqual(response.status_code, 200)
291288

292289

290+
def _create_provider(name='Test Provider', url='https://test.example.com', key='test-provider-key'):
291+
return MAASProvider.objects.create(
292+
name=name,
293+
url=url,
294+
key=sha256_key(key),
295+
is_active=True,
296+
)
297+
298+
299+
def _create_api_key(user, name='Test Key', key='test-user-key', expires_at=None):
300+
return MAASApiKey.objects.create(
301+
user=user,
302+
name=name,
303+
key=sha256_key(key),
304+
expires_at=expires_at,
305+
is_active=True,
306+
)
307+
308+
293309
class MaasAdminTestCase(CustomTestCase):
294310
def test_admin_provider_key_hashed_on_create(self):
295311
response = self.admin_client.post('/admin/maas/maasprovider/add/', {
@@ -301,7 +317,7 @@ def test_admin_provider_key_hashed_on_create(self):
301317
self.assertRedirects(response, '/admin/maas/maasprovider/')
302318
provider = MAASProvider.objects.get(name='Test Provider')
303319
self.assertNotEqual(provider.key, 'my-secret-key')
304-
self.assertEqual(provider.key_hash, sha256_key('my-secret-key'))
320+
self.assertEqual(provider.key, sha256_key('my-secret-key'))
305321

306322
def test_admin_provider_key_not_plaintext(self):
307323
self.admin_client.post('/admin/maas/maasprovider/add/', {
@@ -311,11 +327,11 @@ def test_admin_provider_key_not_plaintext(self):
311327
'is_active': True,
312328
})
313329
provider = MAASProvider.objects.get(name='Secret Provider')
314-
self.assertTrue(provider.key.startswith('pbkdf2_') or provider.key.startswith('argon2') or provider.key.startswith('bcrypt'))
330+
self.assertEqual(provider.key, sha256_key('super-secret-value'))
315331

316332
def test_admin_provider_edit_blank_key_preserves_existing(self):
317-
provider = self._create_provider(key='original-key')
318-
original_key_hash = provider.key_hash
333+
provider = _create_provider(key='original-key')
334+
original_key = provider.key
319335
response = self.admin_client.post(f'/admin/maas/maasprovider/{provider.id}/change/', {
320336
'name': 'Updated Provider',
321337
'url': 'https://updated.example.com',
@@ -325,18 +341,18 @@ def test_admin_provider_edit_blank_key_preserves_existing(self):
325341
self.assertRedirects(response, '/admin/maas/maasprovider/')
326342
provider.refresh_from_db()
327343
self.assertEqual(provider.name, 'Updated Provider')
328-
self.assertEqual(provider.key_hash, original_key_hash)
344+
self.assertEqual(provider.key, original_key)
329345

330346
def test_admin_provider_edit_new_key_overwrites(self):
331-
provider = self._create_provider(key='original-key')
347+
provider = _create_provider(key='original-key')
332348
self.admin_client.post(f'/admin/maas/maasprovider/{provider.id}/change/', {
333349
'name': 'Updated Provider',
334350
'url': 'https://updated.example.com',
335351
'clear_text_key': 'new-secret-key',
336352
'is_active': True,
337353
})
338354
provider.refresh_from_db()
339-
self.assertEqual(provider.key_hash, sha256_key('new-secret-key'))
355+
self.assertEqual(provider.key, sha256_key('new-secret-key'))
340356

341357
def test_admin_api_key_hashed_on_create(self):
342358
response = self.admin_client.post('/admin/maas/maasapikey/add/', {
@@ -352,7 +368,7 @@ def test_admin_api_key_hashed_on_create(self):
352368
self.assertRedirects(response, '/admin/maas/maasapikey/')
353369
api_key = MAASApiKey.objects.get(name='Admin Created Key')
354370
self.assertNotEqual(api_key.key, 'admin-api-secret')
355-
self.assertEqual(api_key.key_hash, sha256_key('admin-api-secret'))
371+
self.assertEqual(api_key.key, sha256_key('admin-api-secret'))
356372

357373
def test_admin_api_key_not_plaintext(self):
358374
self.admin_client.post('/admin/maas/maasapikey/add/', {
@@ -366,11 +382,11 @@ def test_admin_api_key_not_plaintext(self):
366382
'last_used_at_1': '',
367383
})
368384
api_key = MAASApiKey.objects.get(name='Secret API Key')
369-
self.assertTrue(api_key.key.startswith('pbkdf2_') or api_key.key.startswith('argon2') or api_key.key.startswith('bcrypt'))
385+
self.assertEqual(api_key.key, sha256_key('secret-api-value'))
370386

371387
def test_admin_api_key_edit_blank_preserves_existing(self):
372-
api_key = self._create_api_key(self.testuser, key='original-api-key')
373-
original_hash = api_key.key_hash
388+
api_key = _create_api_key(self.testuser, key='original-api-key')
389+
original_hash = api_key.key
374390
self.admin_client.post(f'/admin/maas/maasapikey/{api_key.id}/change/', {
375391
'user': self.testuser.pk,
376392
'name': 'Updated API Key',
@@ -383,4 +399,4 @@ def test_admin_api_key_edit_blank_preserves_existing(self):
383399
})
384400
api_key.refresh_from_db()
385401
self.assertEqual(api_key.name, 'Updated API Key')
386-
self.assertEqual(api_key.key_hash, original_hash)
402+
self.assertEqual(api_key.key, original_hash)

maas/views.py

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from django.http import HttpResponseForbidden, HttpResponseNotFound, JsonResponse
1010
from django.contrib.auth.decorators import login_required
1111
from django.contrib.admin.views.decorators import staff_member_required
12-
from django.contrib.auth.hashers import make_password, check_password
1312
from django.utils import timezone
1413
from django.utils.translation import gettext as _
1514
from django.views.decorators.http import require_POST
@@ -60,13 +59,10 @@ def authenticate(self, request):
6059

6160
key_hash = sha256_key(raw_key)
6261
try:
63-
candidate = MAASApiKey.objects.get(is_active=True, key_hash=key_hash)
62+
candidate = MAASApiKey.objects.get(is_active=True, key=key_hash)
6463
except MAASApiKey.DoesNotExist:
6564
raise AuthenticationFailed(_('Invalid API key'))
6665

67-
if not check_password(raw_key, candidate.key):
68-
raise AuthenticationFailed(_('Invalid API key'))
69-
7066
if candidate.is_expired:
7167
raise AuthenticationFailed(_('API key has expired'))
7268

@@ -82,12 +78,9 @@ def lookup_provider(provider_key):
8278
"""Find an active provider by its plaintext API key."""
8379
key_hash = sha256_key(provider_key)
8480
try:
85-
candidate = MAASProvider.objects.get(is_active=True, key_hash=key_hash)
81+
return MAASProvider.objects.get(is_active=True, key=key_hash)
8682
except MAASProvider.DoesNotExist:
8783
return None
88-
if not check_password(provider_key, candidate.key):
89-
return None
90-
return candidate
9184

9285

9386
# --- API Endpoints ---
@@ -198,10 +191,9 @@ def post(self, request):
198191
if raw_key:
199192
key_hash = sha256_key(raw_key)
200193
try:
201-
candidate = MAASApiKey.objects.get(is_active=True, key_hash=key_hash)
202-
if check_password(raw_key, candidate.key):
203-
candidate.is_active = False
204-
candidate.save(update_fields=['is_active'])
194+
candidate = MAASApiKey.objects.get(is_active=True, key=key_hash)
195+
candidate.is_active = False
196+
candidate.save(update_fields=['is_active'])
205197
except MAASApiKey.DoesNotExist:
206198
pass
207199
return Response({}, status=status.HTTP_200_OK)
@@ -304,7 +296,6 @@ def key_new(request, username):
304296
})
305297

306298
raw_key = secrets.token_urlsafe(48)
307-
hashed_key = make_password(raw_key)
308299
key_hash = sha256_key(raw_key)
309300

310301
expires_at = None
@@ -315,8 +306,7 @@ def key_new(request, username):
315306
api_key = MAASApiKey.objects.create(
316307
user=target_user,
317308
name=name,
318-
key=hashed_key,
319-
key_hash=key_hash,
309+
key=key_hash,
320310
expires_at=expires_at,
321311
)
322312

@@ -370,12 +360,10 @@ def provider_new(request):
370360
'error': _('All fields are required'),
371361
})
372362

373-
hashed_key = make_password(raw_key)
374363
provider = MAASProvider.objects.create(
375364
name=name,
376365
url=url,
377-
key=hashed_key,
378-
key_hash=sha256_key(raw_key),
366+
key=sha256_key(raw_key),
379367
)
380368

381369
return render(request, 'maas/provider_new.html', {

0 commit comments

Comments
 (0)