LTI date updates

[ascholerChemeketa]

Implements changes proposed in Discord.

Maybe wait another day or two to see if anyone has strong opinions. Posting now to see what the review bot says.

[Copilot]

Pull request overview

This PR updates LTI 1.3 integration to support additional date fields (availability start/end) via custom launch parameters and deep-link resource payloads, along with a small UI visibility-mode precedence change and some HTTPS detection adjustments.

Changes:

• Add LTI custom-claim helpers and propagate custom params through the LTI 1.3 launch flow to update visible_on / hidden_on.
• Extend deep-link resource serialization to include available.startDateTime / available.endDateTime and format stored UTC datetimes for outbound LTI messages.
• Adjust assignment visibility-mode precedence when both schedule dates are present, and expand HTTPS detection for proxied FastAPI requests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
sample.env	Updates wording around SERVER_PROTOCOL guidance.
components/rsptx/lti1p3/pylti1p3/message_launch.py	Adds helpers to read LTI custom claim params from the JWT payload.
components/rsptx/lti1p3/pylti1p3/deep_link_resource.py	Adds available start/end datetime fields to deep-link resource serialization.
components/rsptx/lti1p3/pylti1p3/contrib/fastapi/request.py	Treats x-forwarded-proto as HTTPS for secure-request detection.
components/rsptx/auth/session.py	Changes how “production/HTTPS” is detected for cookie flags.
bases/rsptx/assignment_server_api/assignment_builder/src/components/routes/AssignmentBuilder/components/edit/visibilityMode.ts	Makes scheduled_period take precedence when both schedule dates exist.
bases/rsptx/assignment_server_api/assignment_builder/src/components/routes/AssignmentBuilder/components/edit/visibilityMode.spec.ts	Updates test expectation to match the new precedence.
bases/rsptx/admin_server_api/routers/lti1p3.py	Adds LTI datetime parse/format helpers; updates assignment date syncing and deep-link creation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bnmnetp]

should add server_protocol to configuration/core.py

[ascholerChemeketa]

Done

[ascholerChemeketa]

Updated

[ascholerChemeketa]

Overwrite is sometimes desired. I refined the logic to handle a Canvas special case and document choices

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

components/rsptx/lti1p3/pylti1p3/contrib/fastapi/request.py:70

• X-Forwarded-Proto is treated as a raw substring match without normalizing case or handling the common comma-separated format. This can mis-detect secure requests behind some proxies (e.g. header value "HTTPS" or "https, http"), which affects downstream cookie/SameSite behavior and URL construction.

        is_https = (
            request_obj.url.scheme.lower() == "https"
            or "https" in request_obj.headers.get("x-forwarded-proto", "")
        )

bases/rsptx/admin_server_api/routers/lti1p3.py:640

• Availability custom params (visible_on/hidden_on) are parsed as UTC even when the LMS sends a naive ISO datetime (no offset/Z). This is inconsistent with the due-date ingest logic a few lines above (which treats naive timestamps as course-local wall clock), and would store the wrong instant for platforms that omit timezone offsets.

        # If the LMS does not recognize the variable, it will be returned verbatim.
        # Otherewise, it should be a valid ISO datetime string or empty string.
        # Empty indicates a meaningful null value.
        try:
            availability_datetime = parse_lti_datetime_as_utc(raw_value)

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (5)

components/rsptx/lti1p3/pylti1p3/contrib/fastapi/request.py:70

• "https" in x-forwarded-proto can yield false positives (e.g. header value "httpsomething") and doesn’t correctly handle comma-separated lists. This can misclassify requests as secure/insecure and affect cookie/redirect behavior. Consider parsing the header into tokens and checking for an exact https value.

        is_https = (
            request_obj.url.scheme.lower() == "https"
            or "https" in request_obj.headers.get("x-forwarded-proto", "").lower()
        )

bases/rsptx/admin_server_api/routers/lti1p3.py:158

