•  
      request #25691 [oidc-client] sub claim must be verified
    Infos
    #25691
    Manuel Vacelet (vaceletm)
    2022-05-02 11:01
    2022-03-07 10:23
    27224
    Details
    [oidc-client] sub claim must be verified

    Checking if the response has the status code 200 is not enough: sub claim must be verified, the JWT must not be outside its validity period, the nonce must be the same than the one sent, audience claim must be valid, iss claim must match the issuer identifier and JWT signature must be verified https://github.com/wikimedia/mediawiki-extensions-TuleapIntegration/blob/75ced91bb0e1ff1e099ed06a5b5567ad231a1a35/src/Provider/Tuleap.php#L74-L78 https://openid.net/specs/openid-connect-core-1_0.html#TokenResponseValidation

    Mediawiki Standalone
    development
    Empty
    • [ ] enhancement
    • [ ] internal improvement
    Robert Vogel (rvogel)
    Stage
    Dejan Savuljesku (dsavuljesku)
    Closed
    2022-05-02
    Attachments
    Empty
    References
    References list is empty

    Follow-ups

    User avatar
    Thomas Gerbet (tgerbet)2022-03-31 10:54

    Hello,

    I'm a bit confused with the way the keys used to validate the signature are parsed:

    		foreach ( $keys['keys'] as $keyData ) {
    			if ( $keyData['kty'] !== 'RSA' ) {
    				// Dont know how to handle other types
    				continue;
    			}
    			$modulus = sodium_base642bin( $keyData['n'], SODIUM_BASE64_VARIANT_URLSAFE_NO_PADDING );
    			$exponent = sodium_base642bin( $keyData['e'], SODIUM_BASE64_VARIANT_URLSAFE_NO_PADDING );
    			$key = PublicKeyLoader::loadPublicKey( [
    				'e' => new BigInteger( $exponent, 256 ),
    				'n' => new BigInteger( $modulus, 256 ),
    			] );
    
    			$res[$keyData['kid']] = new Key( $key->toString( 'pkcs8' ), $keyData['alg'] );
    		}
    

    https://github.com/wikimedia/mediawiki-extensions-TuleapIntegration/blob/3002953f2cc0cf47a58ec759a0bf1f835c4ea5aa/src/Provider/Tuleap.php#L168-L181

    I'm a bit concerned with using phpseclib3 when we could use the openssl_ primitives to avoid relying on a userland RSA implementation. I took a look at the Firebase JWT package and there is a JWK::parseKeySet method that we could use to achieve that.

    User avatar
    Thomas Gerbet (tgerbet)2022-03-08 14:44

    Yep, it is only supposed to be present since it is the unique ID of the user you authenticate.

    User avatar

    All done except verifying sub claim. Not sure how to do that. As far as i see, sub is the resource owner ID, but we dont get then until after the AT has been verified, right?

    User avatar
    • Original Submission
      Something went wrong, the follow up content couldn't be loaded
      Only formatting have been changed, you should switch to markup to see the changes