Skip to content

Commit

Permalink
Merge pull request #685 from drnlm/feature/redo_user_profile_with_kv
Browse files Browse the repository at this point in the history
Feature/redo user profile with kv
  • Loading branch information
drnlm authored Oct 15, 2023
2 parents 241667c + bf2b82a commit 8425337
Show file tree
Hide file tree
Showing 18 changed files with 542 additions and 82 deletions.
23 changes: 23 additions & 0 deletions docs/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,33 @@ in the ``wafer.settings`` module, so you can import this in your app's
Wafer's settings
================

``CODE_HOSTING_ENTRIES``
A dictionary of code hosting sites for the user profile.
Entries should be of the form: **keyname**: "Description" .

Each entry in the dictionary will be added as a form field
on the profile page - entries are assumed to be for urls to
the approriate site.

Entries in this dictionary will be grouped together on
the edit profile form and on the profile display.

``PAGE_DIR``
The directory that the ``load_pages`` management command will load
pages from.
Should be an absolute path with a trailing ``/``.

``SOCIAL_MEDIA_ENTRIES``
A dictionary of social sites for the user profile.
Entries should be of the form: **keyname**: "Description" .

Each entry in the dictionary will be added as a form field
on the profile page - entries are assumed to be for urls to
the approriate site.

Entries in this dictionary will be grouped together on
the edit profile form and on the profile display.

``WAFER_CACHE``
The name of the Django cache backend that wafer can use.
Defaults to ``'wafer_cache'``.
Expand Down Expand Up @@ -165,6 +187,7 @@ Wafer's settings
email address of someone who will review the talk's video, once it
is ready to publish.


Third party settings
====================

Expand Down
11 changes: 1 addition & 10 deletions wafer/kv/tests/test_kv_api.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,11 @@
"""Tests for wafer.kv api views."""

from django.contrib.auth.models import Group
from django.test import Client, TestCase

from rest_framework.test import APIClient

from wafer.kv.models import KeyValue
from wafer.tests.utils import create_user


def get_group(group):
return Group.objects.get(name=group)


def create_group(group):
return Group.objects.create(name=group)
from wafer.tests.utils import create_group, create_user, get_group


def create_kv_pair(name, value, group):
Expand Down
1 change: 1 addition & 0 deletions wafer/management/commands/wafer_add_default_groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Command(BaseCommand):
('talks', 'view_all_talks'),
),
'Registration': (),
'Online Profiles': (),
}

def add_wafer_groups(self):
Expand Down
69 changes: 51 additions & 18 deletions wafer/registration/sso.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,28 @@ def _configure_user(user, name, email, profile_fields):
profile.save()


def _check_count(kv_search):
"""Return the number of matches found.
We need to check for two possible sources of duplicates which
will make us unable to uniquely identify the user:
1) Multiple different KeyValues created with the same value
2) Multiple user profiles assigned to the same KeyValue
Both errors should require something strange to have happened - importing data
into an existing database, weird API interactions, etc -, but we
don't want to allow incorrect access to an existing account.
"""
if kv_search.count() > 1:
# Multiple KeyValues
return kv_search.count()
if kv_search.count() == 1:
# Could have multiple user profiles
# Will be 1 in the case of a unique match, which is what we want
return kv_search[0].userprofile_set.count()
# No matches
return 0


