Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is throwing expected claim "${claim}" in "${sourceName}" the correct behaviour on missing source? #197

Closed
seybsen opened this issue Oct 15, 2019 · 4 comments

Comments

@seybsen
Copy link

seybsen commented Oct 15, 2019

An error expected claim "${claim}" in "${sourceName}" is thrown if distributed sources do not return all claims referenced in _claim_names:
https://github.com/panva/node-openid-client/blob/master/lib/client.js#L55

Example:

{
	"sub": "dxu....",
	"_claim_names": {
		"address": "55eb6148-9ddf-4f2d-98a6-30cbae6ebbab",
		"gender": "55eb6148-9ddf-4f2d-98a6-30cbae6ebbab",
		"phone_number": "55eb6148-9ddf-4f2d-98a6-30cbae6ebbab",
		"given_name": "55eb6148-9ddf-4f2d-98a6-30cbae6ebbab",
		"family_name": "55eb6148-9ddf-4f2d-98a6-30cbae6ebbab",
		"email": "55eb6148-9ddf-4f2d-98a6-30cbae6ebbab"
	},
	"_claim_sources": {
		"55eb6148-9ddf-4f2d-98a6-30cbae6ebbab": {
			"access_token": "eyJraW...",
			"endpoint": "https://api-identity.example.com/userinfo"
		}
	}
}

when fetching the distributed claims gender is missing:

{
	"aud": "d7466ksfdzdxq",
	"sub": "dxu....",
	"id4me.identifier": "test.example.com",
	"id4me.identity": "test.example.com",
	"nbf": 1571127548,
	"address": {
		"street_address": "",
		"country": "",
		"formatted": "",
		"locality": "",
		"region": "",
		"postal_code": ""
	},
	"iss": "https://api-identity.example.com",
	"exp": 1571127578,
	"given_name": "Tester",
	"iat": 1571127548,
	"family_name": "Test",
	"email": "[email protected]"
}

which then throws RPError: expected claim "gender" in "55eb6148-9ddf-4f2d-98a6-30cbae6ebbab"

Is this the correct behaviour according to RFC?

@seybsen
Copy link
Author

seybsen commented Oct 15, 2019

https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.5.6.2

endpoint
REQUIRED. OAuth 2.0 resource endpoint from which the associated Claim can be retrieved. The endpoint URL MUST return the Claim as a JWT.

... does this mean the endpoint MUST return the Claim or that the Claim MUST be JWT?

@panva
Copy link
Owner

panva commented Oct 15, 2019

Hi Sebastian,

https://openid.net/specs/openid-connect-core-1_0.html#AggregatedDistributedClaims

The JWT returned by the resource (endpoint) is the same JWT as for the aggregated claims, for which the following is defined

JWT that MUST contain all the Claims in the _claim_names object that references the corresponding _claim_sources member.

Its processing is the same, ergo throw if the _claim_names point to something that's not there.

Feel free to double check on the OIDC WG mailing list or issue tracker, i might be in the wrong to apply the same processing for distributed as for aggregated.

@panva panva closed this as completed Oct 15, 2019
@pawel-kow
Copy link

OMHO throwing an exception is a bit hard, as more graceful handling would be possible.

In case of distributed claims IdP may not know what claims are stored by the claims source, therefore there is no such strict language as with Aggregated Claims.
Anyway I've opened an issue at OIDF: https://bitbucket.org/openid/connect/issues/1117/core-562-behavior-for-distributed-claims

@panva panva reopened this Oct 24, 2019
@panva
Copy link
Owner

panva commented Oct 24, 2019

Reopening based on WG feedback to the issue. Expect a fix in the next release.

@panva panva closed this as completed in 48d6633 Oct 24, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants