Skip to content

Use token generator to authenticate just provisioned facility superadmin #13732

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 12, 2025
Merged

Use token generator to authenticate just provisioned facility superadmin #13732

merged 1 commit into from
Sep 12, 2025

Conversation

@AlexVelezLl
Copy link
Member

@AlexVelezLl AlexVelezLl commented Sep 10, 2025 •
edited
Loading

Summary

Imported users using the admin account in the setup wizard plugin were not being able to login after the device was provisioned because the frontend didn't have their password, so the login method was failing. To prevent this, we now use the TokenGenerator class to generate a temporary token to authenticate the superadmin user just after the device was provisioned.

  • Updated the provisiondevice task to generate the auth token once the provision was completed.
  • Updated the SessionViewSet's create method to validate the auth_token generated on the deviceprovision task.
  • The SessionViewSet's create method was refactored to use a serializer to get validate its params and get the user is being authenticated, and improve the readibility of the code.
  • Moved the TokenGenerator class from kolibri/plugins/user_profile/utils.py to kolibri/core/utils/token_generator.py so we can access it in the setup wizard plugin too.
  • Fixed the infinite "setting up" loading screen when there is an error in the login. Now if for some reason an error exists in the login method, the user will be redirected to the sign in page.
  • Cleaned deviceprovision failed tasks, so that it does not affect newly created tasks.
  • Fixed the provisiondevice task error handling.

References

Closes #13715.

Reviewer guidance

  • Replicate steps taken in Setup Wizard - Getting an error when importing users #13715, and make sure the user is correctly authenticated after setting up Kolibri.
  • Check that there are no regressions in the authentication methods:
    • User log in with/without password.
    • User authentication using an OS user (e.g. in the android app).
    • User changing their password.

@github-actions github-actions bot added DEV: backend Python, databases, networking, filesystem... APP: User Re: User app (sign-in, sign-up, user profile, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: frontend SIZE: medium labels Sep 10, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 10, 2025 •
edited
Loading

Build Artifacts

Asset type Download link
PEX file kolibri-0.18.2a0.dev0_git.21.g642bc83d.pex
Windows Installer (EXE) kolibri-0.18.2a0.dev0+git.21.g642bc83d-windows-setup-unsigned.exe
Debian Package kolibri_0.18.2a0.dev0+git.21.g642bc83d-0ubuntu1_all.deb
Mac Installer (DMG) kolibri-0.18.2a0.dev0+git.21.g642bc83d.dmg
Android Package (APK) kolibri-0.18.2a0.dev0+git.21.g642bc83d-0.1.5-debug.apk
Raspberry Pi Image kolibri-pi-image-0.18.2a0.dev0+git.21.g642bc83d.zip
TAR file kolibri-0.18.2a0.dev0+git.21.g642bc83d.tar.gz
WHL file kolibri-0.18.2a0.dev0+git.21.g642bc83d-py2.py3-none-any.whl

@AlexVelezLl AlexVelezLl marked this pull request as ready for review September 11, 2025 15:20
@rtibbles rtibbles self-assigned this Sep 11, 2025
rtibbles
rtibbles reviewed Sep 11, 2025
View reviewed changes
Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

This all makes sense to me - one possible bit of even further cleanup!

kolibri/core/auth/api.py
username = request.data.get("username", "")
password = request.data.get("password", "")
facility_id = request.data.get("facility", None)
def validate(self, attrs):
Copy link
Member

Choose a reason for hiding this comment

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

I wasn't even thinking that we could move so much of the viewset logic into the validate method, but this is a really nice separation of concerns, and provides such a neat interface between the serializer and the viewset, where the viewset only has to worry about the user and nothing else!

kolibri/plugins/user_profile/viewsets.py

from .utils import TokenGenerator
from kolibri.core.auth.models import FacilityUser
from kolibri.core.utils.token_generator import TokenGenerator
Copy link
Member

Choose a reason for hiding this comment

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

My only other thought here is, could we actually delete the MergedUserLoginViewset, and just update the frontend in the user merging workflow to use your new enhanced SessionViewset instead?

Copy link
Member

Choose a reason for hiding this comment

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

We'll file a follow up issue for this to look at on develop as cleanup!

@github-actions github-actions bot added the APP: Learn Re: Learn App (content, quizzes, lessons, etc.) label Sep 11, 2025
@AlexVelezLl AlexVelezLl changed the base branch from develop to release-v0.18.x September 11, 2025 16:24
@rtibbles
Copy link
Member

This is good to go and ready for manual QA.

@pcenov pcenov self-requested a review September 12, 2025 13:07
pcenov
pcenov approved these changes Sep 12, 2025
View reviewed changes
Copy link
Member

@pcenov pcenov left a comment

Choose a reason for hiding this comment

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

Thanks @AlexVelezLl - LGTM, no regressions observed while manually testing.

@AlexVelezLl
Copy link
Member Author

Thanks @pcenov @rtibbles!

@AlexVelezLl AlexVelezLl merged commit 601f202 into learningequality:release-v0.18.x Sep 12, 2025
51 checks passed
@AlexVelezLl AlexVelezLl deleted the fix-lod-import-users branch September 12, 2025 19:13
@marcellamaki marcellamaki mentioned this pull request Sep 15, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rtibbles rtibbles rtibbles left review comments

@pcenov pcenov pcenov approved these changes

Assignees

@rtibbles rtibbles

Labels

APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) DEV: backend Python, databases, networking, filesystem... DEV: frontend SIZE: medium

Projects

None yet

Milestone

Kolibri 0.18: Planned Patch 3

Development

Successfully merging this pull request may close these issues.

Setup Wizard - Getting an error when importing users

3 participants

@AlexVelezLl @rtibbles @pcenov