longhornopen / qualtrics-lti

A simple LTI tool for integrating Qualtrics surveys into a course.
GNU Affero General Public License v3.0
6 stars 3 forks source link

Provide an optional configuration for encryption of user variables sent in URL to prevent value tampering #21

Closed jonespm closed 9 months ago

jonespm commented 10 months ago

To prevent tampering of URL parameters provided in #1 and #19 we should probably provide some type of optional encryption/secret that the user can define in the configuration.

There's two things I can think of

I'm somewhat leaning on trying public key first as there are a lot of web based generators of keys and the functions to decode are well documented.

I think if we did something too trivial like Base64 it would be just the same as sending it as-is.

jonespm commented 10 months ago

I was thinking of maybe something like this that would give the user the option to send this as well as encrypt it.

This was enabled by default in the past so we'd probably need a conversion that turns it on for all previous assessments. We could also just provide just the field to encrypt? It looks like having the sending configurable was mentioned in both #1 and #19 but if you don't actually use it in Qualtrics it doesn't do anything.

image

chrispittman commented 10 months ago

What is the end user supposed to do with this encrypted info once it's gathered the Qualtrics survey? Qualtrics can't decrypt it. So users who want to know student's identities would then have to ask Qualtrics-LTI for the encrypted strings for every user, so that they can cross-index against what's stored in the survey. And if you're doing that, it's not encrypted text; it's just an opaque string that exists for every user...and one of those already exists as a result of the LTI launch, so why not use that instead?

jonespm commented 10 months ago

Yeah right you couldn't do it in Qualtrics, you'd have to export the Excel/CSV from Qualtrics and decode it there somehow if you cared about it. There's probably some modules in Excel to do this but I haven't investigated that part. Hmm, okay so maybe just sending the opaque string from the LTI launch would be enough here to protect the integrity of the launch? Then maybe it doesn't even need this option. That would make this all easier.

jonespm commented 9 months ago

Which value were you thinking about here that exists in the LTI launch? The https://purl.imsglobal.org/spec/lti/claim/lti11_legacy_user_id?

I'm not seeing anything else that's identifying to the user. And that's possibly specific to Canvas?

chrispittman commented 9 months ago

user_id. https://www.imsglobal.org/best-practices-managing-ids-lti

jonespm commented 9 months ago

Okay I see in the launch it looks like it's in a block like this. Could use that.

Do you know if there's anyway to validate that user_id with the Canvas, like with an API? I have to assume it's static. Maybe I'll look into the code. Thanks!

(Some of these value are ... redacted)

If someone wanted to they probably should just make up a fake 40 character string and send that along with other info since it couldn't easily be validated. Maybe nobody would know this. I'm going to see with our client how it important it is and if they have any plans of decoding the values. Maybe it could also be included in the CSV that's exported from this Qualtrics LTI Connector, then it could be paired up easily with the export from Qualtrics.

"https://purl.imsglobal.org/spec/lti/claim/lti1p1": {
    "user_id": "4869...",
    "validation_context": null,
    "errors": {
      "errors": {}
    },
    "resource_link_id": "6fd9...",
    "oauth_consumer_key": "umich_prod",
    "oauth_consumer_key_sign": "QKPF..."
  },
chrispittman commented 9 months ago

user_id is an opaque value. "...make up a fake 40 character string" is exactly what the LTI launch is doing.

Can you talk a little bit about the actual use case you're trying to support here? This issue started as "we want to prevent tampering with user data" and is now at "we don't know if they have any plans to actually use this data". What are you trying to accomplish?

jonespm commented 9 months ago

I'll update you when I get more information back from the client.

I think our initial concern is that since this tool passes the values like sis_user_id over to LTI as a regular GET with values in the URL, anyone could semi-easily change that value to put in whatever they wanted to submit on behalf of other people. The researchers wanted to run their reports just from Qualtrics but be able to have ID's that are the a single response from the actual user.

Maybe nobody will do this but since it's easy to do it was a concern.

I don't see me saying they have no plans to actually use this data, they want to use the email address (since there is no SIS ID) and be confident it accurately represents the user launched from Canvas. I think passing the user_id along with the sis_user_id and email address could work if this tool can captures and exports it so I'll probably look into that as the simpler solution.

chrispittman commented 9 months ago

Let's go with the plan of giving people the option to decide which subset of [user_id, sis_user_id, email] they want to pass, if that works for you.

At UT Austin, our Qualtrics instance is set up to allow login through our SSO, and to allow users to require SSO login on a survey-by-survey basis. We find that this works pretty well for us, for the few cases where we really have to prove a survey-taker's identity. If that's an option for you too, it's worth looking into.

chrispittman commented 9 months ago

By the end of the day, we should have a new 'Options' block at the end of the survey configuration, which'll look something like this: Screenshot 2023-11-28 141102

Clicking these checkboxes should cause the relevant info to be included in the Qualtrics URL.

jonespm commented 9 months ago

Thanks we tested this and it looks like it should work for the needs of the researchers. They just got stuck on testing the survey out with the instructions provided and I created a new issue for that #23 . They were really just missing the last step there.

Thanks for implementing this.