Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Credential handling for password-less learner certificate generation #8239

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions kolibri/core/auth/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,27 @@
from kolibri.core.auth.models import FacilityUser


FACILITY_CREDENTIAL_KEY = "facility"


class FacilityUserBackend(object):
"""
A class that implements authentication for FacilityUsers.
"""

def authenticate(self, username=None, password=None, facility=None):
def authenticate(self, request, username=None, password=None, **kwargs):
"""
Authenticates the user if the credentials correspond to a FacilityUser for the specified Facility.

:param request: The request is a required positional argument in newer versions of Django
:param username: a string
:param password: a string
:param facility: a Facility
:param kwargs: a dict of additional credentials (see `keyword`s)
:keyword facility: a Facility object or facility ID
:return: A FacilityUser instance if successful, or None if authentication failed.
"""
users = FacilityUser.objects.filter(username__iexact=username)
facility = kwargs.get(FACILITY_CREDENTIAL_KEY, None)
if facility:
users = users.filter(facility=facility)
for user in users:
Expand Down
2 changes: 2 additions & 0 deletions kolibri/core/auth/management/commands/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ def handle_async(self, *args, **options): # noqa C901
dataset_id,
network_connection,
user_id=user_id,
facility_id=facility_id,
noninteractive=noninteractive,
)

Expand Down Expand Up @@ -212,6 +213,7 @@ def handle_async(self, *args, **options): # noqa C901
password,
dataset_id,
network_connection,
facility_id=facility_id,
noninteractive=noninteractive,
)

Expand Down
18 changes: 16 additions & 2 deletions kolibri/core/auth/management/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from morango.models import ScopeDefinition
from six.moves.urllib.parse import urljoin

from kolibri.core.auth.backends import FACILITY_CREDENTIAL_KEY
from kolibri.core.auth.constants.morango_sync import ScopeDefinitions
from kolibri.core.auth.models import Facility
from kolibri.core.auth.models import FacilityUser
Expand Down Expand Up @@ -161,7 +162,13 @@ def get_baseurl(baseurl):


def get_client_and_server_certs(
username, password, dataset_id, nc, user_id=None, noninteractive=False
username,
password,
dataset_id,
nc,
user_id=None,
facility_id=None,
noninteractive=False,
):

