Gradebook: Security: Harden access control on the certificate page - #8803
Merged
AngelFQC merged 1 commit intoJul 31, 2026
Merged
Conversation
Consolidate the authorization check in certificates/index.php so it is evaluated once, before the action dispatch, and therefore applies to every action instead of only the default one. Anonymous visitors must now pass the public-visibility check (allow_public_certificates, globally and per course) for any action. Authenticated users other than the certificate owner still require platform admin or course teacher rights, compared against the owner recorded in the database rather than the user_id request parameter. The check also runs before the Certificate object is instantiated, since its constructor regenerates the certificate as a side effect: it writes the HTML file, updates the gradebook row and assigns skills to the owner. To keep the visibility rules in a single place, extract the public-visibility check from Certificate::isVisible() into the static Certificate::isPubliclyVisible(), and add Certificate::getCertificateData() to read a certificate row without constructing the object. isVisible() keeps its signature and semantics and now delegates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidate the authorization check in certificates/index.php so it is
evaluated once, before the action dispatch, and therefore applies to every
action instead of only the default one.
Anonymous visitors must now pass the public-visibility check
(allow_public_certificates, globally and per course) for any action.
Authenticated users other than the certificate owner still require platform
admin or course teacher rights, compared against the owner recorded in the
database rather than the user_id request parameter.
The check also runs before the Certificate object is instantiated, since its
constructor regenerates the certificate as a side effect: it writes the HTML
file, updates the gradebook row and assigns skills to the owner.
To keep the visibility rules in a single place, extract the public-visibility
check from Certificate::isVisible() into the static
Certificate::isPubliclyVisible(), and add Certificate::getCertificateData() to
read a certificate row without constructing the object. isVisible() keeps its
signature and semantics and now delegates.
🤖 Generated with Claude Code