feat: Expose sap_id_type claim via SapIdToken#getIdType#1983
feat: Expose sap_id_type claim via SapIdToken#getIdType#1983NiklasHerrmann21 wants to merge 1 commit into
Conversation
SAP Cloud Identity Service issues the sap_id_type claim to disambiguate
human end-users ("user") from technical/application principals ("app").
Until now, code had to rely on the sub == azp heuristic to detect technical
users, which is brittle.
Add a typed SapIdType enum (USER/APP) and a SapIdToken#getIdType() accessor
that resolves the claim, returning null for absent or unknown values so
callers can stay forward-compatible with future principal types.
DefaultIdTokenExtension#isTechnicalUser now prefers the claim and falls
back to the sub == azp comparison for tokens issued before the claim was
introduced.
finkmanAtSap
left a comment
finkmanAtSap
left a comment
There was a problem hiding this comment.
I have some doubts about the strongly types enum approach. Let's discuss whether you see it the same way as me or not.
| public SapIdType getIdType() { | ||
| return SapIdType.fromClaimValue(getClaimAsString(TokenClaims.SAP_ID_TYPE)); | ||
| } |
There was a problem hiding this comment.
The enum return type is the cleanest approach but it has the problem that it is not forward-compatible if new claim values are added, e.g. "agent" perhaps. You will need to do new releases and consumers will need to update before being able to read the claim correctly. With the old version, it will look like the claim is missing which might be misleading.
I would go for a String return value and export String constants for the currently known values, i.e. APP and USER that consumers can compare with today. It's less nice than comparing with an enum but it's honest about the fact that the claim could have more than these two values and allows programming already against that.
This is important when high-lever modules in the library stack like CAP use this feature: they cannot simply set a minimum version of the BTP sec libs for applications and begin to use newer enum values. Instead, they need to work also when older versions of the library are installed and where no enum value exists for new claim values.
| * @param token the token to inspect | ||
| * @return {@code true} if the token belongs to a technical user | ||
| */ | ||
| private boolean isTechnicalUser(Token token) { |
There was a problem hiding this comment.
Not sure how it is implemented in Node.js (probably like this) but I think it would be best to be conservative and demand "user" before attempting a token exchange. Theoretically, in this case, there could be new types introduced later for which the extension should also do the exchange but it does not do it because it requires "user". However, this is better than the alternative where the extension checks for "app" and attempts an exchange in all other cases, even when it should not (e.g. when new non-named-user values like "agent" are introduced).
2 participants
Summary
SapIdToken#getIdType()returning a typedSapIdTypeenum (USER/APP) backed by thesap_id_typeJWT claim issued by SAP Cloud Identity Service.TokenClaims.SAP_ID_TYPEconstant.DefaultIdTokenExtension#isTechnicalUsernow prefers the claim and falls back to thesub == azpheuristic for tokens issued before the claim was introduced — no behavior change for pre-sap_id_typebindings, but new tokens drive the right decision when the heuristic would have been wrong.Why
The Node library exposes the same claim (
token.idType) and uses it to replace thesub === azpheuristic at decision points likeDefaultIdTokenExtension._getIdTokenandXsuaaLegacyExtension.appliesTo. This brings the Java library in line.Design notes
SapIdTypelives injava-apinext toTokenClaimsso it can be referenced from bothjava-apiandjava-security.nullfor both an absent claim and an unknown value (e.g. a future third principal type). This keeps consumers forward-compatible without throwing on tokens issued by newer IAS versions.isTechnicalUserkeeps the legacysub == azppath as a fallback to avoid breaking technical-user detection for tokens that don't yet carrysap_id_type.Test plan
mvn -pl java-api,java-security -am test— 330 tests green, including new coverage:SapIdTokenTest:getIdTypeforuser,app, missing claim, unknown valueDefaultIdTokenExtensionTest:sap_id_type=appis treated as technical user;sap_id_type=useroverrides thesub == azpheuristicmvn install -DskipTests(full repo compile + javadoc) — greenRelease
CHANGELOG entry added under
4.1.0(minor bump for additive feature). The pom version bump will follow in a separatechore: Release 4.1.0commit once any in-flight Renovate PRs are merged.