search

ministryofjustice / prison-api

API for Nomis DB used by DPS applications and other apis and services
https://api.prison.service.justice.gov.uk/swagger-ui.html
MIT License
7 stars 7 forks source link

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

Open boydingham opened 3 years ago

boydingham commented 3 years ago

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

boydingham commented 3 years ago

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 commented 3 years ago

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

boydingham commented 3 years ago

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 commented 3 years ago

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

I will proceed on the assumption that "_ITAGUSER" 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 commented 3 years ago

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

_I will proceed on the assumption that "_ITAGUSER" is one such <valid nomis user>_...

Dear "other future non-DPS devs",

I am pleased to report that "_ITAGUSER" 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 commented 3 years ago

...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 commented 3 years ago

... ...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 commented 3 years ago

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