IdentityModel / oidc-client-js

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications
Apache License 2.0
2.43k stars 841 forks source link

revokeAccessTokenOnSignout : true, does not revoke access token if access_token has . #1228

Closed jsandeepm closed 3 years ago

jsandeepm commented 4 years ago

If access_token has . then revocation of access token will return resolve with false. If access_token without . causes to revocation of access token(_revocationendpoint invokes).

Examples of access token from server var access_token1= '00D1233434343YA!AQUAQJ0uS5G8jOHFGBeDjxV72YT1CkvfwdX6_3Raxm22j80q57sXyznPWy9UhXrYkWh80Oz';

var access_token2= '00D1233434343YA!AQUAQFVqFG8PGqdT.6Wa.6s6kmqiuN9O_wBdD9qaeipMB7m.t1BO4kXlUF';

Why are we checking for . in access_token?

` UserManager.prototype._revokeAccessTokenInternal = function _revokeAccessTokenInternal(access_token, required) {

    // check for JWT vs. reference token
    if (!access_token || access_token.indexOf('.') >= 0) {
        return Promise.resolve(false);
    }
    return this._tokenRevocationClient.revoke(access_token, required).then(function () {
        return true;
    });
};

` What if we want to revocation_endpoint to revoke access_token irrespective of having .

brockallen commented 3 years ago

If it's a JWT then there's not point in calling revocation.

Gerrit-K commented 3 years ago

@brockallen Pardon me for jumping on this closed issue, but I was about to file a similar report. I believe the check for JWT (i.e. looking for a ., see below) is too naïve as there are IdP implementations which use access tokens in the JWT format (at least that's the case for us with OpenAM in our company).

https://github.com/IdentityModel/oidc-client-js/blob/c150cd26dd29a09fedb4faaa95f00c16ddd90906/src/UserManager.js#L587-L588

We query our access tokens along with the id_token and they appear as JWTs, but without a readable JSON payload. Apart from that, they completely behave like access tokens (e.g. we use them to call the userinfo endpoint and they indeed are revocable).

brockallen commented 3 years ago

Sure, it would make sense to have a config flag for that behavior.

Gerrit-K commented 3 years ago

@brockallen a config flag could be a solution, but I think the default check whether the access token is a JWT shouldn't be just token.indexOf('.') >= 0. Is there any other way to check the type of the token (e.g. some setting in the user object)? Or is this really the only way? (I also don't rule out the possibility that our IdP is misconfigured and shouldn't even respond with pseudo-JWTs in the first place ... but I'm not familiar enough with the Oauth/OIDC spec to say whether or not it is compliant to do that). And how should we proceed with this? Should I raise a separate issue to stop hijacking this one or can this one be reopened?

brockallen commented 3 years ago

Access tokens are opaque to the client, so not really. Most other token servers I've encountered don't use dots in their opaque tokens, so this approach seemed like it was a reasonable, albeit simple approach.

obriematt commented 3 years ago

Sorry to jump into a closed issue, I'm curious if a configuration flag was added for this behavior?

@brockallen - would it be appropriate to open a new issue for this?