Skip to content

Ability to serve LO Blocks via LO while signing in through Canvas - #246

Merged
bradley-erickson merged 2 commits into
masterfrom
berickson/20251215-lo-blocks-through-canvas
Dec 16, 2025
Merged

bradley-erickson merged 2 commits into
masterfrom
berickson/20251215-lo-blocks-through-canvas

Conversation

@bradley-erickson

Copy link
Copy Markdown
Collaborator

No description provided.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +248 to +252
'family_name': claims.get('family_name', ''),
'picture': claims.get('picture', ''),
'role': learning_observer.auth.ROLES.TEACHER if is_instructor else learning_observer.auth.ROLES.STUDENT,
'authorized': is_instructor,
# TODO determine how to determine authorized for both instructors and students
'authorized': True,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Do not mark every LTI launch as authorized

The LTI SSO flow now sets 'authorized': True for every user regardless of their roles, replacing the previous is_instructor check. Downstream guards such as auth.utils._role_required and the client-side authorized() checks gate access primarily on the authorized flag before role-specific logic, so this change treats all LTI students as fully authenticated and bypasses the unauthorized view even when no instructor role is present. Unless the intent is to grant teacher-level access to all LTI roles, authorized should remain tied to the role check or the role gate should be strengthened to avoid exposing protected dashboards to students.

Useful? React with 👍 / 👎.

@bradley-erickson

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@bradley-erickson
bradley-erickson merged commit c1eabcb into master Dec 16, 2025
1 of 2 checks passed
@bradley-erickson
bradley-erickson deleted the berickson/20251215-lo-blocks-through-canvas branch March 10, 2026 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant