simplesamlphp / simplesamlphp-module-oidc

A SimpleSAMLphp module for OIDC OP support.
Other
45 stars 22 forks source link

Logout tokens should have typ header with value 'logout+jwt' #185

Closed IlanaRadinsky closed 2 years ago

IlanaRadinsky commented 2 years ago

OpenID backchannel logout spec recommends that logout tokens be explicitly typed:

It is RECOMMENDED that Logout Tokens be explicitly typed. This is accomplished by including a typ (type) Header Parameter with a value of logout+jwt in the Logout Token. See Section 4.1 for a discussion of the security and interoperability considerations of using explicit typing.

(source)

The nimbus library requires that typed logout tokens have type logout+jwt: https://bitbucket.org/connect2id/oauth-2.0-sdk-with-openid-connect-extensions/src/ef594b7477dd46a4e64c61ee6b6e335e38b4b29f/src/main/java/com/nimbusds/openid/connect/sdk/validators/LogoutTokenValidator.java#lines-340

By default the tokens are typed with type JWT: https://github.com/lcobucci/jwt/blob/058b61b376339208622b6dc9d38b70a122338c20/src/Token/Builder.php#L21

IlanaRadinsky commented 2 years ago

Hi @pradtke - Looks like you were the one to review the logout contribution in https://github.com/simplesamlphp/simplesamlphp-module-oidc/pull/149. Would you be able to review this?

codecov[bot] commented 2 years ago

Codecov Report

Merging #185 (9a733d9) into master (38a7dd8) will increase coverage by 0.02%. The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #185      +/-   ##
============================================
+ Coverage     38.81%   38.83%   +0.02%     
  Complexity      855      855              
============================================
  Files           105      105              
  Lines          2950     2951       +1     
============================================
+ Hits           1145     1146       +1     
  Misses         1805     1805              
Impacted Files Coverage Δ
lib/Services/LogoutTokenBuilder.php 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 38a7dd8...9a733d9. Read the comment docs.

cicnavi commented 2 years ago

@IlanaRadinsky thank you for your valuable contribution! I've updated the test to include typ header check and also updated the composer.json to allow composer-installer plugin so all the checks can run. I'll leave this for review for @pradtke as you suggested :).

@pradtke can you please check why psalm did not run in Quality Control job:

ERROR: MissingFile - vendor/bin/psalm:115:9 - Cannot find file phpvfscomposer:/home/runner/work/simplesamlphp-module-oidc/simplesamlphp-module-oidc/vendor/vimeo/psalm/psalm to include (see https://psalm.dev/107)
include("phpvfscomposer://" . __DIR__ . '/..'.'/vimeo/psalm/psalm');
IlanaRadinsky commented 2 years ago

Thank you @pradtke!

When do you expect the next release to be cut?