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

LTS: Issues with 005-1-invalid-au.zip #20

Closed
cars10 opened this issue Nov 9, 2021 · 7 comments
Closed

LTS: Issues with 005-1-invalid-au.zip #20

cars10 opened this issue Nov 9, 2021 · 7 comments

Comments

@cars10
Copy link

cars10 commented Nov 9, 2021

Hi, we have some issues with 005-1-invalid-au.zip, maybe you can give us some hints.

1. send statement before loading learner prefs

1.1

The requests to retrieve the cmi5LearnerPreferences does not contain any session or AU information at all. I think there is no way for an LMS to check if the learner preferences were requested for this specific AU, at least not if users are potentially able to execute multiple AUs in parallel. How are we supposed to solve this?

1.2

Where does this requirement even come from? The spec only says that the AU MUST request the learner preferences. It does not say that the LMS actually has to verify this.

2. save learner prefs with invalid language preference values

We currently return a 400 BAD REQUEST for invalid learner preferences but the testsuite fails. It shows the following error message:

4.2.0.0-2 - The LMS MUST have an account which is able to retrieve all Resource data (from the Statement API, etc, including attachments and extensions) about another distinct user across multiple sessions for that user.: Save learner preferences response status code: 400  (Testing 11.0.0.0-5 (d))

This error message has nothing to do with the specific test so we are not sure what is wrong. Also, something that we noticed through the whole testsuite: it tells you that (in this case) a 400 is the wrong status code. But it never tells you what the correct one would be, why?

@gavbaa
Copy link
Contributor

gavbaa commented Nov 9, 2021

1.1
The requests to retrieve the cmi5LearnerPreferences does not contain any session or AU information at all. I think there is no way for an LMS to check if the learner preferences were requested for this specific AU, at least not if users are potentially able to execute multiple AUs in parallel. How are we supposed to solve this?

