Following @sampaccoud's work on impress, add new fields to handle
user names in our application.
@sampaccoud preferred having a full and short names instead of
a basic first and last ones, to follow common good practices, and
avoid having frontend formating names (from my understanding).
Please see commit eee20033 on Impress.
I've protected this endpoint with a feature flag, and an authentication
class, as it will be exposed on the public internet.
I've tried to keep the viewset logic as minimal as possible, I've
to ship smth and will continue iterating on this piece of code.
At some point, abstracting webhook endpoint and authentication class
might be beneficial for the project. YAGNI as of today.
A recording is savable only if it's active or stopped. In other
status (error, already saved, etc.) it won't be possible. I might
iterate on this piece of code. Let's ship it.
When a new file is uploaded to a Minio Bucket, a webhook can be
configured to notify third parties about the event. Basically,
it's a POST call with a payload providing informations on the
event that just happened.
When a recording worker will stop, it will upload its data to a Minio
bucket, which will trigger the webhook.
Try to introduce the minimalest code to parse these events, discard
them whener it's relevant, and extract the recording ID, thus we
know which recording was successfully saved to the Minio bucket.
In the longer runner, it will trigger a callback.
Addressed a `DeprecationWarning` in `RecordingFactory` related to the
`_after_postgeneration` method, which will stop saving the instance after
postgeneration hooks in the next major release. To resolve this,
`skip_postgeneration_save=True` was added to `RecordingFactory.Meta` to
avoid extraneous save calls. Alternatively, if instance saving is needed,
the save call can be moved to postgeneration hooks or by overriding
`after_postgeneration`.
Avoided unnecessary manipulation of the room name to prevent issues when
starting an egress worker. Previously, hyphens were stripped from the room
name, likely inherited from the legacy setup with Jitsi in Magnify, though
the purpose of this change is unclear and might be an undesired legacy
feature.
To ensure accurate room matching during egress worker requests, this update
removes any manipulation of the room name. This approach minimizes the risk
of errors when initiating recordings and maintains the integrity of the
original room name throughout the process.
The LiveKit egress worker interactions are proxied through the backend for
security reasons. Allowing clients to directly use tokens with sufficient
grants to start recordings could lead to misuse, enabling users to spam the
egress worker API and potentially initiate a DDOS attack on the egress
service. To prevent this, only users with room-specific privileges can
initiate recordings.
We make sure only one recording at the time can be made on a room.
The requested recording mode is stored so it can be referenced later when
the recording is saved, triggering a callback action as needed.
A feature flag was also introduced for this capability; while this is a simple
approach, a more robust system for managing feature flags could be valuable
long-term. For now, KISS (Keep It Simple, Stupid) applies.
The viewset endpoints were designed to be as straightforward as possible—
let me know if anything can be improved.
Introducing a new worker service architecture. Sorry for the long commit.
This design adheres to several key principles, primarily the Single
Responsibility Principle. Dependency Injection and composition are
prioritized over inheritance, enhancing modularity and maintainability.
Interactions between the backend and external workers are encapsulated in
classes implementing a common `WorkerService` interface. I chose Protocol
over an abstract class for agility, aligning closely with static typing
without requiring inheritance. Each `WorkerService` implementation can
independently manage recordings according to its specific requirements.
This flexibility ensures that adding a new worker service, such as for
LiveKit, can be done without any change to existing components.
Configuration management is centralized in a single `WorkerServiceConfig`
class, which loads and provides all settings for different worker
implementations, keeping configurations organized and extensible. The worker
service class itself handles accessing relevant configurations as needed,
simplifying the configuration process.
A basic dictionary in Django settings acts as a factory, responsible for
instantiating the correct worker service based on the client's request mode.
This approach aligns with Django development conventions, emphasizing
simplicity. While a full factory class with a builder pattern could provide
future flexibility, YAGNI (You Aren't Gonna Need It) suggests deferring
such complexity until it’s necessary.
At the core of this design is the worker mediator, which decouples worker
service implementations from the Django ORM and manages database state
according to worker state. The mediator is purposefully limited in
responsibility, handling only what’s essential. It doesn’t instantiate
worker services directly; instead, services are injected via composition,
allowing the mediator to manage any object conforming to the `WorkerService`
interface. This setup preserves flexibility and maintains a clear
separation of responsibilities. The factory create worker services,
the mediator runs it.
(sorry for this long commit)
Write the proper ORM code to sanitize the rows in db and avoid
existing users lose access to our app.
Existing duplicate user accounts are merged, and resource accesses
are transferred.
Add test cases for email-based user matching fallback logic:
- String comparison edge cases
- Multiple users with matching email addresses
- Invalid email format handling
Fix will follow in subsequent commit.
When OIDC providers return random values in the "sub" field instead of stable
identifiers, implement email-based user matching as fallback.
Note: Current implementation needs improvement. Tests forthcoming.
Original: @sampaccoud (ff7914f) on Impress
Handle case where user is inactive.
Previously this edge case would cause unexpected behavior.
Related to previous commit that added the test coverage.
Remove redundant wrapper method that duplicates SUB validation logic.
Already validated in parent class, following the pattern established by
@sampaccoud in People and Impress modules.
Handle case where sub value is an empty string instead of None.
Previously this edge case would cause unexpected behavior.
Related to previous commit that added the test coverage.
Add failing test for corner case when sub value is an empty string.
This edge case was discovered by @sampaccoud and was previously untested.
Fix will follow in subsequent commit.
Manage Recording in the Django Admin. As of today, I have not enforced
a strict policy to avoid edit on recording rows or even creating new
data point directly from the admin. Will do in the future.
Implements routes to manage recordings within rooms, following the patterns
established in Impress. The viewset exposes targeted endpoints rather than
full CRUD operations, with recordings being created (soon) through
room-specific routes (e.g. room/123/start-recording).
The implementation draws from @sampaccoud's initial work and advices.
Review focus areas:
- Permission implementation choices
- Serializer design and structure
Credit: Initial work by @sampaccoud
The Recording model is introduced to track recording lifecycle within rooms,
while maintaining strict separation of access controls between rooms and
recordings.
Recordings follow the BaseAccess pattern (similar to Documents in Impress),
providing independent access control from room permissions. This ensures that
joining a room doesn't automatically grant access to previous recordings,
allowing for more flexible permission management.
The implementation was driven by TDD, particularly for the get_abilities
function, resulting in reduced nesting levels and improved readability.
The Recording model is deliberately kept minimal to serve as a foundation for
upcoming AI features while maintaining flexibility for future extensions.
I have avoided LiveKit-specific terminology for better abstraction.
Note: Room access control remains unchanged in this commit, pending future
refactor to use BaseAccess pattern (discussed IRL with @sampaccoud).
Renames a Django setting in the Resource base class to use resource-agnostic
terminology. While rooms are currently the only resource type, keeping base
class naming generic improves clarity.
Python module models.py raises too-many-ancestors warning, but linter
still pass, code is rated 10/10.
Inspired from @sampaccoud's commit #eee20032 on impress.
Fix following warning messages :
- You have not set a value for the SECURE_HSTS_SECONDS setting.
- Your SECURE_SSL_REDIRECT setting is not set to True.
Declare the expected support and analytics env variables
expected by the frontend for each environment. To avoid consuming
too much credits from our PostHog free tier plan.
Inspired by Joanie.
In Frontend context, env variables are only available at build time,
not runtime. This is one of the easiest way to pass frontend dynamic
configurations while running.
This commit only exposes the view already existing.
Fixed an issue where a setting cloned from Joanie was missing
in the Django configuration. Although its value was provided, it was
not applied due to the missing reference in the settings file.
'hash' built-in function is randomly seed by Python process.
In staging or production, our backend runs over 3 pods, thus 3
Python processes. For a given identity, it was not prompting
the same hash across all pods.
Why 'hash' is randomly seed? For security reasons, there was
a vulnerability disclosure exploiting key collision. Since Python 3.2,
'hash' is by default randomly seed.
Fixed it! Thx @jonathanperret for your help.
Participants need to be room admin to interact with LiveKit server
api. Until we offer private room, with moderation, all participants
will be considered as room admin.
note: room admin doesn't offer permission to record a room. please,
refer to LiveKit documentation to learn more.
Upgrading Django to 5.1 created a severe issue.
Migrate and create-superuser jobs were failing.
The issue originated from the third party easy_thumbnail.
Please refer to the issue #641 on their repo.
This is the suggested workaround by @Miketsukami.
Color is stored in participant's metadata.
Using a hash of some participant's info allow to persist the color
for logged-in participant without storing the color in db.
Please feel free to tune the saturation and lightness range.
Code is inspired by a tutorial found online.
Updated Django's ALLOWED_HOSTS setting from '*' to the specific host of the
server. Setting ALLOWED_HOSTS to '*' is a security risk as it allows any host
to access the application, potentially exposing it to malicious attacks.
Restricting ALLOWED_HOSTS to the server's host ensures only legitimate
requests are processed.
In a Kubernetes environment, we also needed to whitelist the pod's IP address
to allow health checks to pass. This ensures that Kubernetes liveness and
readiness probes can access the application to verify its health.
Silenced certain Django security warnings because the application is served
behind a reverse proxy. These warnings are not applicable in our deployment
context, where the reverse proxy handles these security concerns.
This change ensures relevant security measures are appropriately managed
while avoiding unnecessary warnings. Any question? asked @rouja.
/!\ actually, this commit is not working, and should be fixed.
