Implement Token Introspection Endpoint

[rozagerardo]

Issue #52
Added support to the Token Introspection Endpoint as per the description in the ticket above.

[rozagerardo]

Hey @jgrandja I prepared this PR for the Token Introspection Endpoint.
I've made a few comments in the PR to discuss a few aspects, but first I want to point out I have quite a few formatting changes even though I'm sticking to the .editorconfig file we have in the root of the project. Should I revert all these changes to make the PR more readable?

[jgrandja]

Thanks for the PR @rozagerardo!

I'm a bit backlogged at the moment so I'll try to review this mid to end next week.

I did take a quick glance and noticed quite a bit of formatting changes. Can you please revert all formatting changes that are not directly related to the changes required for this PR. Thanks!

[rozagerardo]

Hey @jgrandja sure, that was in fact my question just above:
#161 (comment)
Sounds good, I'll fix that in the next few days and resolve all the conflicts that arise too, thanks for the heads up!

[jgrandja]

@rozagerardo Thanks for reverting the formatting changes.

I took a look at OAuth2TokenIntrospectionEndpointFilter and it doesn't follow the general pattern within Spring Security's Filter and AuthenticationManager / AuthenticationProvider architecture.

As a next step, I would encourage you to review the reference documentation to gain a deeper understanding:

• Servlet Security: The Big Picture
• Authentication

After you review that, please review the Token Revocation implementation in detail as it will follow the general pattern as Token Introspection.

This PR contains OAuth2TokenIntrospectionEndpointFilter but is missing OAuth2TokenIntrospectionAuthenticationToken and OAuth2TokenIntrospectionAuthenticationProvider.

Let me know if you have any questions.

[rozagerardo]

Hey @jgrandja sorry for the delay here, the last couple of weeks have been quite busy for me.

Also, thanks for the heads up with the last comment, naturally I had read these sections before on a couple of occasions, but it's certainly different from a "framework developer" perspective :) and yes, I now understand the approach we should follow.

All right, I now added the missing classes and adapted the code I had been working on. I have also added a couple of comments to the PR.

However, I see an error in the CI build (a 401 retrieving a dependency), but it's definitely not related to my changes:

• What went wrong:
  Could not determine the dependencies of task ':spring-security-oauth2-authorization-server-docs-manual:unzipDocumentationResources'.

Could not resolve all dependencies for configuration ':spring-security-oauth2-authorization-server-docs-manual:detachedConfiguration2'.
Could not resolve com.fasterxml.jackson:jackson-bom:2.+.
Required by:
project :spring-security-oauth2-authorization-server-docs-manual

The build is running ok locally, let me know if I can help with anything to fix this CI issue. I think it might be a temporary issue, but I can't trigger the CI plan manually.

Looking forward to the next comments Joe!

[rozagerardo]

Just a quick note about the last comment I made: #161 (comment)
The issue with the ci build seems to be solved, so this should be ready to be reviewed @jgrandja 👍

[rozagerardo]

Quick ping @jgrandja just to make sure you didn't miss my last comment:
#161 (comment)
Although I know you probably are just busy with something else 👍

[jgrandja]

@rozagerardo Apologies for the delay. I'm quite backlogged right now and trying to catch up with Spring Security issues and Spring Security OAuth (legacy) issues/PR's. I'm getting close to catching up and I'm planning on circling back to this PR this week. Then I will focus on getting it merged before end of month.

[jgrandja]

Thanks for the updates @rozagerardo. Please see review comments.

[rozagerardo]

Cool, this is now ready for a new round @jgrandja .
Regarding the comments you made, I now addressed them all, but please have a look at this discussion before marking it as resolved:
#161 (comment)
(I also made a small comment on some of the other points, but nothing important.)

Also, let me know if you want me to squash the commits again before reviewing them.

Looking forward to the next comments Joe 👍

[rozagerardo]

@jgrandja I now answered and marked this conversation as resolved:
#161 (comment)

I also added a test to validate that tokens can be introspected even if issued to a different Client, and updated the branch with the latest changes.

I think we're good to continue here, looking forward to the next comments :)

[jgrandja]

@rozagerardo Thanks for all your work ! This is now merged to master.

FYI, I applied some updates with a follow-up polish commit. Please review and let me know if you have any questions.