Per section 9.3.6 Abandoned ( https://github.com/AICC/CMI-5_Spec_Current/blob/quartz/cmi5_spec.md#936-abandoned ): If an LMS launches an AU with the same registration as an active session, the previous session with that registration MUST be abandoned.

Given this, the spec specifically disallows multiple AUs launched in parallel, which the LMS is responsible for. From there, you can verify that the only active launch is the one making the learner preferences check, and if an "initialized" comes before a learner preferences check on this launch, you have a violation.

1.2
Where does this requirement even come from? The spec only says that the AU MUST request the learner preferences. It does not say that the LMS actually has to verify this.

It's reasonable to state that there is a derived requirement here, as if the AU must do something, the LMS must prevent the AU from not doing something wherever possible. CATAPULT has a collection of derived requirements for situations like this.

That having been said, we will be reviewing this specific item at the next spec group meeting, and will decide on what (if any) the appropriate action or clarification should be in either the spec itself or in CATAPULT.

  1. save learner prefs with invalid language preference values
    We currently return a 400 BAD REQUEST for invalid learner preferences but the testsuite fails. It shows the following error message:

4.2.0.0-2 - The LMS MUST have an account which is able to retrieve all Resource data (from the Statement API, etc, including attachments and extensions) about another distinct user across multiple sessions for that user.: Save learner preferences response status code: 400 (Testing 11.0.0.0-5 (d))
This error message has nothing to do with the specific test so we are not sure what is wrong. Also, something that we noticed through the whole testsuite: it tells you that (in this case) a 400 is the wrong status code. But it never tells you what the correct one would be, why?

To address the "what is wrong" part first: 11.0.0.0-5 (d) is the actual requirement failing, which comes from https://github.com/AICC/CMI-5_Spec_Current/blob/quartz/cmi5_spec.md#110-xapi-agent-profile-data-model : The LMS/LRS MAY choose to ignore or override Learner Preference changes requested by the AU by returning a "403 Forbidden" response as defined in the xAPI specification (Section 7.6). The AU MUST NOT treat the 403 response as an error condition.. Due to this, any issue with storing on this request is treated as requiring a 403, including issues that are cmi5 data validation issues, as (currently) there is no carve-out for data validation in this particular clause. For a future version of the spec, having a separation between data validation and other reasons for write rejection is a topic worth discussing.

As far as bubbling up more helpful errors in the responses, we'd be happy to review any PRs that would improve the messaging about requirements and test failures.

@cars10
Copy link
Author

cars10 commented Nov 10, 2021

Thanks for the quick answer!

Given this, the spec specifically disallows multiple AUs launched in parallel

I do not agree on this, i think this only disallows launching the same AU multiple times. It does not say anything about launching two completely different AUs with a different registration at the same time.

Due to this, any issue with storing on this request is treated as requiring a 403, including issues that are cmi5 data validation issues

Thanks, this was not clear to me.

As far as bubbling up more helpful errors in the responses, we'd be happy to review any PRs that would improve the messaging about requirements and test failures.

I will look into this in the future. Personally i expected something similar to the lrs-conformance-test-suite that uses assertions for status codes etc.

@gavbaa
Copy link
Contributor

gavbaa commented Nov 10, 2021

I do not agree on this, i think this only disallows launching the same AU multiple times. It does not say anything about launching two completely different AUs with a different registration at the same time.

Because these words are so overloaded in learning, I'm going to include the cmi5 definitions here to help clarify things.

Registration: An enrollment instance of a learner in a course. (a registration ID uniquely identifies this). The registration ID persists throughout the course progress to completion and during review of a completed course. A new registration is created for new enrollment instances (such as recurrent courses or re-taking courses).

Session: A period of time marked by the launch of an AU until its termination (or abandonment).

So there's a couple things to take apart in your message:

  1. cmi5 doesn't say anything about a learner in two different registrations. If the LMS allows a learner to have two separate registrations open (which would be two fully separate tabs/windows), all actions on one registration have no relation to the other registration. The entire cmi5 spec is only talking about a learner in a single registration. From an xAPI perspective, the statements generated always have a context.registration stamped on them that's related to what the AU is launched with, so there's no meaningful way they'd overlap anyway and their sessions are independent. (session ID is also stamped on the statements, https://github.com/AICC/CMI-5_Spec_Current/blob/quartz/cmi5_spec.md#9631-session-id )

  2. A session for a learner only exists for exactly one AU at a time within a registration, per the Abandoned text from above. That means it's the LMSs job to maintain state within the registration of which AU currently has an active session, if any. If a session is active, and a new AU is being launched, the spec requires the previous AU get marked as Abandoned and therefore closes the session.

This was a topic in the cmi5 WG earlier this year, specifically around making sure the language was clear that Abandoned also included the same AU as well as any other AU within a single registration. https://github.com/AICC/CMI-5_Spec_Current/wiki/CMI-5-Working-Group-Meeting-Minutes-%E2%80%93-June-25th . You can see the most relevant note here at the bottom:

  • Can the same learner have two sessions in the same AU? (This is an easy “no” because of registration conflicts and possibilities of repeating Statements)
  • Can the same learner have two sessions in the same Course? (This is also an easy no because of registration conflicts and possibilities of stacking “moveOn”)
  • Can the same learner have two sessions in the same LMS? (We believe this is an implementation detail. cmi5 doesn’t forbid it. The two sessions for the learner would have to be different registrations.

The last item matches with the current Abandoned text, and the standard's definition of "registration".

@cars10
Copy link
Author

cars10 commented Nov 11, 2021

But isn't the last point exactly what this issue is about?

Can the same learner have two sessions in the same LMS? (We believe this is an implementation detail. cmi5 doesn’t forbid it. The two sessions for the learner would have to be different registrations.

This says that it IS allowed to run two different AUs from different courses (and therefore different registrations) at the same time. And if this is allowed then there is no way to check if the cmi5LearnerPreferences where requested for the running session 1 or session 2 because the request contains no information about the session, registration or AU.

Or did i misunderstand you?

@gavbaa
Copy link
Contributor

gavbaa commented Nov 11, 2021

That's an implementation detail of your LMS, not one defined by a specific approach in the cmi5 specification. For example, the CTS launches all courses with a session ID in the xAPI endpoint. From my local CTS launch:

http://localhost:63398/content/1/13/index.html?endpoint=http%3A%2F%2Flocalhost%3A63399%2Fapi%2Fv1%2Fsessions%2F170%2Flrs&...

Decoded, that endpoint is http://localhost:63399/api/v1/sessions/170/lrs. Your LMS/LRS would therefore have access to the session ID as part of the URL path. You might choose to do this differently based on your specific environment's needs.

@cars10
Copy link
Author

cars10 commented Nov 11, 2021

That's an implementation detail of your LMS, not one defined by a specific approach in the cmi5 specification.

I know, but i am still asking for guidance because we had trouble coming up with a solution for this.

For example, the CTS launches all courses with a session ID in the xAPI endpoint.

This is a very interesting idea, thanks! We didn't think of that, adding the session ID to the xapi base url seems like a great idea. We will probably use something similar for our lms.


It's reasonable to state that there is a derived requirement here, as if the AU must do something, the LMS must prevent the AU from not doing something wherever possible.

Personally i still think that this is debatable, because then you could argue that every MUST for the AU is also a requirement for the LMS to check (and even vice-versa). But i guess this is a topic for a future version or the weekly meeting.

Thanks for the help and explanation!

@cars10 cars10 closed this as completed Nov 11, 2021
@gavbaa
Copy link
Contributor

gavbaa commented Nov 11, 2021

For future readers, besides modifying the URL path, the most reliable way to pair the session with the learner preferences is that every xAPI request to the LRS must have the Authorization Token in it.

8.2.1: The authorization token returned by the fetch URL MUST be limited to the duration of the session.

8.1.2: The AU MUST place the authorization token in the Authorization headers of all HTTP requests made to the endpoint using the xAPI.

With these combined, looking up your authorization token at the LRS request level is the most reliable way to proceed. Modifying the LRS endpoint URL can provide other benefits, but this is probably the most secure way to identify the session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants