kolibri
kolibri copied to clipboard
Add string prompt to log in with existing username or create an account
Summary
This pull request introduces the necessary string to prompt user to sign in, which improves the user interface when inputting an existing username.
Added String: Sign in instead?
https://github.com/learningequality/kolibri/assets/46411498/f051f98a-1220-47c6-b7d5-59890bb30d4a
Note: This pull request introduces changes to the component UserCredentialsForm which is used within the components SelectSuperAdminAccountForm and LodJoinFacility.
However these changes are only necessary for LodJoinFacility. The Sign in instead? link only displays if errorsCaught includes the error USERNAME_ALREADY_EXISTS, which is set in the component LodJoinFacility but does not get set in SelectSuperAdminAccountForm.
References
Closes #11882
Reviewer guidance
- Initiate a new Kolibri setup by creating a user account to add to an existing facility.
- Input a username that already exists within the facility.
Testing checklist
- [ ] Contributor has fully tested the PR manually
- [ ] If there are any front-end changes, before/after screenshots are included
- [ ] Critical user journeys are covered by Gherkin stories
- [ ] Critical and brittle code paths are covered by unit tests
PR process
- [ ] PR has the correct target branch and milestone
- [ ] PR has 'needs review' or 'work-in-progress' label
- [ ] If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
- [ ] If this is an important user-facing change, PR or related issue has a 'changelog' label
- [ ] If this includes an internal dependency change, a link to the diff is provided
Reviewer checklist
- Automated test coverage is satisfactory
- PR is fully functional
- PR has been tested for accessibility regressions
- External dependency files were updated if necessary (
yarnandpip) - Documentation is updated
- Contributor is in AUTHORS.md
Build Artifacts
| Asset type | Download link |
|---|---|
| PEX file | kolibri-.pex |
| Windows Installer (EXE) | kolibri-0.17.0a0.dev0+git.177.gc008dda6-windows-setup-unsigned.exe |
| Debian Package | kolibri_0.17.0a0.dev0+git.177.gc008dda6-0ubuntu1_all.deb |
| Mac Installer (DMG) | kolibri-0.17.0a0.dev0+git.177.gc008dda6-0.4.2.dmg |
| Android Package (APK) | kolibri-0.17.0a0.dev0+git.177.gc008dda6-0.1.3-debug.apk |
| TAR file | kolibri-0.17.0a0.dev0+git.177.gc008dda6.tar.gz |
| WHL file | kolibri-0.17.0a0.dev0+git.177.gc008dda6-py2.py3-none-any.whl |
Looks like all code changes are approved - so @pcenov and @radinamatic take it away!
Hi @LianaHarris360 for some reason I am always getting the following error in the console when attempting to create a new user regardless of whether that user already exists or doesn't exist:
SyntaxError: Unexpected token '<', " <!DOCTYPE "... is not valid JSON
And the following request response: Response.txt
https://github.com/learningequality/kolibri/assets/79847249/997be89b-6ca8-4799-afa4-dde8d596baba
Logs: Logs.zip
My guess here is that the csrf_exempt decorator and the csrf_protect decorator are not quite working properly together: https://github.com/learningequality/kolibri/blob/develop/kolibri/core/auth/api.py#L807
To fix this, I would suggest we tweak the inheritance slightly - to something like this:
class BaseSignUpViewSet(viewsets.GenericViewSet, CreateModelMixin):
serializer_class = FacilityUserSerializer
def check_can_signup(self, serializer):
facility = serializer.validated_data["facility"]
if (
not facility.dataset.learner_can_sign_up
or not facility.dataset.full_facility_import
):
raise PermissionDenied("Cannot sign up to this facility")
def perform_create(self, serializer):
self.check_can_signup(serializer)
serializer.save()
data = serializer.validated_data
authenticated_user = authenticate(
username=data["username"],
password=data["password"],
facility=data["facility"],
)
login(self.request, authenticated_user)
@method_decorator(csrf_protect, name="dispatch")
class SignUpViewSet(BaseSignUpViewSet):
pass
@method_decorator(csrf_exempt, name="dispatch")
class PublicSignUpViewSet(BaseSignUpViewSet):
"""
Identical to the SignUpViewset except that it does not login the user.
This endpoint is intended to allow a FacilityUser in a different facility
on another device to be cloned into a facility on this device, to facilitate
moving a user from one facility to another.
It also allows for historic serializer classes in the case that we
make an update to our implementation, and we want to keep the API stable.
"""
legacy_serializer_classes = []
def create(self, request, *args, **kwargs):
exception = None
serializer_kwargs = dict(data=request.data)
serializer_kwargs.setdefault("context", self.get_serializer_context())
for serializer_class in [
self.serializer_class
] + self.legacy_serializer_classes:
serializer = serializer_class(**serializer_kwargs)
try:
serializer.is_valid(raise_exception=True)
break
except Exception as e:
exception = e
if exception:
raise exception
self.perform_create(serializer)
headers = self.get_success_headers(serializer.data)
return Response(
serializer.data, status=status.HTTP_201_CREATED, headers=headers
)
def perform_create(self, serializer):
self.check_can_signup(serializer)
serializer.save()
That should ensure that the public sign up viewset is definitely not CSRF protected.
Hi @pcenov could you check to see if you are still experiencing that error?
Hi @LianaHarris360 - yes, I'm still getting the same error.
@pcenov were you testing with the asset from this PR for both sides of the learner creation? I tested with the asset from this PR for both the LOD that was creating a user and the server it was creating it on, and I had no issues. The fix I suggested @LianaHarris360 apply here was needed on the server side, rather than locally.
Yeah, I was using the asset from this PR on both sides but as I was in a hurry might have tried to create the user on one of the other servers that I was running... Anyways - after carefully checking today I confirm that this is fixed indeed, thanks @LianaHarris360!