LTI user provider may allow LMS admins to become Opencast admins

Steps to reproduce

Matthias Neugebauer
AttachmentsApr 17 (3 days ago)
to security
Hi,

The LTI documentation currently reads:

To give LMS users the same username in Opencast as the LMS username, uncomment the constructor arguments below and update CONSUMERKEY to the same key used above:
https://docs.opencast.org/r/5.x/admin/modules/ltimodule/

We use Opencast in this configuration so that the role provider can load additional roles. I have read the LTI
authentication code multiple times, but it never occurred to me that Opencast trusts the provided username
even for users like admin or opencast_system_account. I know that this a configuration problem, but in my
opinion it is quite surprising that the LMS admin could become the Opencast admin through LTI.

The attached patch adds an additional String parameter to the LtiLaunchAuthenticationHandler, which contains
an RegEx. Usernames that match will be rewritten like in the untrusted LTI configuration. The patch configures
admin and opencast_system_account to be in that expression by default.

Best regards
Matthias Neugebauer


Matthias Neugebauer
eLectures
Westfälische Wilhelms-Universität
Leonardo-Campus 3 - Raum 334
48149 Münster
Telefon: +49 251 83-38268
E-mail: matthias.neugebauer@uni-muenster.de

Activity

Show:
Greg Logan
May 8, 2018, 10:47 PM

Generally we wait until right before the release, then merge a branch like this in. I'm planning on cutting 3.6 a week or two before the 5.0 release, and 4.3 will release right after 3.6.

Matthias Neugebauer
May 9, 2018, 7:56 AM

That makes sense. Thanks for the info.

Stephen Marquard
August 10, 2018, 2:48 PM
Edited

The commit for this has swapped out use of the field lis_person_sourcedid (which is part of the LTI spec, i.e. a standard field) with ext_user_username which is not defined by the spec and platform-specific (probably this is set by moodle, but other LMSes may do something different).

https://github.com/opencast/opencast/commit/ef7bc8e94936b426bec5cb52f846d81c8c903657#diff-b59bf6d5547016d01917d80f5357b9feR177

Is there a reason why this change included that? It does not seem related to the actual issue that the commit was addressing.

Matthias Neugebauer
August 10, 2018, 3:17 PM

You are right. When I developed this patch, I looked at our Moodle instance where this field was used for the username. Having not read the LTI spec, I thought this is the proper field. Should have known from the name that this is an extension :/

I will create a followup patch next week. Just for clarification: are you saying that Opencast shouldn't use extension fields at all? I.e. would a patch work for you that prefers ext_user_username over lis_person_sourcedid over userIdFromConsumer?

Matthias Neugebauer
August 14, 2018, 11:08 AM

See MH-13034.

Assignee

Greg Logan

Reporter

Matthias Neugebauer

Severity

Security

Tags (folksonomy)

None

Components

Fix versions

Affects versions

Priority

Major
Configure