-
Notifications
You must be signed in to change notification settings - Fork 2
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
SDIT-1336 Add in Spring Security by including the library hmpps-kotlin-spring-boot-starter #196
base: main
Are you sure you want to change the base?
Changes from 1 commit
9fc5b27
be3e39a
4eb7fb7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,16 @@ | ||||||
package uk.gov.justice.digital.hmpps.hmppstemplatepackagename | ||||||
|
||||||
import org.springframework.security.access.prepost.PreAuthorize | ||||||
import org.springframework.web.bind.annotation.GetMapping | ||||||
import org.springframework.web.bind.annotation.RequestMapping | ||||||
import org.springframework.web.bind.annotation.RestController | ||||||
import java.time.LocalDateTime | ||||||
|
||||||
@RestController | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wondering if you could do something like only enabling for local / dev/ test based on a bean property so it doesn't get deployed to other people's instances if they don't remove it? |
||||||
@RequestMapping("/time") | ||||||
class TimeResource { | ||||||
|
||||||
@PreAuthorize("hasRole('TEMPLATE_EXAMPLE')") | ||||||
@GetMapping | ||||||
fun getTime() = "${LocalDateTime.now()}" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the secure endpoint we were asked to add by @rj-adams. The plan is to call this from the Typescript template so it has an example of calling a secured endpoint. I like it because it's simple and there's no harm if somebody forgets to remove it - it doesn't poolute the API model. On the other hand we're going to have to add some kind of model if when we get onto the OpenAPI docs ticket as an example of an OpenAPI specification. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,12 +7,20 @@ import org.springframework.http.HttpStatus.BAD_REQUEST | |
import org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR | ||
import org.springframework.http.HttpStatus.NOT_FOUND | ||
import org.springframework.http.ResponseEntity | ||
import org.springframework.security.access.AccessDeniedException | ||
import org.springframework.web.bind.annotation.ExceptionHandler | ||
import org.springframework.web.bind.annotation.RestControllerAdvice | ||
import org.springframework.web.servlet.resource.NoResourceFoundException | ||
import uk.gov.justice.hmpps.kotlin.common.ErrorResponse | ||
|
||
@RestControllerAdvice | ||
class HmppsTemplateKotlinExceptionHandler { | ||
Comment on lines
16
to
17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a prime candidate for moving to the library |
||
@ExceptionHandler(AccessDeniedException::class) | ||
fun handleAccessDeniedException(e: AccessDeniedException): ResponseEntity<ErrorResponse> = ResponseEntity | ||
.status(HttpStatus.FORBIDDEN) | ||
.body(ErrorResponse(status = HttpStatus.FORBIDDEN.value())) | ||
.also { log.debug("Forbidden (403) returned", e) } | ||
|
||
@ExceptionHandler(ValidationException::class) | ||
fun handleValidationException(e: ValidationException): ResponseEntity<ErrorResponse> = ResponseEntity | ||
.status(BAD_REQUEST) | ||
|
@@ -50,20 +58,3 @@ class HmppsTemplateKotlinExceptionHandler { | |
private val log = LoggerFactory.getLogger(this::class.java) | ||
} | ||
} | ||
|
||
data class ErrorResponse( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now comes from the library |
||
val status: Int, | ||
val errorCode: Int? = null, | ||
val userMessage: String? = null, | ||
val developerMessage: String? = null, | ||
val moreInfo: String? = null, | ||
) { | ||
constructor( | ||
status: HttpStatus, | ||
errorCode: Int? = null, | ||
userMessage: String? = null, | ||
developerMessage: String? = null, | ||
moreInfo: String? = null, | ||
) : | ||
this(status.value(), errorCode, userMessage, developerMessage, moreInfo) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
package uk.gov.justice.digital.hmpps.hmppstemplatepackagename.config | ||
|
||
import org.springframework.beans.factory.annotation.Value | ||
import org.springframework.context.annotation.Bean | ||
import org.springframework.context.annotation.Configuration | ||
import org.springframework.http.client.reactive.ReactorClientHttpConnector | ||
import org.springframework.security.oauth2.client.AuthorizedClientServiceOAuth2AuthorizedClientManager | ||
import org.springframework.security.oauth2.client.OAuth2AuthorizedClientManager | ||
import org.springframework.security.oauth2.client.OAuth2AuthorizedClientProviderBuilder | ||
import org.springframework.security.oauth2.client.OAuth2AuthorizedClientService | ||
import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository | ||
import org.springframework.security.oauth2.client.web.reactive.function.client.ServletOAuth2AuthorizedClientExchangeFilterFunction | ||
import org.springframework.web.reactive.function.client.WebClient | ||
import reactor.netty.http.client.HttpClient | ||
import java.time.Duration | ||
import kotlin.apply as kotlinApply | ||
|
||
@Configuration | ||
class WebClientConfiguration( | ||
@Value("\${api.base.url.oauth}") val oauthApiBaseUri: String, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could this be |
||
@Value("\${api.health-timeout:2s}") val healthTimeout: Duration, | ||
@Value("\${api.timeout:90s}") val timeout: Duration, | ||
) { | ||
@Bean | ||
fun oauthApiHealthWebClient(builder: WebClient.Builder): WebClient = builder.healthWebClient(oauthApiBaseUri, healthTimeout) | ||
|
||
/** | ||
* TODO | ||
* Once you have a client registration defined in properties `spring.security.client.registration` then you'll | ||
* need to uncomment this @Bean and create both a health and authorized web client. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've got an idea around generating all web clients in the library |
||
* | ||
* e.g. if your client registration config looks like this (registrationId is `prison-api`): | ||
* ``` | ||
* spring: | ||
* security: | ||
* client: | ||
* registration: | ||
* prison-api: | ||
* provider: hmpps-auth | ||
* client-id: ${prison-api.client.id} | ||
* client-secret: ${prison-api.client.secret} | ||
* authorization-grant-type: client_credentials | ||
* scope: read | ||
* ``` | ||
* Then you need to create web clients in this class as follows: | ||
* ``` | ||
* @Bean | ||
* fun prisonApiHealthWebClient(builder: WebClient.Builder): WebClient = builder.healthWebClient(prisonApiBaseUri, healthTimeout) | ||
* | ||
* @Bean | ||
* fun prisonApiWebClient(authorizedClientManager: OAuth2AuthorizedClientManager, builder: WebClient.Builder): WebClient = | ||
* builder.authorisedWebClient(authorizedClientManager, registrationId = "prison-api", url = prisonApiBaseUri, timeout) | ||
* ``` | ||
*/ | ||
// @Bean | ||
fun authorizedClientManager( | ||
clientRegistrationRepository: ClientRegistrationRepository, | ||
oAuth2AuthorizedClientService: OAuth2AuthorizedClientService, | ||
): OAuth2AuthorizedClientManager { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you don't have any client registrations then Spring doesn't create a ClientRegistrationRepository, hence the |
||
val authorizedClientProvider = OAuth2AuthorizedClientProviderBuilder.builder().clientCredentials().build() | ||
return AuthorizedClientServiceOAuth2AuthorizedClientManager( | ||
clientRegistrationRepository, | ||
oAuth2AuthorizedClientService, | ||
).kotlinApply { setAuthorizedClientProvider(authorizedClientProvider) } | ||
} | ||
} | ||
|
||
fun WebClient.Builder.authorisedWebClient(authorizedClientManager: OAuth2AuthorizedClientManager, registrationId: String, url: String, timeout: Duration): WebClient { | ||
val oauth2Client = ServletOAuth2AuthorizedClientExchangeFilterFunction(authorizedClientManager).kotlinApply { | ||
setDefaultClientRegistrationId(registrationId) | ||
} | ||
|
||
return baseUrl(url) | ||
.clientConnector(ReactorClientHttpConnector(HttpClient.create().responseTimeout(timeout))) | ||
.filter(oauth2Client) | ||
.build() | ||
} | ||
|
||
fun WebClient.Builder.healthWebClient(url: String, healthTimeout: Duration): WebClient = | ||
baseUrl(url) | ||
.clientConnector(ReactorClientHttpConnector(HttpClient.create().responseTimeout(healthTimeout))) | ||
.build() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package uk.gov.justice.digital.hmpps.hmppstemplatepackagename.health | ||
|
||
import org.springframework.beans.factory.annotation.Qualifier | ||
import org.springframework.boot.actuate.health.Health | ||
import org.springframework.boot.actuate.health.ReactiveHealthIndicator | ||
import org.springframework.stereotype.Component | ||
import org.springframework.web.reactive.function.client.WebClient | ||
import org.springframework.web.reactive.function.client.WebClientResponseException | ||
import reactor.core.publisher.Mono | ||
|
||
abstract class HealthCheck(private val webClient: WebClient) : ReactiveHealthIndicator { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be extending the |
||
|
||
override fun health(): Mono<Health> = webClient.get() | ||
.uri("/health/ping") | ||
.retrieve() | ||
.toEntity(String::class.java) | ||
.flatMap { Mono.just(Health.up().withDetail("HttpStatus", it?.statusCode).build()) } | ||
.onErrorResume(WebClientResponseException::class.java) { | ||
Mono.just( | ||
Health.down(it).withDetail("body", it.responseBodyAsString).withDetail("HttpStatus", it.statusCode).build(), | ||
) | ||
} | ||
.onErrorResume(Exception::class.java) { Mono.just(Health.down(it).build()) } | ||
} | ||
|
||
@Component("hmppsAuthApi") | ||
class OAuthApiHealth(@Qualifier("oauthApiHealthWebClient") webClient: WebClient) : HealthCheck(webClient) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
package uk.gov.justice.digital.hmpps.hmppstemplatepackagename.helper | ||
|
||
import io.jsonwebtoken.Jwts | ||
import org.springframework.context.annotation.Bean | ||
import org.springframework.context.annotation.Primary | ||
import org.springframework.http.HttpHeaders | ||
import org.springframework.security.oauth2.jwt.JwtDecoder | ||
import org.springframework.security.oauth2.jwt.NimbusJwtDecoder | ||
import org.springframework.stereotype.Component | ||
import java.security.KeyPair | ||
import java.security.KeyPairGenerator | ||
import java.security.interfaces.RSAPublicKey | ||
import java.time.Duration | ||
import java.util.Date | ||
import java.util.UUID | ||
|
||
@Component | ||
class JwtAuthHelper { | ||
private val keyPair: KeyPair = KeyPairGenerator.getInstance("RSA").apply { initialize(2048) }.generateKeyPair() | ||
|
||
@Bean | ||
@Primary | ||
fun jwtDecoder(): JwtDecoder = NimbusJwtDecoder.withPublicKey(keyPair.public as RSAPublicKey).build() | ||
|
||
fun setAuthorisation( | ||
user: String = "AUTH_ADM", | ||
roles: List<String> = listOf(), | ||
scopes: List<String> = listOf(), | ||
): (HttpHeaders) -> Unit { | ||
val token = createJwt( | ||
subject = user, | ||
scope = scopes, | ||
expiryTime = Duration.ofHours(1L), | ||
roles = roles, | ||
) | ||
return { it.set(HttpHeaders.AUTHORIZATION, "Bearer $token") } | ||
} | ||
|
||
internal fun createJwt( | ||
subject: String?, | ||
scope: List<String>? = listOf(), | ||
roles: List<String>? = listOf(), | ||
expiryTime: Duration = Duration.ofHours(1), | ||
jwtId: String = UUID.randomUUID().toString(), | ||
): String = | ||
mutableMapOf<String, Any>() | ||
.also { subject?.let { subject -> it["user_name"] = subject } } | ||
.also { it["client_id"] = "test-app" } | ||
.also { roles?.let { roles -> it["authorities"] = roles } } | ||
.also { scope?.let { scope -> it["scope"] = scope } } | ||
.let { | ||
Jwts.builder() | ||
.id(jwtId) | ||
.subject(subject) | ||
.claims(it.toMap()) | ||
.expiration(Date(System.currentTimeMillis() + expiryTime.toMillis())) | ||
.signWith(keyPair.private, Jwts.SIG.RS256) | ||
.compact() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package uk.gov.justice.digital.hmpps.hmppstemplatepackagename.integration | ||
|
||
import org.assertj.core.api.Assertions.assertThat | ||
import org.junit.jupiter.api.Test | ||
import java.time.LocalDate | ||
|
||
class TimeResourceIntTest : IntegrationTestBase() { | ||
|
||
@Test | ||
fun `should return unauthorized if no token`() { | ||
webTestClient.get() | ||
.uri("/time") | ||
.exchange() | ||
.expectStatus().isUnauthorized | ||
} | ||
|
||
@Test | ||
fun `should return forbidden if no role`() { | ||
webTestClient.get() | ||
.uri("/time") | ||
.headers(setAuthorisation(roles = listOf())) | ||
.exchange() | ||
.expectStatus().isForbidden | ||
.expectBody().jsonPath("status").isEqualTo(403) | ||
} | ||
|
||
@Test | ||
fun `should return forbidden if wrong role`() { | ||
webTestClient.get() | ||
.uri("/time") | ||
.headers(setAuthorisation(roles = listOf("ROLE_WRONG"))) | ||
.exchange() | ||
.expectStatus().isForbidden | ||
.expectBody().jsonPath("status").isEqualTo(403) | ||
} | ||
|
||
@Test | ||
fun `should return OK`() { | ||
webTestClient.get() | ||
.uri("/time") | ||
.headers(setAuthorisation(roles = listOf("ROLE_TEMPLATE_EXAMPLE"))) | ||
.exchange() | ||
.expectStatus() | ||
.isOk | ||
.expectBody() | ||
.jsonPath("$").value<String> { | ||
assertThat(it).startsWith("${LocalDate.now()}") | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if we could have a better variable now for HMPPS Auth instead