openid / AppAuth-JS

JavaScript client SDK for communicating with OAuth 2.0 and OpenID Connect providers.
Apache License 2.0
985 stars 161 forks source link

Token revocation request: Conflicting content-type value and data format #71

Closed malloy045 closed 6 years ago

malloy045 commented 6 years ago

Expected Behavior

The data format of the token revocation request should match the content-type header value used.

I have the code fixed, and tested it against an instance of IdentityServer. I can PR this at any time.

Describe the problem

The request is being sent with a content-type value of "application/x-www-form-urlencoded", but the data is being sent as json.

Code:

...
      headers: {'Content-Type': 'application/x-www-form-urlencoded'},
      data: request.toJson()
...

Steps to reproduce the behavior

Example:

private async revokeAccessToken() {
        this.tokenHandler = new BaseTokenRequestHandler(this.requestor);

        let request = new RevokeTokenRequest(this.tokenResponse.refreshToken, "refresh_token", ClientId);
        let response = await this.tokenHandler.performRevokeTokenRequest(this.configuration, request)
        this.AuthorizationCallback(EndSessionRedirectUri);
    }

Environment

tikurahul commented 6 years ago

Yes, that is a bug in https://github.com/openid/AppAuth-JS/blob/master/src/token_request_handler.ts#L61

To unblock yourself, please feel free to remove the headers dict from the request.

malloy045 commented 6 years ago

I can't just remove it. I'm pulling this library in as a dependency via yarn. To unblock myself, I extended the BaseTokenRequestHandler and properly formatted the data.

FWIW, I pulled down a copy of this repo and fixed this bug. I tried to PR it, but I'm blocked from pushing a new branch. I'm willing to PR it if I can get permission.... the files I changed are attached.

revoke_token_request.ts.txt token_request_handler.ts.txt

tikurahul commented 6 years ago

I meant using npm link, for setting up local overrides. Its pretty nifty.

Yes, send me a PR. Also, you will need to sign the CLA đź‘Ť

tikurahul commented 6 years ago

Fixed.