fix(whitelist): Recognise every two-factor provider app for guests - #1672
Open
solracsf wants to merge 1 commit into
Open
fix(whitelist): Recognise every two-factor provider app for guests#1672solracsf wants to merge 1 commit into
solracsf wants to merge 1 commit into
Conversation
CarlSchwan
reviewed
Aug 21, 2026
Comment on lines
+69
to
+81
| /** | ||
| * Two-factor authentication has to keep working for guests, otherwise an | ||
| * instance with enforced two-factor authentication locks them out. So an | ||
| * app that declares a two-factor provider is allowed even when the | ||
| * administrator did not whitelist it. | ||
| * | ||
| * ProviderLoader also picks up providers that apps register from their | ||
| * bootstrap, but that list is only reachable through private API, so those | ||
| * apps are named in WHITELIST_ALWAYS instead. | ||
| */ | ||
| private function isTwoFactorProviderApp(string $appId): bool { | ||
| // Also keeps unknown app ids away from the info.xml lookup below, | ||
| // which would resolve the app path on disk for every guess. |
Member
There was a problem hiding this comment.
Suggested change
| /** | |
| * Two-factor authentication has to keep working for guests, otherwise an | |
| * instance with enforced two-factor authentication locks them out. So an | |
| * app that declares a two-factor provider is allowed even when the | |
| * administrator did not whitelist it. | |
| * | |
| * ProviderLoader also picks up providers that apps register from their | |
| * bootstrap, but that list is only reachable through private API, so those | |
| * apps are named in WHITELIST_ALWAYS instead. | |
| */ | |
| private function isTwoFactorProviderApp(string $appId): bool { | |
| // Also keeps unknown app ids away from the info.xml lookup below, | |
| // which would resolve the app path on disk for every guess. | |
| private function isTwoFactorProviderApp(string $appId): bool { |
we don't need this gigantic comments
Member
Author
There was a problem hiding this comment.
Sorry, I've learned that good and extensive doc is as good as good code. I'll remove it. 馃槉
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
solracsf
force-pushed
the
fix/whitelist-all-twofactor-provider-apps
branch
from
August 21, 2026 19:21
0d7093d to
d4d6d66
Compare
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
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.
Fix #1628
WHITELIST_ALWAYShardcoded four twofactor apps, so a guest hitting any other provider got a 403 and could not finish the setup or the challenge. On an instance with enforced two-factor authentication that locks the guest out until an admin figures out that the provider app has to be whitelisted by hand.Instead of growing that list by hand, an app that declares two-factor-providers in its
info.xmlis now allowed at runtime. That covers twofactor_admin, twofactor_email, twofactor_oath, twofactor_u2f and twofactor_webeid, plus any provider app that ships later.Providers can also be registered from the app bootstrap, and that list is only reachable through private API, so those apps still have to be named. twofactor_gateway is added for that reason. The four that were already listed stay listed too: the runtime lookup only helps while info.xml is readable and while the app keeps declaring its provider there, and losing either would lock guests out again, which is the bug this fixes.
The lookup is gated on the app being enabled. That matches what
ProviderLoaderdoes, and it also keeps guessed app ids from resolving an app path on disk on every request.