# get any full-facility certificates we have for the facility
Expand Down Expand Up @@ -227,11 +234,18 @@ def get_client_and_server_certs(
username = input("Please enter username: ")
password = getpass.getpass("Please enter password: ")

userargs = username
if user_id:
# add facility so `FacilityUserBackend` can validate
userargs = {
FacilityUser.USERNAME_FIELD: username,
FACILITY_CREDENTIAL_KEY: facility_id,
Copy link
Member

Choose a reason for hiding this comment

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

Ahah, yes that answers my question!

}
client_cert = nc.certificate_signing_request(
server_cert,
client_scope,
csr_scope_params,
userargs=username,
userargs=userargs,
password=password,
)
else:
Expand Down
2 changes: 1 addition & 1 deletion kolibri/core/auth/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def ensure_compatibility(self, *args, **kwargs):
if self.learner_can_login_with_no_password and self.learner_can_edit_password:
raise IncompatibleDeviceSettingError(
"Device Settings [learner_can_login_with_no_password={}] & [learner_can_edit_password={}] "
"values incompatible togeather.".format(
"values incompatible together.".format(
self.learner_can_login_with_no_password,
self.learner_can_edit_password,
)
Expand Down
33 changes: 27 additions & 6 deletions kolibri/core/auth/test/test_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from __future__ import print_function
from __future__ import unicode_literals

import mock
from django.test import TestCase

from ..backends import FacilityUserBackend
Expand All @@ -16,30 +17,46 @@ def setUpTestData(cls):
cls.user = FacilityUser(username="Mike", facility=cls.facility)
cls.user.set_password("foo")
cls.user.save()
cls.request = mock.Mock()

def test_facility_user_authenticated(self):
self.assertEqual(
self.user,
FacilityUserBackend().authenticate(
username="Mike", password="foo", facility=self.facility
self.request, username="Mike", password="foo", facility=self.facility
),
)

def test_facility_user_authenticated__facility_id(self):
self.assertEqual(
self.user,
FacilityUserBackend().authenticate(
self.request, username="Mike", password="foo", facility=self.facility.pk
),
)

def test_facility_user_authentication_does_not_require_facility(self):
self.assertEqual(
self.user,
FacilityUserBackend().authenticate(username="Mike", password="foo"),
FacilityUserBackend().authenticate(
self.request, username="Mike", password="foo"
),
)

def test_device_owner_not_authenticated(self):
self.assertIsNone(
FacilityUserBackend().authenticate(username="Chuck", password="foobar")
FacilityUserBackend().authenticate(
self.request, username="Chuck", password="foobar"
)
)

def test_incorrect_password_does_not_authenticate(self):
self.assertIsNone(
FacilityUserBackend().authenticate(
username="Mike", password="blahblah", facility=self.facility
self.request,
username="Mike",
password="blahblah",
facility=self.facility,
)
)

Expand All @@ -52,7 +69,11 @@ def test_nonexistent_user_returns_none(self):
)

def test_authenticate_nonexistent_user_returns_none(self):
self.assertIsNone(FacilityUserBackend().authenticate("foo", "bar"))
self.assertIsNone(
FacilityUserBackend().authenticate(self.request, "foo", "bar")
)

def test_authenticate_with_wrong_password_returns_none(self):
self.assertIsNone(FacilityUserBackend().authenticate("Mike", "goo"))
self.assertIsNone(
FacilityUserBackend().authenticate(self.request, "Mike", "goo")
)
49 changes: 49 additions & 0 deletions kolibri/core/auth/test/test_morango_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
import unittest
import uuid

from django.core.management import call_command
from django.test import TestCase
from django.utils import timezone
from morango.models import InstanceIDModel
from morango.models import ScopeDefinition
from morango.models import Store
from morango.models import syncable_models
from morango.sync.controller import MorangoProfileController
Expand All @@ -22,6 +24,7 @@
from ..models import Role
from .helpers import DUMMY_PASSWORD
from .sync_utils import multiple_kolibri_servers
from kolibri.core.auth.management.utils import get_client_and_server_certs
from kolibri.core.exams.models import Exam
from kolibri.core.exams.models import ExamAssignment
from kolibri.core.lessons.models import Lesson
Expand Down Expand Up @@ -66,6 +69,52 @@ def test_deserializing_field(self):
self.fail(e.message)


@unittest.skipIf(
not os.environ.get("INTEGRATION_TEST"),
"This test will only be run during integration testing.",
)
class CertificateAuthenticationTestCase(TestCase):
@multiple_kolibri_servers(1)
def test_learner_passwordless_authentication(self, servers):
# START: setup server
server = servers[0]
server.manage("loaddata", "scopedefinitions")
server.manage("loaddata", "content_test")
server.manage("generateuserdata", "--no-onboarding", "--num-content-items", "1")

facility = Facility.objects.using(server.db_alias).first()
facility.dataset.learner_can_login_with_no_password = True
facility.dataset.learner_can_edit_password = False
facility.dataset.save(using=server.db_alias)
learner = FacilityUser(
username=uuid.uuid4().hex[:30], password=DUMMY_PASSWORD, facility=facility
)
learner.save(using=server.db_alias)
# END: setup server

# START: local setup
if not ScopeDefinition.objects.filter():
call_command("loaddata", "scopedefinitions")

controller = MorangoProfileController(PROFILE_FACILITY_DATA)
network_connection = controller.create_network_connection(server.baseurl)

# if it's not working, this will throw:
# requests.exceptions.HTTPError: 401 Client Error: Unauthorized for url
client_cert, server_cert, username = get_client_and_server_certs(
learner.username,
"NOT_THE_DUMMY_PASSWORD",
facility.dataset_id,
network_connection,
user_id=learner.pk,
facility_id=facility.id,
noninteractive=True,
)
self.assertIsNotNone(client_cert)
self.assertIsNotNone(server_cert)
self.assertIsNotNone(username)


@unittest.skipIf(
not os.environ.get("INTEGRATION_TEST"),
"This test will only be run during integration testing.",
Expand Down
4 changes: 2 additions & 2 deletions kolibri/core/discovery/test/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_creating_good_address(self):
format="json",
)
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
self.assertEqual(response.data["base_url"], "https://kolibrihappyurl.qqq/")
self.assertEqual(response.data["base_url"], "https://kolibrihappyurl.qqq")

def test_creating_good_address_with_one_url_timing_out(self):
self.login(self.superuser)
Expand All @@ -63,7 +63,7 @@ def test_creating_good_address_with_one_url_timing_out(self):
)
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
self.assertEqual(
response.data["base_url"], "http://timeoutonport80url.qqq:8080/"
response.data["base_url"], "http://timeoutonport80url.qqq:8080"
)

def test_creating_bad_address(self):
Expand Down
8 changes: 4 additions & 4 deletions kolibri/core/discovery/test/test_network_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def test_valid_domain_name_with_invalid_nonnumeric_port(self):
class TestNetworkClientConnections(TestCase):
def test_successful_connection_to_kolibri_address(self):
nc = NetworkClient(address="kolibrihappyurl.qqq")
self.assertEqual(nc.base_url, "https://kolibrihappyurl.qqq/")
self.assertEqual(nc.base_url, "https://kolibrihappyurl.qqq")

def test_unsuccessful_connection_to_unavailable_address(self):
with self.assertRaises(errors.NetworkLocationNotFound):
Expand All @@ -156,11 +156,11 @@ def test_unsuccessful_connection_to_nonkolibri_address(self):

def test_successful_connection_to_address_with_port80_timeout(self):
nc = NetworkClient(address="timeoutonport80url.qqq")
self.assertEqual(nc.base_url, "http://timeoutonport80url.qqq:8080/")
self.assertEqual(nc.base_url, "http://timeoutonport80url.qqq:8080")

def test_successful_connection_to_kolibri_base_url(self):
nc = NetworkClient(base_url="https://kolibrihappyurl.qqq/")
self.assertEqual(nc.base_url, "https://kolibrihappyurl.qqq/")
self.assertEqual(nc.base_url, "https://kolibrihappyurl.qqq")

def test_unsuccessful_connection_to_unavailable_base_url(self):
with self.assertRaises(errors.NetworkLocationNotFound):
Expand All @@ -172,4 +172,4 @@ def test_unsuccessful_connection_to_nonkolibri_base_url(self):

def test_unsuccessful_connection_to_base_url_with_timeout(self):
with self.assertRaises(errors.NetworkLocationNotFound):
NetworkClient(base_url="http://timeoutonport80url.qqq/")
NetworkClient(base_url="http://timeoutonport80url.qqq")
10 changes: 6 additions & 4 deletions kolibri/core/discovery/utils/network/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ def _attempt_connections(self, urls):
allow_redirects=True,
params={"v": DEVICE_INFO_VERSION},
)
# check that we successfully connected, and if we were redirected that it's still the right endpoint
response_path = urlparse(response.url).path
if response.status_code == 200 and response_path.rstrip("/").endswith(
# check that we successfully connected, and if we were redirected that it's still
# the right endpoint
parsed_url = urlparse(response.url)
if response.status_code == 200 and parsed_url.path.rstrip("/").endswith(
"/api/public/info"
):
info = response.json()
Expand All @@ -65,7 +66,8 @@ def _attempt_connections(self, urls):
"Server is not running Kolibri or Studio"
)
logger.info("Success! We connected to: {}".format(response.url))
return response.url.rstrip("/").replace("api/public/info", "")

