[MBL-19218][S/T/P] CrossShard & Trusts - Grades & Submissions Not Being Accessible #3707
Conversation
…In-Trusted-Accounts-Setup
refs: MBL-19218 affects: Student, Teacher, Parent builds: Student, Teacher, Parent release note: Resolved the issue of inaccessible content of courses in trusted accounts.
BuildsCommit: Address code review comments (e5b8733)
|
| guard let cSession = base.currentSession else { return nil } | ||
|
|
||
| let userID: String | ||
| if let userShardID = cSession.userID.shardID, userShardID == contextShardID { |
There was a problem hiding this comment.
Given that both PostLogin & GetUser APIs always respond with userID of global-form when user is not defined in the same account is being logged in. We only need to compare against contextShardID in this case.
For the other case, the environment override definitely was triggered by a course URL hosted in a different account than current user account. Thus it must be always set to the global-form.
| url.percentEncodedQuery = url.percentEncodedQuery?.replacingOccurrences(of: "+", with: "%20") | ||
|
|
||
| if let apiHostOverride = courseTabUrlInteractor?.baseUrlHostOverride(for: url) { | ||
| if url.path.isNotEmpty, url.path.hasPrefix("/") == false { |
There was a problem hiding this comment.
Indeed, what motivated this change is the line below:
And I had multiple cases where, while testing, routing would fail for missing out this / prefix.
So I thought we can have it integrated into our route matching logic.
| set {} | ||
| } | ||
|
|
||
| public override func transformContentIDs(params: [String: String], url: URLComponents) -> ([String: String], URLComponents) { |
There was a problem hiding this comment.
I would extend the name to tell the reader that what IDs are transformed to. transformContetIDsToLocalIDs maybe?
3 participants
refs: MBL-19218, MBL-19410
affects: Student, Teacher, Parent
builds: Student, Teacher, Parent
release note: Resolved the issue of inaccessible content of courses in trusted accounts.
Root Cause Analysis
Upon investigation, turned out that most of reported issues lately are caused by the mismatched
userIDpassed as an argument for API requests, or used for filtering and use-case scoping.To solve those, following actions were taken:
1- Changing all course/group content related code to make sure
userIDvalue used in them is taken fromenv.currentSession.userIDproperty. Which gets overridden inAppEnvironmentOverride. This value will be used for API arguments, filtering and scoping within those parts of code.2- Fixing the environment overridden value of
userIDaccording to the following rationale:If target course/group is hosted in the same account where user is defined:
-> userID must be set with local form.
If target course/group is hosted in an account different from that the user belongs to:
-> userID must be set with global form.
In order to check if a course/group and a user is hosted in the same account, their
shardIDis compared.As a result of implementing above steps, all issues with Masquerading got resolved.
3- To be more consistent on the IDs part of the problem, all internal content IDs (assignmentID, quizID, submissionID ..etc) would always be transformed intentionally to their local-form when they get used to fetch, filter, scope data for courses hosted in other accounts. The only exception was made for Course & Course Settings related use cases, which performed by the overridden "modified(for:)" functions, that's to match them to those objects fetched in Dashboard, right before user navigates to their details.
ℹ️ A Useful Information 🙂
All content of an "external-to-current-account" course can be accessed with root-host URL, for that you need to use global-form for all IDs involved in the request (Course ID, Assignment ID, Submission ID .. etc).
But the caveat would be the problems with permissions and
can_ ..attributes retrieved from API calls. Also, posting content APIs (submit assignment for example) will fail when used through that URL host.As a clear example for this, check out this issue :
When consulting assignment details API through root host URL, the retrieved object will have the value of
can_submitset to false. While, it will be retrievedtrueif requested through owning account host URL.Therefore, we are obliged to use the local form of course content APIs to solve such issues. Almost mimicking the same way used on the web version.
Other Changes
All "Customize Course" screens, (both presented from Dashboard, and the one by tapping on the gear button of Course Details screen), were tweaked to respect overridden environment. This solves a failure happening when trying to customize a course of a different account.
Groups tabs access now are fixed when accessed by a user of different account, in the same way used for courses ones.
Test Plan
To complete this effort, a regression test was made on all previous reported issues of cross-shard, consortia, and trusted accounts topics. While that might be a cumbersome task, doing the following tests might be sufficient:
1- Test the use case reported in ticket using accounts listed there, using u/p login & with QR code login (Masquerading). Also, please check if this fixes the issue reported on ticket MBL-19410.
2- Login with a regular account user, make sure to you can access all course content. Do a smoke test for submissions and comments.
Checklist