Skip to content

Conversation

@bverhoeven
Copy link
Contributor

First of all: Thanks for your work! Getting the OpenID Connect client library to work was surprisingly easy.

As per https://tools.ietf.org/html/rfc7662#section-2.2 the sub key should identify the resource owner in oauth2 introspection responses:

sub
OPTIONAL. Subject of the token, as defined in JWT [RFC7519].
Usually a machine-readable identifier of the resource owner who
authorized this token.

I've found that this OpenID Connect client library checks for the (non-standard?) user_id key, which prevents responses from RFC-compliant servers to be parsed.

This pull request adds support for the sub key and will allow the introspection response of RFC-compliant servers to be parsed. Will still try user_id first as to not break backward compatibility.

As per https://tools.ietf.org/html/rfc7662#section-2.2 the `sub` key should
identify the resource owner in oauth2 introspection responses. 

This change adds support for the `sub` key and will allow the introspection 
response of RFC-compliant servers to be parsed.

Will still try `user_id` first as to not break backward compatibility.
@codecov-io
Copy link

Codecov Report

Merging #1320 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master   #1320      +/-   ##
===========================================
- Coverage      23.7%   23.7%   -0.01%     
  Complexity      849     849              
===========================================
  Files           209     209              
  Lines         11664   11666       +2     
  Branches       2116    2117       +1     
===========================================
  Hits           2765    2765              
- Misses         8422    8424       +2     
  Partials        477     477
Impacted Files Coverage Δ Complexity Δ
...introspectingfilter/IntrospectingTokenService.java 0% <0%> (ø) 0 <0> (ø) ⬇️

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 ce9bf35...85246d2. Read the comment docs.

@jricher
Copy link
Member

jricher commented Nov 20, 2017

That's really interesting, I thought for sure that had been changed in this code when the RFC was published (since I'm the editor for both, this is somewhat embarassing). Thanks for the catch, we'll get this rolled in!

@jricher jricher merged commit 9d6f42b into mitreid-connect:master May 3, 2018
@bverhoeven bverhoeven deleted the rfc7662-sub branch March 12, 2019 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants