Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct the principle username detection for OAuth 2.0 Client Credentials grant #946

Open
boydingham opened this issue Jun 11, 2021 · 8 comments

Comments

@boydingham
Copy link

Expected Behavior

When the /api/offenders/{offenderNo} endpoint is called, an OFFENDER_BOOKINGS record should be created in persistent storage.

Current Behavior

The system fails to create the OFFENDER_BOOKINGS record (full stack trace)...

...

org.springframework.dao.InvalidDataAccessApiUsageException: The given id must not be null!; nested exception is java.lang.IllegalArgumentException: The given id must not be null!

...

Caused by: java.lang.IllegalArgumentException: The given id must not be null!
        at org.springframework.util.Assert.notNull(Assert.java:201)
        at org.springframework.data.jpa.repository.support.SimpleJpaRepository.findById(SimpleJpaRepository.java:297)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:567)
        at org.springframework.data.repository.core.support.RepositoryMethodInvoker$RepositoryFragmentMethodInvoker.lambda$new$0(RepositoryMethodInvoker.java:289)
        at org.springframework.data.repository.core.support.RepositoryMethodInvoker.doInvoke(RepositoryMethodInvoker.java:137)
        at org.springframework.data.repository.core.support.RepositoryMethodInvoker.invoke(RepositoryMethodInvoker.java:121)
        at org.springframework.data.repository.core.support.RepositoryComposition$RepositoryFragments.invoke(RepositoryComposition.java:529)
        at org.springframework.data.repository.core.support.RepositoryComposition.invoke(RepositoryComposition.java:285)
        at org.springframework.data.repository.core.support.RepositoryFactorySupport$ImplementationMethodExecutionInterceptor.invoke(RepositoryFactorySupport.java:599)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
        at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.doInvoke(QueryExecutorMethodInterceptor.java<===========--> 87% EXECUTING [5h 31m 35s]
        at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.invoke(QueryExecutorMethodInterceptor.java:13
...

Context

I am an external developer of a non-DPS system. My application is specified to consume API endpoints exposed by this project. My application will be authorized by HMPPS Auth using the OAuth 2.0 Client Credentials grant.

While stepping through the debugger during investigation of a separate, unrelated issue, I had observed that Spring Security refers to the security principle as "AnonymousUser" during the OAuth 2.0 Client Credentials grant flow.

Assuming that an "AnonymousUser" would not have a user name, then the following two lines are reasonable suspects for the root cause of the reported "The given id must not be null" error...

  1. (username = ((UserDetails) userPrincipal).getUsername();)
  2. staffUserAccountRepository.findById(currentUsername)

Steps to Reproduce

  1. Call the /api/offenders/{offenderNo} endpoint with a JWT access token issued by HMPPS Auth for an OAuth 2.0 Client Credentials grant.

Your Environment

  • Windows 10
  • Docker Desktop Version 3.3.3 (64133)
  • IntelliJ IDEA 2021.1.2 (Community Edition)
    • Build #IC-211.7442.40
@boydingham
Copy link
Author

boydingham commented Jun 17, 2021

This one and these other two...

  1. Fix o.s.d.DataIntegrityViolationException in /api/offenders/
  2. Fix the build/config so that running locally on Windows 'Just Works'

...Will be what I would like to pick your brains on @petergphillips @andymarke, whenever we get a chance to talk off line.

@petergphillips
Copy link
Contributor

For endpoints that create data you need to provide an extra parameter when you create the client credentials of username=<valid nomis user>.

@boydingham
Copy link
Author

Thank you @petergphillips,

"...For endpoints that create data you need to provide an extra parameter when you create the client credentials of username=<valid nomis user>..."

I will give that a shot in a few.

I guess that's an example of what they call "Implicit Knowledge" (aka, "Tribal Knowledge"). Right? Because I haven't seen that documented anywhere. And I've read a ton of stuff so far about several different HMPPS/MOJ/DPS projects.

Please correct me if I'm wrong though. Please point me to the documentation of that, that I overlooked? TIA.

@boydingham
Copy link
Author

"...<valid nomis user>..."

I will proceed on the assumption that "ITAG_USER" is one such <valid nomis user>.

Or can you share any shortcuts to getting a hold of a known <valid nomis user> @petergphillips ? It might be valuable to share that here; on the off chance that other future non-DPS devs stumble across this same issue. TIA.

@boydingham
Copy link
Author

"...<valid nomis user>..."

I will proceed on the assumption that "ITAG_USER" is one such <valid nomis user>...

Dear "other future non-DPS devs",

I am pleased to report that "ITAG_USER" is, indeed, one such <valid nomis user> :)

I confirmed that, @petergphillips, with two curl calls from the command line...

  1. Appending username=<valid nomis user> to the token request to hmpps-auth
  2. Calling the /api/offenders/{offenderId}/booking endpoint with that access token

Now that leaves only the Spring Security OAuth 2.0 client credentials configuration so that it does those two things on my behalf, under the covers. Right @petergphillips?

If you could share any shortcuts for (or even better, links to documentation of) that configuration, that'd be so super deluxe! TIA.

@boydingham
Copy link
Author

...Now that leaves only the Spring Security OAuth 2.0 client credentials configuration so that it does those two things on my behalf, under the covers. Right @petergphillips?...

The Customizing the Access Token Request section of the Spring Security OAuth 2.0 reference documentation offers some pretty decent guidance.

I'm reasonably certain as far as the "What" needs to be customized goes. My first stab at the specifics (i.e., the "How") still needs a little more work though.

In the meantime, any links to hints, tips or suggestions are welcome @petergphillips. TIA.

@boydingham
Copy link
Author

...
...My first stab at the specifics (i.e., the "How") still needs a little more work though...

I've worked out a now-working configuration @petergphillips. I found the missing piece of the puzzle in the Spring Security JavaDoc...

...
Use OAuth2AuthorizedClientArgumentResolver(OAuth2AuthorizedClientManager) instead. Create an instance of ClientCredentialsOAuth2AuthorizedClientProvider configured with a DefaultClientCredentialsTokenResponseClient (or a custom one) and than supply it to DefaultOAuth2AuthorizedClientManager
...

The Spring Security OAuth 2.0 configuration I now have in place, now programmatically does the needful "...provide an extra parameter when you create the client credentials of username=<valid nomis user>" step.

Thanks again for sharing that username=<valid nomis user> clue 👍

@petergphillips
Copy link
Contributor

Sorry for the delay. An example of where we configure the web client to add the username can be found in WebClientConfiguration and the code for the converter is CustomOAuth2ClientCredentialsGrantRequestEntityConverter

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

No branches or pull requests

2 participants