Make the studio login over the lms optional using a feature flag #20007
Conversation
Squashed from: 80b977f Make the studio login over the lms optional using a feature flag 14b4223 Addressing feedback 9195ec9 Addressing second feedback about redirect logic on logout behing feature flag 923a917 Fixing lettuce tests (cherry picked from commit 6f91a0d9e8ea2364f58a2cd38e804ac57f8f416b) Commit 6f91a0d9 was a squashed version of the four original commits, to make resolving conflicts easier.
nedbat
force-pushed
the
nedbat/ironwood-19845
branch
from
10780a5 to
f8319d8
Compare
|
@nasthagiri @pwnage101 @felipemontoya I've cherry-picked #19845, but there were conflicts. I think I got it right, but please take a quick look. Thanks. |
|
By looking at the code, everything looks good to me. I'm launching a new ironwood devstack to test it |
|
@felipemontoya The tests are failing, due to /logout returning 200 instead of other things. @pwnage101 is looking into it. |
|
The reason the tests are failing is because the backported changes in https://github.com/edx/edx-platform/pull/19804 |
|
@nedbat @pwnage101 do you reckon we should backport those? or should I modify this lightly so that it works on top of ironwood as it is? |
|
I just spoke with @doctoryes about that option (slightly modify this backport PR logic to make it work), but we decided against that because we want to avoid diverging codebases as much as possible. If we diverge, that may cause another merge conflict in a future backport. |
Currently, the LMS logout endpoint should iframe in the logout pages of all the IDAs you were logged into. In short, this was made possible with DOP because keeping track of the logout URIs and leaving a trail of evidence in the user cookies was part of what we added in our fork of DOP. In the case of DOT, we don't have time or desire to fork DOT to mirror this behavior, so our stop-gap solution is to log out the user from a list of logout URIs in settings. (cherry picked from commit 10afe5e)
I missed the LMS production settings, and Studio in its entirety. (cherry picked from commit 11c3588)
pwnage101
left a comment
pwnage101
left a comment
There was a problem hiding this comment.
It also occurs to me that If we backport those extra PRs, we also should backport the corresponding config PR: https://github.com/edx/configuration/pull/5011/files
|
ironwood run bokchoy |
1 similar comment
|
ironwood run bokchoy |
|
Your PR has finished running tests. There were no failures. |
Squashed from:
80b977f Make the studio login over the lms optional using a feature flag
14b4223 Addressing feedback
9195ec9 Addressing second feedback about redirect logic on logout behing feature flag
923a917 Fixing lettuce tests
(cherry picked from commit 6f91a0d9e8ea2364f58a2cd38e804ac57f8f416b)
Commit 6f91a0d9 was a squashed version of the four original commits, to
make resolving conflicts easier.