wso2 / wso2/wso2-cli

bearerClaims decodes exp as a plain int64: decide whether #131's rule applies inside a token

Open
#142 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

ready-for-human Severity/Minor Type/Question
Dominant language
Go
Stars
0
Forks
3
Avg merge
8h 43m
Merged PRs (30d)
60

Description

bearerClaims in internal/auth/claims.go decodes the JWT exp claim as a plain int64. That is the same shape as #131, where a single member of an unexpected type failed the whole decode and took a valid credential down with it.

Found by the sweep #131's acceptance criteria asked for — "whether any other member of a wire struct in internal/auth has the same shape, where a decorative field can fail a decode that carries a credential" — and deliberately left out of that fix's scope. Confirmed independently during review of #136.

Why it is filed separately, and why it is Minor

The consequence is materially softer than #131's, which is the whole reason it was not folded in.

A failed bearerClaims decode reaches verify in internal/auth/narrowing.go, which returns a typed denial naming the real cause. A maintainer reading it is pointed at the claim that could not be read. #131's failure mode was worse in kind: the decode error was swallowed and the caller saw errNoAccessToken, which describes a token that never arrived rather than one that arrived and was discarded — so the refusal actively misdirected whoever debugged it.

exp is also better behaved on the wire than expires_in was. RFC 7519 §2 defines it as a NumericDate, and it arrives inside a signed token rather than as a decoration on a token response, so the string-shaped variant that is common in the wild for expires_in is not the same hazard here.

What to decide

Whether to harden it at all, and if so how far.

The narrow reading is that this is fine as it stands: the failure is honest, it names the cause, and inventing tolerance for a malformed exp inside a token the shell is about to trust is not obviously an improvement. An unreadable expiry on a bearer token is arguably a token worth refusing.

The wider reading is that #131 established a rule — a member that only supplies an expiry must never fail a decode that carries a credential — and consistency is worth something. LifetimeSeconds already exists as the shared decoder for exactly this, and applying it here is a small change.

These pull in opposite directions and the difference matters, because exp participates in an access decision in a way expires_in does not. That is why this is a question rather than a task.

Acceptance criteria

  • A decision is recorded on whether an unreadable exp should refuse or narrow to unstated
  • If it narrows, exp decodes by the same rule as the two lifetime members, and a test covers a shape that is not a JSON number
  • If it refuses, the refusal names exp specifically rather than reporting a generic decode failure
  • Either way, the reasoning is recorded where the next person doing this sweep will find it

Refs #131.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read internal/auth/claims.go and internal/auth/narrowing.go, then compare the handling and acceptance criteria from #131 and the review context in #136. Decide whether unreadable exp should be tolerated or rejected, record the reasoning, and, if changing behavior, add the specified test and ensure the resulting refusal or narrowing names exp appropriately.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
authentication
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.