[Course Search] Restrict search results based on user permissions

[bradenmacdonald]

"As a course author, I want to control who can see my course content."

"As a library author, I want to control who can see my library content."

Requirements:

1. Update the search code from [Course Search] Initial Integration of Studio Search Backend #195 so that searches are limited to those courses which users have permission to view.
2. Should match how Studio interprets the permissions, handling things like org-level access correctly.

Notes:

• The frontend will connect directly to Meilisearch, so it's necessary to encode the permissions for the user into a "restricted key" that users can use. The place to put this is here. It should probably look something like the following filter: org = "SomeOrg" OR context_key IN ["lib:...", "course-v1:...", ...] (for a user with org-level permissions on some org, and explicit permissions on some libraries and courses) (Update: see below - a different approach is likely required)

[rpenido]

The frontend will connect directly to Meilisearch,

@bradenmacdonald I didn't understand how we could do this in the frontend (i.e a MFE) in a secure way. Could we pass the encoded JWT and let the Meilisearch parse it?

[bradenmacdonald]

@rpenido Yes, that's exactly what we do. An example of that is already implemented in the prototype.

[ormsbee]

FWIW, we've had situations where, for one reason or another, an individual is manually given access to hundreds of courses, rather than an org-level role. This led to massive JWTs that caused a bunch of problems.

Could we flip it the other way by encoding the course authors into the documents for a particular course? This would make adding and removing Studio permissions roles more expensive, since we'd have to update potentially a few thousand documents for large courses. But it would make the query part of it just have to send over the user's ID, instead of a potentially huge list of course IDs.

(We'd still encode global staff and org roles into the query, and not write them to docs.)

[bradenmacdonald]

@ormsbee Could we test this first and see what the limits of the JWT based approach is?

From what I can tell in the Meilisearch docs, there is no limit on how many values you can include in a flat filter expression, and this JWT is not being stored in a cookie nor URL nor GET request, so I think there's a chance it may be OK even with huge queries.

If not... could we just say you're limited to searching N courses if you don't have org-level permissions? (I think N could be very high). Or is this a not-uncommon case? If we need to support it, your approach makes sense but I'd prefer not to go that route unless necessary.

[ormsbee]

Sure. If the JWT is just being POSTed and isn't a part of the headers, then yeah, let's try that first. Thank you!

[bradenmacdonald]

@ormsbee Ah, turns out that the Meilisearch API client does POST the JWT as a header, not in the body (and there's no way around it). On my devstack it works fine with about 2,500 course IDs encoded in the JWT (a 133 KiB header value uncompressed), but going higher than that results in a HTTP 431 Request Header Fields Too Large. This is a bit surprising as I believe the default header size limit is usually much lower, like 8 KiB.

I think we'll have to go with the approach you suggested, unless there are any other ideas that are even simpler.

Though really I think the idea of managing permissions in a way that ever involves "an individual is manually given access to hundreds of courses" is a sign of a bad system. IMHO the correct fix is to support Groups or (better) something like GitHub Teams - so course permissions can only be managed by granting permissions to teams, which don't have to be 1:1 with orgs, and users get permissions if they're on a team that has the permission. This still requires updating the search index when the set of authorized groups for a course changes, but that happens even more rarely than individual user changes. Adding/removing a user to a team would not require any re-indexing.

[ormsbee]

@ormsbee Ah, turns out that the Meilisearch API client does POST the JWT as a header, not in the body (and there's no way around it). On my devstack it works fine with about 2,500 course IDs encoded in the JWT (a 133 KiB header value uncompressed), but going higher than that results in a HTTP 431 Request Header Fields Too Large. This is a bit surprising as I believe the default header size limit is usually much lower, like 8 KiB.

Yeah. I unfortunately had to look into this during an edX investigation a few years ago that was internally dubbed "Cookie Monster". By default, 8K is all you can depend on working everywhere. You can raise that in configuration on gunicorn and nginx, but at 16K you hit the AWS ELB limit (assuming you're deploying that way). At 32K you hit Cloudflare limits.

Though really I think the idea of managing permissions in a way that ever involves "an individual is manually given access to hundreds of courses" is a sign of a bad system. IMHO the correct fix is to support Groups or (better) something like GitHub Teams - so course permissions can only be managed by granting permissions to teams, which don't have to be 1:1 with orgs, and users get permissions if they're on a team that has the permission. This still requires updating the search index when the set of authorized groups for a course changes, but that happens even more rarely than individual user changes. Adding/removing a user to a team would not require any re-indexing.

I agree that it's a better world to be in, but doing so is beyond the scope of this project. But even if we got there, we'd likely still end up supporting backwards compatibility or making an implicit group per course as a migration step... which gets us back to where we started.

I think we'll have to go with the approach you suggested, unless there are any other ideas that are even simpler.

Though wait, maybe we can cheat on the representation. CourseKeys are really long, but we could have a much more compact representation available to us on any given server. What if we add a unique AutoField to CourseOverview (you know, the thing that should be the primary key for that table if it were saner)? (Or a new table, if we wanted to be paranoid.) Then we add a new index in the docs that has just that integer course identifier. We'd be back at "send ridiculous list of course IDs in the JWT", but we can stuff a thousand IDs without going over the limit.

[ormsbee]

Though wait, maybe we can cheat on the representation. CourseKeys are really long, but we could have a much more compact representation available to us on any given server. What if we add a unique AutoField to CourseOverview (you know, the thing that should be the primary key for that table if it were saner)? (Or a new table, if we wanted to be paranoid.) Then we add a new index in the docs that has just that integer course identifier. We'd be back at "send ridiculous list of course IDs in the JWT", but we can stuff a thousand IDs without going over the limit.

FYI @feanil, since this tangentially relates to improving the modeling for course runs in the system.

[ormsbee]

Okay, fresh data from our favorite instance:

• the top 35 authors have course-level permission granted on the order of 1000-1200 individual courses
• looking at the top 100 authors, it drops to access to ~400 individual courses
• top 500-level authors have authoring access to ~100 individual courses

[bradenmacdonald]

Thanks @ormsbee. I like that idea of using integer IDs to CourseOverview. From some quick checks, if the average integer ID is 4 digits, we would need to set a limit of 1,000 course IDs (which would result in a JWT that's 7 KiB). Presumably this would be OK - if you are explicitly added as an author on more than 1,000 courses to which you don't have any other (org-level or global) access, you will only be able to search the 1,000 most recent such course runs.

[ormsbee]

Yeah, I'm quite comfortable with saying, "If you've been manually added to over a thousand courses instead of having an org role, then Studio cross-course content search will only cover the last thousand courses you were added to." @jmakowski1123: ^ Please let us know if you think that is okay.

FYI also to @JAAkana, as you folks have one of the few instances I can imagine being affected by this limitation.

[pomegranited]

@bradenmacdonald @ormsbee Instead of adding another ID field to CourseOverview, can we use a hashed course ID? We can generate short hex digests using hashlib.shake_256 (cf stackoverflow, hashlib), and we only need a one-way hash.