spring-projects / spring-projects/spring-security

Consider making `ClaimAccessor#getClaimXXX` methods behave consistently if claim value is `null`

Open
#10,383 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

in: oauth2 type: enhancement
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

This ticket is created based on this comment.

Expected Behavior
ClaimAccessor#getClaim() and ClaimAccessor#getClaimAsString() should behave the same way as other ClaimAccessor#getClaimAsXXX methods.

There are few options to consider:

  • all ClaimAccessor#getClaimAsXXX methods will throw NullPointerException if claim value is null and claim value is converted to null. I suppose, this is a preferred way.
  • all ClaimAccessor#getClaimAsXXX methods will throw IllegalArgumentException without claim value class, if claim value is null and claim value is converted to null (no NullPointerException)
  • null will be valid return value even if converter converts claim value to null (no assertion). This was discussed here
  • ...

Current Behavior
All ClaimAccessor#getClaimAsXXX methods (except ClaimAccessor#getClaim() and ClaimAccessor#getClaimAsString()) throw NullPointerException if claim value is null and claim value is converted to null.

Exception is thrown when message for IllegalArgumentException is evaluated:

Assert.isTrue(convertedValue != null,
				() -> "Unable to convert claim '" + claim + "' of type '" + claimValue.getClass() + "' to XXX.");

On the other side,

  • ClaimAccessor#getClaim() returns null if claim value is null.
  • ClaimAccessor#getClaimAsString() returns null if claim value is converted to null.

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

Start by reading the ClaimAccessor#getClaim(), getClaimAsString(), and other getClaimAsXXX methods to compare their null handling. Review the linked pull-request comment and issue discussion before choosing a consistent behavior. Done means the selected behavior is applied consistently across these accessors and verified for null claim values.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring
Domain
security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.