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 1 commit
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
8 changes: 3 additions & 5 deletions kolibri/core/auth/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
The appropriate classes should be listed in the AUTHENTICATION_BACKENDS. Note that authentication
backends are checked in the order they're listed.
"""
from kolibri.core.auth.models import Facility
from kolibri.core.auth.models import FacilityUser


FACILITY_CREDENTIAL_NAME = "facility"
FACILITY_CREDENTIAL_KEY = "facility"


class FacilityUserBackend(object):
Expand All @@ -26,11 +25,10 @@ def authenticate(self, request, username=None, password=None, **kwargs):
:keyword facility: a Facility object or facility ID
:return: A FacilityUser instance if successful, or None if authentication failed.
"""
facility = kwargs.get(FACILITY_CREDENTIAL_NAME, None)
users = FacilityUser.objects.filter(username__iexact=username)
facility = kwargs.get(FACILITY_CREDENTIAL_KEY, None)
if facility:
facility_id = facility.pk if isinstance(facility, Facility) else facility
users = users.filter(facility_id=facility_id)
users = users.filter(facility=facility)
for user in users:
if user.check_password(password):
return user
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
22 changes: 15 additions & 7 deletions kolibri/core/auth/management/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from morango.models import ScopeDefinition
from six.moves.urllib.parse import urljoin

from kolibri.core.auth.backends import FACILITY_CREDENTIAL_NAME
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 @@ -162,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 @@ -228,11 +234,13 @@ def get_client_and_server_certs(
username = input("Please enter username: ")
password = getpass.getpass("Please enter password: ")

# add facility so `FacilityUserBackend` can validate
userargs = {
FacilityUser.USERNAME_FIELD: username,
FACILITY_CREDENTIAL_NAME: dataset_id,
}
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,
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: 1 addition & 3 deletions kolibri/core/discovery/utils/network/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import requests
from six.moves.urllib.parse import urljoin
from six.moves.urllib.parse import urlparse
from six.moves.urllib.parse import urlunparse

from . import errors
from .urls import get_normalized_url_variations
Expand Down Expand Up @@ -68,8 +67,7 @@ def _attempt_connections(self, urls):
)
logger.info("Success! We connected to: {}".format(response.url))

# return (scheme, netloc) of url
return urlunparse(parsed_url[:2])
return "{}://{}".format(parsed_url.scheme, parsed_url.netloc)
except (requests.RequestException) as e:
logger.info("Unable to connect: {}".format(e))
except ValueError:
Expand Down