Skip to content

Commit 1933fe8

Browse files
umeetiusbaarumeetiusbaar
authored
HAI-3541 Fix backchannel-logout (#1128)
* Return an error HAI0006 when the session is terminated due to backchannel-logout. In this case UI will know about it and can log out also.
1 parent 778634a commit 1933fe8

31 files changed

Lines changed: 480 additions & 151 deletions

‎services/hanke-service/build.gradle.kts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,11 @@ dependencies {
137137
annotationProcessor("org.springframework.boot:spring-boot-configuration-processor")
138138
}
139139

140-
tasks.withType<KotlinCompile> { compilerOptions { freeCompilerArgs = listOf("-Xjsr305=strict") } }
140+
tasks.withType<KotlinCompile> {
141+
compilerOptions {
142+
freeCompilerArgs = listOf("-Xjsr305=strict", "-Xannotation-default-target=param-property")
143+
}
144+
}
141145

142146
kotlin { jvmToolchain { languageVersion.set(JavaLanguageVersion.of(21)) } }
143147

‎services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/IntegrationTestConfiguration.kt‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import fi.hel.haitaton.hanke.profiili.ProfiiliClient
3131
import fi.hel.haitaton.hanke.profiili.ProfiiliService
3232
import fi.hel.haitaton.hanke.security.AccessRules
3333
import fi.hel.haitaton.hanke.security.LogoutService
34-
import fi.hel.haitaton.hanke.security.UserSessionCache
3534
import fi.hel.haitaton.hanke.security.UserSessionRepository
3635
import fi.hel.haitaton.hanke.security.UserSessionService
3736
import fi.hel.haitaton.hanke.taydennys.TaydennysAuthorizer
@@ -142,8 +141,6 @@ class IntegrationTestConfiguration {
142141

143142
@Bean fun tormaystarkasteluLaskentaService(): TormaystarkasteluLaskentaService = mockk()
144143

145-
@Bean fun userSessionCache(): UserSessionCache = mockk(relaxed = true)
146-
147144
@Bean fun userSessionRepository(): UserSessionRepository = mockk()
148145

149146
@Bean fun userSessionService(): UserSessionService = mockk()

‎services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/profiili/ProfiiliClientITest.kt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,29 @@ class ProfiiliClientITest {
124124
assertThat(request.headers["Accept"]).isEqualTo(MediaType.APPLICATION_JSON_VALUE)
125125
}
126126

127+
@Test
128+
fun `throws exception when unauthorized`() {
129+
mockApiTokensApi.enqueue(
130+
MockResponse.Builder()
131+
.code(401)
132+
.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)
133+
.body(
134+
"{\"error\":\"invalid_grant\",\"error_description\":\"Invalid bearer token\"}"
135+
)
136+
.build()
137+
)
138+
139+
val failure = assertFailure { profiiliClient.getVerifiedName(ACCESS_TOKEN) }
140+
141+
failure.all {
142+
hasClass(UnauthorizedException::class)
143+
messageContains("Profiili API token request was unauthorized.")
144+
messageContains("invalid_grant")
145+
messageContains("Invalid bearer token")
146+
}
147+
assertThat(mockApiTokensApi.requestCount).isEqualTo(1)
148+
}
149+
127150
@Test
128151
fun `throws exception when API tokens not found`() {
129152
mockApiTokensApi.enqueue(MockResponse.Builder().code(404).build())

‎services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/profiili/ProfiiliControllerITest.kt‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,18 @@ class ProfiiliControllerITest(@Autowired override val mockMvc: MockMvc) : Contro
6868
}
6969

7070
@Test
71-
fun `returns 404 when profiili client throws expected exception`() {
71+
fun `returns 401 when profiili client throws unauthorized exception`() {
72+
every { profiiliService.getVerifiedName(any()) } throws
73+
UnauthorizedException("Because of reasons.")
74+
75+
get(url).andExpect(MockMvcResultMatchers.status().isUnauthorized)
76+
77+
verifyAll { profiiliService.getVerifiedName(any()) }
78+
verify { disclosureLogService wasNot Called }
79+
}
80+
81+
@Test
82+
fun `returns 404 when profiili client throws not found exception`() {
7283
every { profiiliService.getVerifiedName(any()) } throws
7384
VerifiedNameNotFound("Because of reasons.")
7485

‎services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/security/LogoutServiceITest.kt‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package fi.hel.haitaton.hanke.security
22

33
import assertk.assertThat
4-
import assertk.assertions.isEmpty
54
import assertk.assertions.isEqualTo
65
import com.nimbusds.jose.JWSAlgorithm
76
import com.nimbusds.jose.JWSHeader
@@ -52,7 +51,7 @@ class LogoutServiceITest(
5251
}
5352

5453
@Test
55-
fun `deletes matching user session`() {
54+
fun `terminates matching user session`() {
5655
userSessionRepository.save(
5756
UserSessionEntity(subject = SUB, sessionId = SID, createdAt = Instant.now())
5857
)
@@ -61,7 +60,9 @@ class LogoutServiceITest(
6160

6261
logoutService.logout(logoutToken)
6362

64-
assertThat(userSessionRepository.findAll()).isEmpty()
63+
val session = userSessionRepository.findAll().single()
64+
assertThat(session.sessionId).isEqualTo(SID)
65+
assertThat(session.terminated).isEqualTo(true)
6566
}
6667

6768
@Test
@@ -128,7 +129,7 @@ class LogoutServiceITest(
128129
iat: Instant? = Instant.now(),
129130
exp: Instant? = Instant.now().plusSeconds(60),
130131
jti: String? = "id",
131-
events: Map<String, Any>? = mapOf(BACKCHANNEL_LOGOUT_EVENT to "{}"),
132+
events: Map<String, Any>? = mapOf(BACKCHANNEL_LOGOUT_EVENT to emptyMap<String, Any>()),
132133
sid: String? = "session-id",
133134
nonce: String? = null,
134135
): JWTClaimsSet =

‎services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/security/UserSessionServiceITest.kt‎

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,13 @@ class UserSessionServiceITest(
2424
UserSessionEntity(
2525
subject = "subject",
2626
sessionId = UUID.randomUUID().toString(),
27-
createdAt = Instant.now().minusSeconds(10),
27+
createdAt = Instant.now(),
28+
expiresAt = Instant.now().minusSeconds(10),
2829
)
2930
)
3031
assertThat(repository.findAll().single().subject).isEqualTo("subject")
3132

32-
service.cleanupExpiredSessions(5)
33+
service.cleanupExpiredSessions()
3334

3435
assertThat(repository.findAll()).isEmpty()
3536
}
@@ -40,50 +41,77 @@ class UserSessionServiceITest(
4041
UserSessionEntity(
4142
subject = "subject",
4243
sessionId = UUID.randomUUID().toString(),
43-
createdAt = Instant.now().plusSeconds(60),
44+
createdAt = Instant.now(),
45+
expiresAt = Instant.now().plusSeconds(60),
4446
)
4547
)
4648
assertThat(repository.findAll().single().subject).isEqualTo("subject")
4749

48-
service.cleanupExpiredSessions(5)
50+
service.cleanupExpiredSessions()
4951

5052
assertThat(repository.findAll().single().subject).isEqualTo("subject")
5153
}
5254
}
5355

5456
@Nested
55-
inner class SaveSessionIfNotExists {
57+
inner class ValidateAndSaveSession {
5658

5759
@Test
58-
fun `saves non-existing session`() {
59-
service.saveSessionIfNotExists(
60-
subject = "subject",
61-
sessionId = UUID.randomUUID().toString(),
62-
createdAt = Instant.now(),
60+
fun `returns true when session exists and is not terminated`() {
61+
val sessionId = UUID.randomUUID().toString()
62+
val expiresAt = Instant.now().plusSeconds(3600)
63+
repository.save(
64+
UserSessionEntity(
65+
subject = "subject",
66+
sessionId = sessionId,
67+
createdAt = Instant.now(),
68+
expiresAt = expiresAt,
69+
terminated = false,
70+
)
6371
)
6472

65-
assertThat(repository.findAll().single().subject).isEqualTo("subject")
73+
val isValid =
74+
service.validateAndSaveSession("subject", sessionId, Instant.now(), expiresAt)
75+
76+
assertThat(isValid).isEqualTo(true)
6677
}
6778

6879
@Test
69-
fun `does not save existing session`() {
80+
fun `returns true and saves new session`() {
7081
val sessionId = UUID.randomUUID().toString()
82+
// Truncate to microseconds to match PostgreSQL TIMESTAMP precision
83+
val expiresAt =
84+
Instant.now().plusSeconds(3600).truncatedTo(java.time.temporal.ChronoUnit.MICROS)
85+
assertThat(repository.findAll()).isEmpty()
86+
87+
val isValid =
88+
service.validateAndSaveSession("subject", sessionId, Instant.now(), expiresAt)
89+
90+
assertThat(isValid).isEqualTo(true)
91+
val saved = repository.findAll().single()
92+
assertThat(saved.sessionId).isEqualTo(sessionId)
93+
assertThat(saved.expiresAt).isEqualTo(expiresAt)
94+
assertThat(saved.terminated).isEqualTo(false)
95+
}
96+
97+
@Test
98+
fun `returns false when session is terminated`() {
99+
val sessionId = UUID.randomUUID().toString()
100+
val expiresAt = Instant.now().plusSeconds(3600)
71101
repository.save(
72102
UserSessionEntity(
73103
subject = "subject",
74104
sessionId = sessionId,
75105
createdAt = Instant.now(),
106+
expiresAt = expiresAt,
107+
terminated = true,
76108
)
77109
)
78-
assertThat(repository.findAll().single().subject).isEqualTo("subject")
79110

80-
service.saveSessionIfNotExists(
81-
subject = "subject",
82-
sessionId = sessionId,
83-
createdAt = Instant.now(),
84-
)
111+
val isValid =
112+
service.validateAndSaveSession("subject", sessionId, Instant.now(), expiresAt)
85113

86-
assertThat(repository.findAll().single().subject).isEqualTo("subject")
114+
assertThat(isValid).isEqualTo(false)
87115
}
88116
}
89117
}

‎services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/testdata/TestDataControllerEnabledITest.kt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,17 @@ class TestDataControllerEnabledITest(@Autowired override val mockMvc: MockMvc) :
129129
verify { testDataService wasNot Called }
130130
}
131131
}
132+
133+
@Nested
134+
inner class TerminateCurrentSession {
135+
private val url = "$BASE_URL/terminate-session"
136+
137+
@Test
138+
@WithAnonymousUser
139+
fun `returns 401 when not authenticated`() {
140+
delete(url).andExpect(MockMvcResultMatchers.status().isUnauthorized)
141+
142+
verify { testDataService wasNot Called }
143+
}
144+
}
132145
}

‎services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/HankeError.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ enum class HankeError(val errorMessage: String) {
1313
HAI0003("Invalid data"),
1414
HAI0004("Resource does not exist"),
1515
HAI0005("Insufficient permissions"),
16+
HAI0006("Session terminated"),
1617
HAI1001("Hanke not found"),
1718
HAI1002("Invalid Hanke data"),
1819
HAI1003("Internal error while saving Hanke"),
@@ -57,6 +58,7 @@ enum class HankeError(val errorMessage: String) {
5758
HAI4005("Could not verify user identity"),
5859
HAI4006("Duplicate hankekayttaja"),
5960
HAI4007("Verified name not found in Profiili"),
61+
HAI4008("Verified name call was unauthorized"),
6062
HAI5001("Decision not found"),
6163
HAI6001("Taydennys not found"),
6264
HAI6002("Taydennys has no changes"),

‎services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/profiili/ProfiiliClient.kt‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ import mu.KotlinLogging
77
import org.springframework.beans.factory.annotation.Value
88
import org.springframework.context.annotation.Profile
99
import org.springframework.core.ParameterizedTypeReference
10+
import org.springframework.http.HttpStatus
1011
import org.springframework.http.MediaType
1112
import org.springframework.stereotype.Component
1213
import org.springframework.web.reactive.function.BodyInserters
1314
import org.springframework.web.reactive.function.client.WebClient
1415
import org.springframework.web.reactive.function.client.WebClientResponseException
1516
import org.springframework.web.reactive.function.client.body
17+
import org.springframework.web.reactive.function.client.bodyToMono
1618
import reactor.core.publisher.Mono
1719

1820
private val logger = KotlinLogging.logger {}
@@ -73,6 +75,13 @@ class ProfiiliClient(
7375
.with("permission", TOKEN_API_PERMISSION)
7476
)
7577
.retrieve()
78+
// handle 401 errors specifically to give a more informative error message
79+
.onStatus({ responseStatus -> responseStatus == HttpStatus.UNAUTHORIZED }) { response ->
80+
response.bodyToMono<String>().flatMap { body ->
81+
logger.error { "Error from Profiili API call. Response status=401, body=$body" }
82+
Mono.error(UnauthorizedException(body))
83+
}
84+
}
7685
.bodyToMono(JsonNode::class.java)
7786
.doOnError(WebClientResponseException::class.java) { ex ->
7887
logger.error {
@@ -83,7 +92,7 @@ class ProfiiliClient(
8392
}
8493

8594
private fun getTokenApiUrl(): String {
86-
logger.info { "Loading Profiili token URI from OpenID configuration.." }
95+
logger.info { "Loading Profiili token URI from OpenID configuration..." }
8796
val configurationUri = "$issuer/.well-known/openid-configuration"
8897

8998
val conf =
@@ -130,5 +139,8 @@ class ProfiiliClient(
130139
class VerifiedNameNotFound(reason: String) :
131140
RuntimeException("Verified name of user could not be obtained. $reason")
132141

142+
class UnauthorizedException(reason: String) :
143+
RuntimeException("Profiili API token request was unauthorized. $reason")
144+
133145
class ProfiiliConfigurationError(reason: String, cause: Exception? = null) :
134146
RuntimeException("Error in Profiili API connection: $reason", cause)

‎services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/profiili/ProfiiliController.kt‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import io.swagger.v3.oas.annotations.Parameter
88
import io.swagger.v3.oas.annotations.media.Content
99
import io.swagger.v3.oas.annotations.media.Schema
1010
import io.swagger.v3.oas.annotations.responses.ApiResponse
11+
import io.swagger.v3.oas.annotations.responses.ApiResponses
1112
import io.swagger.v3.oas.annotations.security.SecurityRequirement
1213
import mu.KotlinLogging
1314
import org.springframework.http.HttpStatus
@@ -28,16 +29,35 @@ class ProfiiliController(private val profiiliService: ProfiiliService) {
2829

2930
@GetMapping("/verified-name")
3031
@Operation(summary = "Get verified name from Profiili")
31-
@ApiResponse(description = "Success", responseCode = "200")
32-
@ApiResponse(
33-
description = "Verification not found.",
34-
responseCode = "404",
35-
content = [Content(schema = Schema(implementation = HankeError::class))],
32+
@ApiResponses(
33+
value =
34+
[
35+
ApiResponse(description = "Success", responseCode = "200"),
36+
ApiResponse(
37+
description = "Not authorized.",
38+
responseCode = "401",
39+
content = [Content(schema = Schema(implementation = HankeError::class))],
40+
),
41+
ApiResponse(
42+
description = "Verification not found.",
43+
responseCode = "404",
44+
content = [Content(schema = Schema(implementation = HankeError::class))],
45+
),
46+
]
3647
)
3748
fun verifiedName(
3849
@Parameter(hidden = true) @CurrentSecurityContext securityContext: SecurityContext
3950
): Names = profiiliService.getVerifiedName(securityContext)
4051

52+
@ExceptionHandler(UnauthorizedException::class)
53+
@ResponseStatus(HttpStatus.UNAUTHORIZED)
54+
@Hidden
55+
fun unauthorized(ex: UnauthorizedException): HankeError {
56+
logger.warn { ex.message }
57+
Sentry.captureException(ex)
58+
return HankeError.HAI4008
59+
}
60+
4161
@ExceptionHandler(VerifiedNameNotFound::class)
4262
@ResponseStatus(HttpStatus.NOT_FOUND)
4363
@Hidden

0 commit comments

Comments
 (0)