Remove w2py session cookie from lti logins - #1346
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to fully remove legacy Web2py session-cookie handling from the LTI 1.1 and LTI 1.3 login/launch flows, leaving only the newer auth/session mechanism (JWT cookie) in place.
Changes:
- LTI 1.3: Stop setting the Web2py session cookie during launch and deep-link responses; adjust call sites to expect
login_or_create_user()to return only anLti1p3User. - LTI 1.1: Remove Web2py session-cookie retrieval/setting during
_finalize_login()and simplify imports accordingly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| bases/rsptx/admin_server_api/routers/lti1p3.py | Removes Web2py cookie setting in LTI 1.3 routes and changes login_or_create_user()’s declared return type (but currently leaves a runtime-breaking return mismatch). |
| bases/rsptx/admin_server_api/routers/lti1p1.py | Removes Web2py cookie bridging from LTI 1.1 final login response and drops related imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async def login_or_create_user( | ||
| launch: FastAPIMessageLaunch, lti_course: Lti1p3Course, course: CoursesValidator | ||
| ) -> tuple[Lti1p3User, str]: | ||
| ) -> Lti1p3User: |
c0f7182 to
01b51e6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
bases/rsptx/admin_server_api/routers/lti1p1.py:326
- The
_finalize_logindocstring above this block still says it sets a web2py session cookie, but the web2py cookie logic was removed in this PR. Please update the docstring to reflect that only the Runestone JWT cookie is set now.
data={"sub": user.username}, expires=datetime.timedelta(hours=12)
)
auth_manager.set_cookie(response, access_token)
rslogger.debug(f"LTI1.1 - sending user to {redirect_to}")
bases/rsptx/admin_server_api/routers/lti1p3.py:986
user_confirmationis assigned but never used. This is dead code and may fail linting; if the intent is only to ensure the user/mapping exists, just await the call without binding the result.
user_confirmation = await login_or_create_user(message_launch, lti_course, course)
01b51e6 to
c1938b9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
bases/rsptx/admin_server_api/routers/lti1p3.py:155
login_or_create_userstill has a silent failure path: theexcept Exception as e:block aroundcreate_user(...)constructs anHTTPException(...)but does notraiseit, so execution can continue with an invaliduserand later crash (e.g., onuser.id) instead of returning a proper error response.
async def login_or_create_user(
launch: FastAPIMessageLaunch, lti_course: Lti1p3Course, course: CoursesValidator
) -> Lti1p3User:
"""
Helper function for routes that bring an LMS user to Runestone.
"""
c1938b9 to
ced9225
Compare
|
Just fixed the linting issues. The |
Purge w2py from LTI logins.
This pull request simplifies the LTI login and session management logic by removing all handling of the legacy Web2py session cookie from both LTI 1.1 and LTI 1.3 authentication flows. The changes streamline user login and session creation, focusing solely on the new authentication mechanism.
LTI Session Handling Simplification:
add_w2py_session_cookieandget_web2py_session_cookiefrom both LTI 1.1 and LTI 1.3 routers, eliminating legacy Web2py session cookie logic. [1] [2] [3] [4]login_or_create_userfunction inlti1p3.pyto return only the user object instead of a tuple with a session cookie, and adjusted all calling sites accordingly. [1] [2] [3]