Repository navigation
Conversation
A before hook returning {:halt, reason} where reason is not an {:error, _}
tuple was wrapped as {:ok, reason} by Joken.Hooks.run_before_hook/3.
Joken.verify/3 (and validate/4, generate_claims/3, encode_and_sign/3) then
returned that {:ok, _} value verbatim without running signature verification
or claim validation, because the `with` had no else branch. Callers matching
on {:ok, _} would treat a hook "deny" as a verified token.
Tag the reduce_while accumulator so run_before_hook/3 only ever returns
{:ok, input} or {:error, reason}; a halt with a non-error payload is now
{:error, :wrong_hook_return}, consistent with the documented contract and
with how other wrong return shapes are already handled.
run_after_hook/4 had the inverse problem: a halt with {:error, tuple} was
destructured by the untagged `{result, input} when is_tuple(input)` clause
and surfaced as the bare atom :error. The accumulator is now explicitly
tagged and the halt payload is returned verbatim.
Tighten the before hook callback specs to {:halt, {:error, term}}, document
the behaviour and add regression tests.
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.
A before hook that halts with anything other than an
{:error, _}tuple currently makes the operation succeed.run_before_hook/3returns the raw halt payload, then wraps everything that isn't{:error, _}as{:ok, payload}.Joken.verify/3matches{:ok, {token, signer}}in awithwithoutelse, so{:ok, :kid_not_found}is returned as-is andSigner.verify/2never runs:Code that checks
{:ok, _}fromverify/2(channel joins, socket connects, anywhere the claims aren't needed) treats that as a verified token.@type halt_tuple :: {:halt, tuple}allows the shape, so this isn't a misuse.validate/4,generate_claims/3andencode_and_sign/3have the samewith.run_after_hook/4has the inverse problem:{:halt, {:error, {:missing, "aud"}}}is destructured by thewhen is_tuple(input)clause and comes out as the bare atom:error.The accumulators are now tagged. A before hook halt that isn't
{:error, _}becomes{:error, :wrong_hook_return}, same as the other wrong return shapes; an after hook halt is returned verbatim. Before hook specs tightened to{:halt, {:error, term}}, documented in the moduledoc, CHANGELOG entry, three regression tests.mix test,mix format --check-formatted,mix credo --strict,mix dialyzerall clean.Prepared with AI assistance (Claude); I reproduced the bypass and verified the tests fail on main.