def github_sso(code):
r = requests.post(
'https://github.com/login/oauth/access_token', data={
Expand Down Expand Up @@ -119,22 +141,30 @@ def github_sso(code):
log.warning('Error extracting github email address: %s', e)
raise SSOError('Failed to obtain email address from GitHub')

profile_fields = {
'github_username': login,
}
# TODO: Extend this to also set the github profile url KV
profile_fields = {}
if 'blog' in gh:
profile_fields['blog'] = gh['blog']

try:
user = get_user_model().objects.get(userprofile__github_username=login)
except MultipleObjectsReturned:
log.warning('Multiple accounts have GitHub username %s', login)
raise SSOError('Multiple accounts have GitHub username %s' % login)
except ObjectDoesNotExist:
user = None
group = Group.objects.get_by_natural_key('Registration')
user = None

kv_search = KeyValue.objects.filter(
group=group, key='github_sso_account_id', value=login,
userprofile__isnull=False)
count = _check_count(kv_search)
if count > 1:
message = 'Multiple accounts have the same GitHub SSO id: %s'
log.warning(message, login)
raise SSOError(message % login)

if count:
user = kv_search[0].userprofile_set.first().user

user = sso(user=user, desired_username=login, name=name, email=email,
profile_fields=profile_fields)
user.userprofile.kv.get_or_create(group=group, key='github_sso_account_id',
defaults={'value': login})
return user


Expand Down Expand Up @@ -172,15 +202,18 @@ def gitlab_sso(code, redirect_uri):

group = Group.objects.get_by_natural_key('Registration')
user = None
for kv in KeyValue.objects.filter(
kv_search = KeyValue.objects.filter(
group=group, key='gitlab_sso_account_id', value=gl['id'],
userprofile__isnull=False):
if kv.userprofile_set.count() > 1:
message = 'Multiple accounts have GitLab SSOed for User ID %s'
log.warning(message, gl['id'])
raise SSOError(message % gl['id'])
user = kv.userprofile_set.first().user
break
userprofile__isnull=False)

count = _check_count(kv_search)
if count > 1:
message = 'Multiple accounts have GitLab SSOed for User ID %s'
log.warning(message, gl['id'])
raise SSOError(message % gl['id'])

if count:
user = kv_search[0].userprofile_set.first().user

user = sso(user=user, desired_username=username, name=name, email=email)
user.userprofile.kv.get_or_create(group=group, key='gitlab_sso_account_id',
Expand Down
Empty file.
211 changes: 211 additions & 0 deletions wafer/registration/tests/test_sso.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
"""Basic tests for the SSO behaviour
These tests mock out the actual SSO interactions and only
test that accounts are created correctly and some of the
error paths"""


import mock

from django.test import TestCase
from django.contrib.auth import get_user_model

from wafer.tests.utils import create_group, create_user

from wafer.registration.sso import github_sso, gitlab_sso, SSOError
from wafer.kv.models import KeyValue


# Helper classes for mocking request calls
class SSOPost:

def __init__(self, status_code, access_token):
self.status_code = status_code
self.access_token = access_token

def json(self):
return {'access_token': self.access_token}

class SSOGet:

def __init__(self, status_code, username, name, email):
self.status_code = status_code
self.username = username
self.name = name
self.email = email

def json(self):
# We duplicate username, id and login so we can use this for
# both gitlab and github
json = {
'login': self.username,
'username': self.username,
'id': self.username,
'name': self.name,
}
# We allow a null email address to remove the field for
# testing error cases
if self.email:
json['email'] = self.email
return json


class RegistrationSSOTests(TestCase):

def setUp(self):
self.reg_group = create_group('Registration')

@mock.patch('requests.post', new=lambda x, **kw: SSOPost(status_code=200, access_token='aaaa'))
@mock.patch('requests.get', new=lambda x, **kw: SSOGet(status_code=200, username='joe',
name='Joe Soap', email='[email protected]'))
def test_github_success(self):
"""Test that a successful github auth call creates the user and
sets the right values"""
# Check that no user currently exists
self.assertEqual(get_user_model().objects.filter(username='joe').count(), 0)
with self.settings(WAFER_GITHUB_CLIENT_ID='testy', WAFER_GITHUB_CLIENT_SECRET='abc'):
user = github_sso('abcde')
self.assertEqual(user.username, 'joe')
self.assertEqual(user.first_name, 'Joe')
self.assertEqual(user.last_name, 'Soap')
self.assertEqual(user.email, '[email protected]')

kv = KeyValue.objects.filter(group=self.reg_group, key="github_sso_account_id").get()
self.assertEqual(kv.value, user.username)

# Check that calling this again gives the same user back
with self.settings(WAFER_GITHUB_CLIENT_ID='testy', WAFER_GITHUB_CLIENT_SECRET='abc'):
user2 = github_sso('abcde')

self.assertEqual(user, user2)
# Check we only created 1 user
self.assertEqual(get_user_model().objects.filter(username='joe').count(), 1)

def test_github_fail(self):
"""Test that a failed github auth call doesn't create a user"""
# Test incorrect code response
cur_users = get_user_model().objects.count()
# We use the nested syntax to have something legible that supports Python < 3.10
with self.assertRaises(SSOError) as cm:
with self.settings(WAFER_GITHUB_CLIENT_ID='testy', WAFER_GITHUB_CLIENT_SECRET='abc'):
with mock.patch('requests.post', new=lambda x, **kw: SSOPost(status_code=333, access_token='aaaa')):
user = github_sso('abcde')
self.assertTrue('Invalid code' in str(cm.exception))
self.assertEqual(cur_users, get_user_model().objects.count())

# Test auth token response failure
with self.assertRaises(SSOError) as cm:
with self.settings(WAFER_GITHUB_CLIENT_ID='testy', WAFER_GITHUB_CLIENT_SECRET='abc'):
with mock.patch('requests.post', new=lambda x, **kw: SSOPost(status_code=200,
access_token='aaaa')):
with mock.patch('requests.get', new=lambda x, **kw: SSOGet(status_code=333,
username='github_dup',
name='Joe Soap',
email='[email protected]')):
user = github_sso('abcde')
self.assertTrue('Failed response from GitHub' in str(cm.exception))
self.assertEqual(cur_users, get_user_model().objects.count())

@mock.patch('requests.post', new=lambda x, **kw: SSOPost(status_code=200, access_token='aaaa'))
@mock.patch('requests.get', new=lambda x, **kw: SSOGet(status_code=200, username='github_dup',
name='Joe Soap', email='[email protected]'))
def test_github_duplicate(self):
"""Test that we get the expected error message if someone tries
to reuse the github username for sso"""
# This tests multiple KeyValues with the same data
dup1 = create_user('github_dup1')
dup1.userprofile.kv.get_or_create(group=self.reg_group,
key='github_sso_account_id',
defaults={'value': 'github_dup'})
dup2 = create_user('github_dup2')
dup2.userprofile.kv.get_or_create(group=self.reg_group,
key='github_sso_account_id',
defaults={'value': 'github_dup'})
with self.assertRaises(SSOError) as cm:
with self.settings(WAFER_GITHUB_CLIENT_ID='testy', WAFER_GITHUB_CLIENT_SECRET='abcd'):
user = github_sso('abcdef')
exception = cm.exception
self.assertTrue('Multiple accounts have the same GitHub' in str(exception))

@mock.patch('requests.post', new=lambda x, **kw: SSOPost(status_code=200, access_token='aaaa'))
@mock.patch('requests.get', new=lambda x, **kw: SSOGet(status_code=200, username='joe2',
name='Joe2 Soap', email='[email protected]'))
def test_gitlab_success(self):
"""Test that a successful gitlab auth call creates the user and
sets the right values"""
# Check that no user currently exists
self.assertEqual(get_user_model().objects.filter(username='joe2').count(), 0)
with self.settings(WAFER_GITLAB_CLIENT_ID='testy', WAFER_GITLAB_CLIENT_SECRET='abc'):
user = gitlab_sso('abcde', 'http://localhost/')
self.assertEqual(user.username, 'joe2')
self.assertEqual(user.first_name, 'Joe2')
self.assertEqual(user.last_name, 'Soap')
self.assertEqual(user.email, '[email protected]')

kv = KeyValue.objects.filter(group=self.reg_group, key="gitlab_sso_account_id").get()
self.assertEqual(kv.value, user.username)

# Check that calling this again gives the same user back
with self.settings(WAFER_GITLAB_CLIENT_ID='testy', WAFER_GITLAB_CLIENT_SECRET='abc'):
user2 = gitlab_sso('abcde', 'http://localhost/')

self.assertEqual(user, user2)
# Check we only created 1 user
self.assertEqual(get_user_model().objects.filter(username='joe2').count(), 1)

def test_gitlab_fail(self):
"""Test that a failed gitlab auth call doesn't create a user"""
# Test incorrect code response
cur_users = get_user_model().objects.count()
with self.assertRaises(SSOError) as cm:
with self.settings(WAFER_GITLAB_CLIENT_ID='testy', WAFER_GITLAB_CLIENT_SECRET='abc'):
with mock.patch('requests.post', new=lambda x, **kw: SSOPost(status_code=333,
access_token='aaaa')):
user = gitlab_sso('abcde', 'http://localhost/')
self.assertTrue('Invalid code' in str(cm.exception))
self.assertEqual(cur_users, get_user_model().objects.count())

# Test auth token response failure
with self.assertRaises(SSOError) as cm:
with self.settings(WAFER_GITLAB_CLIENT_ID='testy', WAFER_GITLAB_CLIENT_SECRET='abc'):
with mock.patch('requests.post', new=lambda x, **kw: SSOPost(status_code=200,
access_token='aaaa')):
with mock.patch('requests.get', new=lambda x, **kw: SSOGet(status_code=333,
username='github_dup',
name='Joe Soap',
email='[email protected]')):
user = gitlab_sso('abcde', 'http://localhost/')
self.assertTrue('Failed response from GitLab' in str(cm.exception))
self.assertEqual(cur_users, get_user_model().objects.count())
# Test profile error
with self.assertRaises(SSOError) as cm:
with self.settings(WAFER_GITLAB_CLIENT_ID='testy', WAFER_GITLAB_CLIENT_SECRET='abc'):
with mock.patch('requests.post', new=lambda x, **kw: SSOPost(status_code=200, access_token='aaaa')):
with mock.patch('requests.get', new=lambda x, **kw: SSOGet(status_code=200,
username='github_dup',
name='Joe Soap',
email=None)):
user = gitlab_sso('abcde', 'http://localhost/')
self.assertEqual('GitLab profile missing required content', str(cm.exception))
self.assertEqual(cur_users, get_user_model().objects.count())

@mock.patch('requests.post', new=lambda x, **kw: SSOPost(status_code=200, access_token='aaaa'))
@mock.patch('requests.get', new=lambda x, **kw: SSOGet(status_code=200, username='dup2',
name='Joe2 Soap', email='[email protected]'))
def test_gitlab_duplicate(self):
"""Test that we get the expected error message if someone tries
to reuse the gitlab username for sso"""
# Thi tests mulitple users sharing a single KeyValue entry
dup1 = create_user('gitlab_dup1')
dup1.userprofile.kv.get_or_create(group=self.reg_group,
key='gitlab_sso_account_id',
defaults={'value': 'dup2'})
dup2 = create_user('gitlab_dup2')
kv = KeyValue.objects.filter(group=self.reg_group, key='gitlab_sso_account_id', value='dup2').get()
kv.userprofile_set.add(dup2.userprofile)
with self.assertRaises(SSOError) as cm:
with self.settings(WAFER_GITLAB_CLIENT_ID='testy', WAFER_GITLAB_CLIENT_SECRET='abcd'):
gitlab_sso('abcdef', 'http://localhost/')
exception = cm.exception
self.assertTrue('Multiple accounts have GitLab' in str(exception))

Loading

0 comments on commit 8425337

Please sign in to comment.