• parse_lti_datetime_as_utc() treats any falsy value as “missing” (if not datetime_string ...). If a non-string falsy value (e.g. 0/False) ever leaks into custom_params, this will be interpreted as a meaningful null and may clear an existing date. Since the function contract is Optional[str], it should check explicitly for None/empty-string and reject non-strings.

    if not datetime_string or datetime_string == "":
        return None

bases/rsptx/assignment_server_api/assignment_builder/src/components/routes/AssignmentBuilder/components/edit/visibilityMode.spec.ts:34

• This test encodes the same scheduled_period-when-visible behavior as getVisibilityMode() and will pass even though it conflicts with server-side visibility semantics (scheduled period is visible=false + both dates). If getVisibilityMode is corrected to only return scheduled_period when visible is false, this should continue to expect scheduled_hidden for visible=true + hidden_on (even if visible_on is also set).

  it("prefers scheduled_period when both visible_on and hidden_on are set, even if visible", () => {
    expect(getVisibilityMode(true, "2026-01-01T00:00:00Z", "2026-02-01T00:00:00Z")).toBe(
      "scheduled_period"
    );
  });

bases/rsptx/admin_server_api/routers/lti1p3.py:623

• Typo in comment: “Otherewise” → “Otherwise”.

        # Otherewise, it should be a valid ISO datetime string or empty string.

bases/rsptx/assignment_server_api/assignment_builder/src/components/routes/AssignmentBuilder/components/edit/visibilityMode.ts:15

• scheduled_period on the server side only applies when visible is false (see is_assignment_visible_to_students in components/rsptx/db/crud/assignment.py:65-76). Returning scheduled_period whenever both dates are set (even when visible is true) misrepresents the persisted state and can cause UI edits to flip visible to false via getVisibilityValues("scheduled_period", ...). Gate this mode on !visible so visible=true + hidden_on continues to map to scheduled_hidden.

  if (visibleOn && hiddenOn) {
    return "scheduled_period";
  }

[ascholerChemeketa]

Think we may need to talk visibility values.

What is your mental model for the visibility values in relation to visible/visible_on/hidden_on?

In my head (informed by Canvas published flag), if visible is off, any available dates do not matter.

An alternate formulation would be that visible is automatically changed to true if after visible_on but before hidden_on.

This block seems to be halfway between those two.

    case "hidden":
      return { visible: false, visible_on: null, hidden_on: null };
    case "visible":
      return { visible: true, visible_on: null, hidden_on: null };
    case "scheduled_visible":
      return { visible: false, visible_on: visibleOn, hidden_on: null };
    case "scheduled_hidden":
      return { visible: true, visible_on: null, hidden_on: hiddenOn };
    case "scheduled_period":
      return { visible: false, visible_on: visibleOn, hidden_on: hiddenOn };

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

sample.env:92

• The SERVER_PROTOCOL env var is still consumed by the legacy web2py app (e.g. applications/runestone/models/0.py reads it), but FastAPI cookie behavior now derives HTTPS from Settings.server_protocol. The updated comment no longer mentions web2py and could mislead operators into thinking this knob affects the FastAPI services directly.

# this is used to decide on how to set the session cookie settings.
# In production you will want to change this to https://
SERVER_PROTOCOL=http://

bases/rsptx/admin_server_api/routers/lti1p3.py:156

• parse_lti_datetime_as_utc()'s docstring says unresolved LTI substitution values indicate a null, but the function currently treats any non-ISO value as an exception (and the Canvas-specific “unresolved means null” handling happens in the caller). Updating the docstring to match the actual behavior will avoid future misuse of this helper.

    """
    Parse an LTI ISO datetime and return a naive UTC datetime for storage.
    Unresolved LTI substitution values indicate that the LMS has a null (but knows about the variable).
    An unresolved parameter string indicates that the LMS does not recognize the variable. That will become an exception.
    """