Add issuer, audience and azp checks in bearer token validator

[julien-nc]

The bearer token validation is less complete than the login controller one.

• add issuer, audience and azp (authorized party) checks in bearer token validator
• make it possible to disable bearer token audience check via config.php.

closes #856

[julien-nc]

@wielinde Could you say again what was your concern about the azp check?

The current check (when validating the token for API calls) is: If there are multiple audiences in the token (the aud attribute of the token is an array), we want the azp token attribute (authorized party) to be equal to the Oidc client ID.

This was implemented following points 4 and 5 of https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation

Do you think this check only makes sense to authenticate in NC but not when validating a token used to make API calls?

[wielinde]

@julien-nc I understand the specification that you have linked above in the following manner:

Only when there is an azp is given, then it shall be taken into account. However, that it is present in a token is not the default. In a vanilla OIDC setup, the azp shall not be added to a token, unless required by a OIDC extension.

And I am not aware of any such OIDC extension. So from my understanding, I would remove the requirement that the an azp must be given whenever an array of aud is present. And when azp is given, then it needs to be in the list auf aud AND be the Nextcloud client. Does that make sense?

// FYI @ba1ash

[julien-nc]

@wielinde I can't remember where I found a different description of how the azp check should be done. Anyway let's stick to this one. For both login token and bearer token, the check is now: (Independently of the aud claim) If the token has the azp claim, it should be equal to the Oidc client ID. If audience check is disabled for bearer token validation, the azp is not checked even if it's present.

All good?

[wielinde]

@julien-nc From my understanding of the spec, that looks good, now. Thank you.

[col-panic]

I'm using user_oidc to protect my API. Now since this patch, I cannot access Nextcloud using API anymore, the output is This token is not for us, authorized party (azp) is different than the client ID

I found the setting selfencoded_bearer_validation_audience_check but it seems to catch only when coming in via https://github.com/nextcloud/user_oidc/blob/f5bb30caef3a31ba19fe3f44da256b46cac90e9f/lib/User/Validator/SelfEncodedValidator.php#L90 not when coming in via https://github.com/nextcloud/user_oidc/blob/f5bb30caef3a31ba19fe3f44da256b46cac90e9f/lib/Controller/LoginController.php#L506

Check for example https://github.com/search?q=repo%3Anextcloud%2Fuser_oidc+%22This+token+is+not+for+us%2C+authorized+party+%28azp%29+is+different+than+the+client+ID%22&type=code

Could you please verify, that this option does also disable azp and aud checkin when used via API?

Should I create a new issue for this?