return "{}://{}".format(parsed_url.scheme, parsed_url.netloc)
except (requests.RequestException) as e:
logger.info("Unable to connect: {}".format(e))
except ValueError:
Expand Down
23 changes: 17 additions & 6 deletions kolibri/core/tasks/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,13 @@ def validate_peer_sync_job(request):


def prepare_peer_sync_job(baseurl, facility_id, username, password, **kwargs):
job_data = prepare_sync_job(facility_id, baseurl=baseurl, **kwargs) # ToDo
"""
Initializes and validates connection to peer with username and password for the sync command. If
already initialized, the username and password do not need to be supplied
"""
# get the `user` for the sync command if present for provisioning
user_id = kwargs.get("user", None)
job_data = prepare_sync_job(facility_id, baseurl=baseurl, **kwargs)

# call this in case user directly syncs without migrating database
if not ScopeDefinition.objects.filter():
Expand All @@ -1237,7 +1243,13 @@ def prepare_peer_sync_job(baseurl, facility_id, username, password, **kwargs):

# username and password are not required for this to succeed unless there is no cert
get_client_and_server_certs(
username, password, dataset_id, network_connection, noninteractive=True
username,
password,
dataset_id,
network_connection,
user_id=user_id,
facility_id=facility_id,
noninteractive=True,
)
except (CommandError, HTTPError) as e:
if not username and not password:
Expand All @@ -1251,11 +1263,10 @@ def prepare_peer_sync_job(baseurl, facility_id, username, password, **kwargs):
def prepare_soud_sync_job(baseurl, facility_id, user_id, **kwargs):
"""
A SoUD sync requires that the device is already "registered" with the server, so there
shouldn't be a need for username/password and the verification of those
shouldn't be a need for username/password and the verification of those. This eliminates the
validation to keep overhead low for automated single-user syncing. To initialize with a peer
for a SoUD, use `prepare_peer_sync_job` with `user` keyword argument
"""
if not ScopeDefinition.objects.filter():
call_command("loaddata", "scopedefinitions")

kwargs.update(user=user_id)
return prepare_sync_job(facility_id, baseurl=baseurl, **kwargs)

Expand Down
8 changes: 7 additions & 1 deletion kolibri/core/tasks/test/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,13 @@ def test_validate_peer_sync_job(
)

get_client_and_server_certs.assert_called_with(
"tester", "mypassword", dataset_id, network_connection, noninteractive=True
"tester",
"mypassword",
dataset_id,
network_connection,
user_id=None,
facility_id=123,
noninteractive=True,
)

def test_validate_peer_sync_job__no_baseurl(self):
Expand Down