openedx / xblock-lti-consumer

GNU Affero General Public License v3.0
27 stars 80 forks source link

Potential creation of duplicate LtiConfiguration models #487

Open Zacharis278 opened 1 month ago

Zacharis278 commented 1 month ago

Problem

On the edx.org site we have noticed a small number of LtiConfiguration objects show up that are duplicates for the same block location. This should not be possible since we only every create one LtiConfiguration per block based on the get_or_create call referenced below. This in the only function that can create an LtiConfiguration instance.

https://github.com/openedx/xblock-lti-consumer/blob/2efe2bb35332f66813351ed99ec6ed8d06b23dbc/lti_consumer/api.py#L38

However, this constraint is only enforced in code and not at the database level. If multiple calls to this function are received concurrently on a site with multiple workers, it is possible to end up with multiple objects.

I believe this is the cause for two reasons:

  1. All duplicate instances have adjacent database ids
  2. During a period of degraded site performance or even a bad client network connection it is easy to POST multiple requests. The modal to save the lti configuration does not close or disable while a request is in flight.

Impact

If there are multiple configurations with the same location identifier the LTI unit will not render due to a lti_consumer.models.LtiConfiguration.MultipleObjectsReturned error when any code attempts to reference that config.

Possible Solutions

  1. Add a database constraint to enforce the location attribute is unique.
    • This might be tricky to release since the new constraint will fail without manually cleaning up this data. It's possible other site unknowingly have this problem.
  2. Alter the UI so it's less likely a user could submit the same configuration multiple times.
    • Doesn't fix the underlying issue but may make it less likely to occur