From dd18b3d60de935bae7910a511a0934989dbdc9a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Li=C3=A9tar?= Date: Tue, 24 Sep 2024 14:53:32 +0100 Subject: [PATCH 1/7] Add secret-less authentication for CI jobs. This change allows automated environments, such as Github Actions or Buildkite to authenticate to the Packit API without needing to be manually provisioned with a secret. The client's environment creates and signs a JWT containing claims describing the job. For example, on Github Actions, GitHub provides a token which includes the organisation name, repository name, type of event, branch name, etc. Packit needs to be configured to trust the 3rd party JWT issuer, with policies specifying what claims are needed. The client sends the JWT it received from its environment to the Packit `/auth/login/jwt` endpoint, and in exchange receives a Packit JWT which may be used to access the rest of the API. --- api/app/build.gradle.kts | 1 + api/app/src/main/kotlin/packit/App.kt | 6 +- .../kotlin/packit/config/JwtLoginConfig.kt | 22 + .../packit/controllers/LoginController.kt | 27 ++ .../packit/security/provider/TokenProvider.kt | 82 +++- .../packit/service/GithubAPILoginService.kt | 12 +- .../kotlin/packit/service/JwtLoginService.kt | 103 ++++ .../main/kotlin/packit/service/UserService.kt | 11 + .../src/main/resources/errorBundle.properties | 2 + .../packit/integration/IntegrationTest.kt | 7 +- .../controllers/LoginControllerTest.kt | 91 ++++ .../kotlin/packit/testing/TestJwtIssuer.kt | 25 + .../unit/controllers/LoginControllerTest.kt | 14 +- .../security/provider/TokenDecoderTest.kt | 7 +- .../unit/service/GithubAPILoginServiceTest.kt | 15 +- .../unit/service/JwtLoginServiceTest.kt | 439 ++++++++++++++++++ app/src/types.ts | 1 - docs/auth.md | 38 ++ 18 files changed, 849 insertions(+), 54 deletions(-) create mode 100644 api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt create mode 100644 api/app/src/main/kotlin/packit/service/JwtLoginService.kt create mode 100644 api/app/src/test/kotlin/packit/testing/TestJwtIssuer.kt create mode 100644 api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt diff --git a/api/app/build.gradle.kts b/api/app/build.gradle.kts index 167bea05..6fcbf2bd 100644 --- a/api/app/build.gradle.kts +++ b/api/app/build.gradle.kts @@ -40,6 +40,7 @@ dependencies { testImplementation("org.mockito.kotlin:mockito-kotlin:4.1.0") testImplementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.14.1") testImplementation("com.github.fge:json-schema-validator:2.2.6") + testImplementation("org.mock-server:mockserver-junit-jupiter:5.15.0") // This dependency is used by the application. implementation("com.google.guava:guava:31.1-jre") diff --git a/api/app/src/main/kotlin/packit/App.kt b/api/app/src/main/kotlin/packit/App.kt index f52596fa..f7fae344 100644 --- a/api/app/src/main/kotlin/packit/App.kt +++ b/api/app/src/main/kotlin/packit/App.kt @@ -1,14 +1,16 @@ package packit import org.springframework.boot.autoconfigure.SpringBootApplication +import org.springframework.boot.context.properties.ConfigurationPropertiesScan import org.springframework.boot.runApplication import org.springframework.scheduling.annotation.EnableScheduling @EnableScheduling @SpringBootApplication +@ConfigurationPropertiesScan class App -fun main() +fun main(args: Array) { - runApplication() + runApplication(args = args) } diff --git a/api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt b/api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt new file mode 100644 index 00000000..97c6db47 --- /dev/null +++ b/api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt @@ -0,0 +1,22 @@ +package packit.config + +import org.springframework.boot.context.properties.ConfigurationProperties +import org.springframework.boot.convert.DurationUnit +import java.time.Duration +import java.time.temporal.ChronoUnit + +data class JwtPolicy( + val jwkSetURI: String, + val issuer: String, + val requiredClaims: Map = mapOf(), + val grantedPermissions: List = listOf(), + + @DurationUnit(ChronoUnit.SECONDS) + val tokenDuration: Duration? = null, +) + +@ConfigurationProperties(prefix = "auth.external-jwt") +data class JwtLoginConfig( + val audience: String?, + val policy: List = listOf() +) diff --git a/api/app/src/main/kotlin/packit/controllers/LoginController.kt b/api/app/src/main/kotlin/packit/controllers/LoginController.kt index dd2c21e0..96e01933 100644 --- a/api/app/src/main/kotlin/packit/controllers/LoginController.kt +++ b/api/app/src/main/kotlin/packit/controllers/LoginController.kt @@ -11,6 +11,7 @@ import packit.model.dto.LoginWithToken import packit.model.dto.UpdatePassword import packit.service.BasicLoginService import packit.service.GithubAPILoginService +import packit.service.JwtLoginService import packit.service.UserService @RestController @@ -18,6 +19,7 @@ import packit.service.UserService class LoginController( val gitApiLoginService: GithubAPILoginService, val basicLoginService: BasicLoginService, + val jwtLoginService: JwtLoginService, val config: AppConfig, val userService: UserService, ) @@ -50,6 +52,31 @@ class LoginController( return ResponseEntity.ok(token) } + @PostMapping("/login/jwt") + @ResponseBody + fun loginJWT( + @RequestBody @Validated user: LoginWithToken, + ): ResponseEntity> + { + if (jwtLoginService.audience == null) { + throw PackitException("jwtLoginDisabled", HttpStatus.FORBIDDEN) + } + + val token = jwtLoginService.authenticateAndIssueToken(user) + return ResponseEntity.ok(token) + } + + @GetMapping("/login/jwt/audience") + @ResponseBody + fun jwtAudience(): ResponseEntity> + { + if (jwtLoginService.audience == null) { + throw PackitException("jwtLoginDisabled", HttpStatus.FORBIDDEN) + } else { + return ResponseEntity.ok(mapOf("audience" to jwtLoginService.audience!!)) + } + } + @GetMapping("/config") @ResponseBody fun authConfig(): ResponseEntity> diff --git a/api/app/src/main/kotlin/packit/security/provider/TokenProvider.kt b/api/app/src/main/kotlin/packit/security/provider/TokenProvider.kt index 768bc967..82ebd8d3 100644 --- a/api/app/src/main/kotlin/packit/security/provider/TokenProvider.kt +++ b/api/app/src/main/kotlin/packit/security/provider/TokenProvider.kt @@ -9,9 +9,25 @@ import java.time.Duration import java.time.Instant import java.time.temporal.ChronoUnit +interface JwtBuilder +{ + fun withExpiresAt(expiry: Instant): JwtBuilder + fun withDuration(duration: Duration): JwtBuilder + fun withPermissions(permissions: Collection): JwtBuilder + fun issue(): String +} + interface JwtIssuer { - fun issue(userPrincipal: UserPrincipal): String + fun issue(userPrincipal: UserPrincipal): String { + return builder(userPrincipal).issue() + } + + // The builder method can be used to customize the returned token. + // + // The builder can only ever be used to weaken a token, eg. by reducing its + // permissions or shorten its lifespan. + fun builder(userPrincipal: UserPrincipal): JwtBuilder } @Component @@ -23,23 +39,59 @@ class TokenProvider(val config: AppConfig) : JwtIssuer const val TOKEN_AUDIENCE = "packit" } - override fun issue(userPrincipal: UserPrincipal): String - { + inner class Builder(val userPrincipal: UserPrincipal) : JwtBuilder { + var duration: Duration = Duration.of(config.authExpiryDays, ChronoUnit.DAYS) + var expiry: Instant? = null + var permissions: MutableSet? = null + + override fun withExpiresAt(expiry: Instant): JwtBuilder { + if (this.expiry == null || expiry > this.expiry) { + this.expiry = expiry + } + return this + } + + override fun withDuration(duration: Duration): JwtBuilder { + if (duration < this.duration) { + this.duration = duration + } + return this + } - val roles = userPrincipal.authorities.map { it.authority } + override fun withPermissions(permissions: Collection): JwtBuilder { + if (this.permissions == null) { + this.permissions = permissions.toMutableSet() + } else { + this.permissions!!.retainAll(permissions) + } + return this + } - val createdDate = Instant.now() + override fun issue(): String { + val issuedAt = Instant.now() + var expiresAt = issuedAt.plus(duration) + if (expiry != null && expiry!! < expiresAt) { + expiresAt = expiry + } - val expiredDate = createdDate.plus(Duration.of(config.authExpiryDays, ChronoUnit.DAYS)) + val permissions = userPrincipal.authorities.map { it.authority }.toMutableList() + if (this.permissions != null) { + permissions.retainAll(this.permissions!!) + } + + return JWT.create() + .withAudience(TOKEN_AUDIENCE) + .withIssuer(TOKEN_ISSUER) + .withClaim("userName", userPrincipal.name) + .withClaim("displayName", userPrincipal.displayName) + .withClaim("au", permissions) + .withIssuedAt(issuedAt) + .withExpiresAt(expiresAt) + .sign(Algorithm.HMAC256(config.authJWTSecret)) + } + } - return JWT.create() - .withAudience(TOKEN_AUDIENCE) - .withIssuer(TOKEN_ISSUER) - .withClaim("userName", userPrincipal.name) - .withClaim("displayName", userPrincipal.displayName) - .withClaim("datetime", createdDate) - .withClaim("au", roles) - .withExpiresAt(expiredDate) - .sign(Algorithm.HMAC256(config.authJWTSecret)) + override fun builder(userPrincipal: UserPrincipal): JwtBuilder { + return Builder(userPrincipal) } } diff --git a/api/app/src/main/kotlin/packit/service/GithubAPILoginService.kt b/api/app/src/main/kotlin/packit/service/GithubAPILoginService.kt index 8acf6ebc..e9713631 100644 --- a/api/app/src/main/kotlin/packit/service/GithubAPILoginService.kt +++ b/api/app/src/main/kotlin/packit/service/GithubAPILoginService.kt @@ -6,7 +6,6 @@ import packit.AppConfig import packit.clients.GithubUserClient import packit.exceptions.PackitException import packit.model.dto.LoginWithToken -import packit.security.profile.UserPrincipal import packit.security.provider.JwtIssuer @Component @@ -15,7 +14,6 @@ class GithubAPILoginService( val jwtIssuer: JwtIssuer, val githubUserClient: GithubUserClient, val userService: UserService, - val roleService: RoleService ) { fun authenticateAndIssueToken(loginRequest: LoginWithToken): Map @@ -30,15 +28,7 @@ class GithubAPILoginService( val githubUser = githubUserClient.getGithubUser() var user = userService.saveUserFromGithub(githubUser.login, githubUser.name, githubUser.email) - - val token = jwtIssuer.issue( - UserPrincipal( - user.username, - user.displayName, - roleService.getGrantedAuthorities(user.roles), - mutableMapOf() - ) - ) + val token = jwtIssuer.issue(userService.getUserPrincipal(user)) return mapOf("token" to token) } diff --git a/api/app/src/main/kotlin/packit/service/JwtLoginService.kt b/api/app/src/main/kotlin/packit/service/JwtLoginService.kt new file mode 100644 index 00000000..77101dcd --- /dev/null +++ b/api/app/src/main/kotlin/packit/service/JwtLoginService.kt @@ -0,0 +1,103 @@ +package packit.service + +import org.slf4j.LoggerFactory +import org.springframework.http.HttpStatus +import org.springframework.security.oauth2.core.DelegatingOAuth2TokenValidator +import org.springframework.security.oauth2.core.oidc.IdTokenClaimNames +import org.springframework.security.oauth2.jwt.Jwt +import org.springframework.security.oauth2.jwt.JwtClaimValidator +import org.springframework.security.oauth2.jwt.JwtException +import org.springframework.security.oauth2.jwt.JwtIssuerValidator +import org.springframework.security.oauth2.jwt.JwtTimestampValidator +import org.springframework.security.oauth2.jwt.NimbusJwtDecoder +import org.springframework.stereotype.Component +import org.springframework.web.client.RestOperations +import org.springframework.web.client.RestTemplate +import packit.config.JwtLoginConfig +import packit.config.JwtPolicy +import packit.exceptions.PackitException +import packit.model.dto.LoginWithToken +import packit.security.provider.JwtIssuer +import java.util.function.Predicate + +data class TokenPolicy( + val decoder: NimbusJwtDecoder, + val config: JwtPolicy, +) + +@Component +class JwtLoginService( + val jwtIssuer: JwtIssuer, + val userService: UserService, + val jwtLoginConfig: JwtLoginConfig, + val restOperations: RestOperations = RestTemplate(), +) { + lateinit var policies: List + + val audience: String? = jwtLoginConfig.audience + + init { + if (audience != null) { + val audValidator = JwtClaimValidator>( + IdTokenClaimNames.AUD, { claimValue -> claimValue != null && claimValue.contains(audience) } + ) + policies = jwtLoginConfig.policy.map { policy -> + val validators = mutableListOf( + JwtTimestampValidator(), + JwtIssuerValidator(policy.issuer), + audValidator, + ) + + policy.requiredClaims.mapTo(validators, { entry -> + JwtClaimValidator(entry.key, Predicate.isEqual(entry.value)) + }) + + val decoder = NimbusJwtDecoder.withJwkSetUri(policy.jwkSetURI) + .restOperations(restOperations) + .build() + + decoder.setJwtValidator(DelegatingOAuth2TokenValidator(validators)) + TokenPolicy(decoder, policy) + } + } else { + policies = listOf() + } + } + + private fun issueToken(verifiedToken: Jwt, policy: JwtPolicy): String { + val user = userService.getServiceUser() + val userPrincipal = userService.getUserPrincipal(user) + val tokenBuilder = jwtIssuer.builder(userPrincipal) + tokenBuilder.withPermissions(policy.grantedPermissions) + + val expiresAt = verifiedToken.expiresAt + if (expiresAt != null) { + tokenBuilder.withExpiresAt(expiresAt) + } + if (policy.tokenDuration != null) { + tokenBuilder.withDuration(policy.tokenDuration) + } + return tokenBuilder.issue() + } + + fun authenticateAndIssueToken(user: LoginWithToken): Map { + for (policy in policies) { + val decodedToken = try { + policy.decoder.decode(user.token) + } catch (e: JwtException) { + log.info("JWT policy rejected: {}", e.message) + continue + } + val issuedToken = issueToken(decodedToken, policy.config) + + return mapOf("token" to issuedToken) + } + + throw PackitException("externalJwtTokenInvalid", HttpStatus.UNAUTHORIZED) + } + + companion object + { + private val log = LoggerFactory.getLogger(JwtLoginService::class.java) + } +} diff --git a/api/app/src/main/kotlin/packit/service/UserService.kt b/api/app/src/main/kotlin/packit/service/UserService.kt index 7e70a75e..29b66f14 100644 --- a/api/app/src/main/kotlin/packit/service/UserService.kt +++ b/api/app/src/main/kotlin/packit/service/UserService.kt @@ -9,6 +9,7 @@ import packit.model.User import packit.model.dto.CreateBasicUser import packit.model.dto.UpdatePassword import packit.repository.UserRepository +import packit.security.profile.UserPrincipal import java.time.Instant import javax.naming.AuthenticationException @@ -25,6 +26,7 @@ interface UserService fun updatePassword(username: String, updatePassword: UpdatePassword) fun checkAndUpdateLastLoggedIn(username: String) fun getServiceUser(): User + fun getUserPrincipal(user: User): UserPrincipal } @Service @@ -166,4 +168,13 @@ class BaseUserService( return userRepository.findByUsernameAndUserSource("SERVICE", "service") ?: throw PackitException("serviceUserNotFound", HttpStatus.INTERNAL_SERVER_ERROR) } + + override fun getUserPrincipal(user: User): UserPrincipal { + return UserPrincipal( + user.username, + user.displayName, + roleService.getGrantedAuthorities(user.roles), + mutableMapOf() + ) + } } diff --git a/api/app/src/main/resources/errorBundle.properties b/api/app/src/main/resources/errorBundle.properties index 7a5ce1ff..63366bd8 100644 --- a/api/app/src/main/resources/errorBundle.properties +++ b/api/app/src/main/resources/errorBundle.properties @@ -15,6 +15,8 @@ githubTokenInvalid=The supplied GitHub token is invalid. githubTokenInsufficientPermissions=The supplied GitHub token does not have sufficient permissions to check user credentials. githubTokenUnexpectedError=Unexpected error when checking user credentials with GitHub. githubUserRestrictedAccess=User is not in allowed organization or team. Please contact your administrator. +jwtLoginDisabled=JWT login is disabled +externalJwtTokenInvalid=The supplied token is invalid httpClientError=Http Client error occurred invalidPassword=Invalid password insufficientPrivileges=You do not have sufficient privileges for attempted action diff --git a/api/app/src/test/kotlin/packit/integration/IntegrationTest.kt b/api/app/src/test/kotlin/packit/integration/IntegrationTest.kt index aeca2273..87301803 100644 --- a/api/app/src/test/kotlin/packit/integration/IntegrationTest.kt +++ b/api/app/src/test/kotlin/packit/integration/IntegrationTest.kt @@ -57,19 +57,18 @@ abstract class IntegrationTest return HttpEntity(data, headers) } - protected fun assertSuccess(responseEntity: ResponseEntity) + protected fun assertSuccess(responseEntity: ResponseEntity) { assertEquals(responseEntity.statusCode, HttpStatus.OK) assertEquals(responseEntity.headers.contentType, MediaType.APPLICATION_JSON) - assertThat(responseEntity.body).isNotEmpty } - protected fun assertUnauthorized(responseEntity: ResponseEntity) + protected fun assertUnauthorized(responseEntity: ResponseEntity) { assertEquals(responseEntity.statusCode, HttpStatus.UNAUTHORIZED) } - protected fun assertForbidden(responseEntity: ResponseEntity) + protected fun assertForbidden(responseEntity: ResponseEntity) { assertEquals(responseEntity.statusCode, HttpStatus.FORBIDDEN) } diff --git a/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt index 41497505..43e2674f 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt @@ -1,10 +1,18 @@ package packit.integration.controllers +import com.fasterxml.jackson.databind.JsonNode import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import org.mockserver.integration.ClientAndServer +import org.mockserver.junit.jupiter.MockServerExtension +import org.mockserver.junit.jupiter.MockServerSettings +import org.mockserver.model.HttpRequest.request +import org.mockserver.model.HttpResponse.response +import org.mockserver.model.JsonBody import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.web.client.TestRestTemplate import org.springframework.http.* @@ -16,6 +24,8 @@ import packit.model.dto.LoginWithPassword import packit.model.dto.LoginWithToken import packit.model.dto.UpdatePassword import packit.repository.UserRepository +import packit.security.provider.TokenDecoder +import packit.testing.TestJwtIssuer import java.time.Instant import kotlin.test.assertEquals @@ -143,6 +153,87 @@ class LoginControllerTestBasic : IntegrationTest() } } +@ExtendWith(MockServerExtension::class) +@MockServerSettings(ports = [8787]) +@TestPropertySource( + properties = [ + "auth.external-jwt.audience=packit", + "auth.external-jwt.policy[0].jwk-set-uri=http://127.0.0.1:8787/jwks.json", + "auth.external-jwt.policy[0].issuer=issuer", + "auth.external-jwt.policy[0].granted-permissions=outpack.read,outpack.write", +] +) +class LoginControllerTestJwt(val jwksServer: ClientAndServer) : IntegrationTest() +{ + val trustedIssuer = TestJwtIssuer() + + @Autowired + private lateinit var tokenDecoder: TokenDecoder + + @BeforeEach + fun configureJwksServer() + { + jwksServer.`when`(request().withMethod("GET").withPath("/jwks.json")) + .respond(response().withStatusCode(200).withBody(JsonBody(trustedIssuer.jwkSet.toString()))) + } + + @AfterEach + fun resetJwksServer() + { + jwksServer.reset() + } + + @Test + fun `can get expected audience`() + { + val result = restTemplate.getForEntity("/auth/login/jwt/audience", JsonNode::class.java) + assertSuccess(result) + assertEquals(result.body?.required("audience")?.textValue(), "packit") + } + + @Test + fun `can login with external JWT`() + { + val externalToken = trustedIssuer.issue { builder -> + builder.issuer("issuer") + builder.audience(listOf("packit")) + } + val result = restTemplate.postForEntity( + "/auth/login/jwt", + LoginWithToken(externalToken), + JsonNode::class.java, + ) + assertSuccess(result) + + val token = tokenDecoder.decode(result.body?.required("token")?.textValue()!!) + assertEquals(token.getClaim("userName").asString(), "SERVICE") + assertEquals(token.getClaim("au").asList(String::class.java), listOf("outpack.read", "outpack.write")) + } + + @Test + fun `returns unauthorized when token is signed with different key`() + { + val untrustedIssuer = TestJwtIssuer() + val externalToken = untrustedIssuer.issue { builder -> + builder.issuer("issuer") + builder.audience(listOf("packit")) + } + val result = restTemplate.postForEntity("/auth/login/jwt", LoginWithToken(externalToken), JsonNode::class.java) + assertUnauthorized(result) + } + + @Test + fun `returns unauthorized when token has invalid claims`() + { + val token = trustedIssuer.issue { builder -> + builder.issuer("issuer") + builder.audience(listOf("not-packit")) + } + val result = restTemplate.postForEntity("/auth/login/jwt", LoginWithToken(token), String::class.java) + assertUnauthorized(result) + } +} + object LoginTestHelper { fun getBasicLoginResponse(body: LoginWithPassword, restTemplate: TestRestTemplate): ResponseEntity diff --git a/api/app/src/test/kotlin/packit/testing/TestJwtIssuer.kt b/api/app/src/test/kotlin/packit/testing/TestJwtIssuer.kt new file mode 100644 index 00000000..4f356401 --- /dev/null +++ b/api/app/src/test/kotlin/packit/testing/TestJwtIssuer.kt @@ -0,0 +1,25 @@ +package packit.testing + +import com.nimbusds.jose.jwk.JWKSet +import com.nimbusds.jose.jwk.gen.RSAKeyGenerator +import com.nimbusds.jose.jwk.source.ImmutableJWKSet +import org.mockito.kotlin.* +import org.springframework.security.oauth2.jwt.JwtClaimsSet +import org.springframework.security.oauth2.jwt.JwtEncoderParameters +import org.springframework.security.oauth2.jwt.NimbusJwtEncoder +import org.springframework.test.web.client.match.MockRestRequestMatchers.* +import org.springframework.test.web.client.response.DefaultResponseCreator.* +import org.springframework.test.web.client.response.MockRestResponseCreators.* + +class TestJwtIssuer +{ + val jwkSet = JWKSet(RSAKeyGenerator(2048).keyIDFromThumbprint(true).generate()) + private val encoder = NimbusJwtEncoder(ImmutableJWKSet(jwkSet)) + + fun issue(f: (JwtClaimsSet.Builder) -> Unit): String { + val builder = JwtClaimsSet.builder() + f(builder) + val claims = builder.build() + return encoder.encode(JwtEncoderParameters.from(claims)).getTokenValue() + } +} diff --git a/api/app/src/test/kotlin/packit/unit/controllers/LoginControllerTest.kt b/api/app/src/test/kotlin/packit/unit/controllers/LoginControllerTest.kt index 82cf58c6..200af706 100644 --- a/api/app/src/test/kotlin/packit/unit/controllers/LoginControllerTest.kt +++ b/api/app/src/test/kotlin/packit/unit/controllers/LoginControllerTest.kt @@ -31,7 +31,7 @@ class LoginControllerTest on { authEnableGithubLogin } doReturn true } - val sut = LoginController(mockLoginService, mock(), mockAppConfig, mock()) + val sut = LoginController(mockLoginService, mock(), mock(), mockAppConfig, mock()) val result = sut.loginWithGithub(request) @@ -49,7 +49,7 @@ class LoginControllerTest on { authEnableGithubLogin } doReturn false } - val sut = LoginController(mock(), mock(), mockAppConfig, mock()) + val sut = LoginController(mock(), mock(), mock(), mockAppConfig, mock()) val ex = assertThrows { sut.loginWithGithub(request) @@ -70,7 +70,7 @@ class LoginControllerTest on { authEnableBasicLogin } doReturn true } - val sut = LoginController(mock(), mockLoginService, mockAppConfig, mock()) + val sut = LoginController(mock(), mockLoginService, mock(), mockAppConfig, mock()) val result = sut.loginBasic(request) @@ -87,7 +87,7 @@ class LoginControllerTest on { authEnableBasicLogin } doReturn false } - val sut = LoginController(mock(), mock(), mockAppConfig, mock()) + val sut = LoginController(mock(), mock(), mock(), mockAppConfig, mock()) val ex = assertThrows { sut.loginBasic(request) @@ -105,7 +105,7 @@ class LoginControllerTest on { authEnableBasicLogin } doReturn true } - val sut = LoginController(mock(), mock(), mockAppConfig, mock()) + val sut = LoginController(mock(), mock(), mock(), mockAppConfig, mock()) val result = sut.authConfig() @@ -126,7 +126,7 @@ class LoginControllerTest on { authEnableBasicLogin } doReturn false } - val sut = LoginController(mock(), mock(), mockAppConfig, mock()) + val sut = LoginController(mock(), mock(), mock(), mockAppConfig, mock()) val ex = assertThrows { sut.updatePassword("user@email", UpdatePassword("current", "newpassword")) @@ -143,7 +143,7 @@ class LoginControllerTest } val mockUserService = mock() val updatePassword = UpdatePassword("current", "newpassword") - val sut = LoginController(mock(), mock(), mockAppConfig, mockUserService) + val sut = LoginController(mock(), mock(), mock(), mockAppConfig, mockUserService) sut.updatePassword("test@email.com", updatePassword) diff --git a/api/app/src/test/kotlin/packit/unit/security/provider/TokenDecoderTest.kt b/api/app/src/test/kotlin/packit/unit/security/provider/TokenDecoderTest.kt index 77c16eb9..1e688f3d 100644 --- a/api/app/src/test/kotlin/packit/unit/security/provider/TokenDecoderTest.kt +++ b/api/app/src/test/kotlin/packit/unit/security/provider/TokenDecoderTest.kt @@ -12,7 +12,6 @@ import packit.security.profile.UserPrincipal import packit.security.provider.TokenDecoder import packit.security.provider.TokenProvider import java.time.Duration -import java.time.temporal.ChronoUnit import java.util.* import kotlin.test.assertEquals @@ -46,16 +45,14 @@ class TokenDecoderTest val result = TokenDecoder(mockAppConfig).decode(jwtToken) - val expectedDatetime = result.getClaim("datetime").asDate().toInstant() - - val expectedExpiresAt = Date.from(expectedDatetime.plus(Duration.of(1, ChronoUnit.DAYS))) + val expectedExpiresAt = result.issuedAtAsInstant.plus(Duration.ofDays(1)) assertEquals(listOf("packit"), result.audience) assertEquals("packit-api", result.issuer) assertEquals("Fake Name", result.getClaim("displayName").asString()) assertEquals("fakeName", result.getClaim("userName").asString()) assertEquals(emptyList(), result.getClaim("au").asList(String::class.java)) - assertEquals(expectedExpiresAt, result.expiresAt) + assertEquals(expectedExpiresAt, result.expiresAtAsInstant) } @Test diff --git a/api/app/src/test/kotlin/packit/unit/service/GithubAPILoginServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/GithubAPILoginServiceTest.kt index eba5fb28..1136252a 100644 --- a/api/app/src/test/kotlin/packit/unit/service/GithubAPILoginServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/GithubAPILoginServiceTest.kt @@ -18,7 +18,6 @@ import packit.model.dto.LoginWithToken import packit.security.profile.UserPrincipal import packit.security.provider.JwtIssuer import packit.service.GithubAPILoginService -import packit.service.RoleService import packit.service.UserService import kotlin.test.assertEquals @@ -35,29 +34,27 @@ class GithubAPILoginServiceTest username = username, displayName = displayName, disabled = false, userSource = "github", roles = mutableListOf(Role(name = "USER")) ) - private val mockUserService = mock { - on { saveUserFromGithub(username, displayName, null) } doReturn fakeUser - } private val userPrincipal = UserPrincipal( username, displayName, mutableSetOf(SimpleGrantedAuthority("USER")), mutableMapOf() ) + private val mockUserService = mock { + on { saveUserFromGithub(username, displayName, null) } doReturn fakeUser + on { getUserPrincipal(fakeUser) } doReturn userPrincipal + } private val mockIssuer = mock { on { issue(argThat { this == userPrincipal }) } doReturn "fake jwt" } private val mockGithubUserClient = mock { on { getGithubUser() } doReturn fakeGHMyself } - private val mockRoleService = mock { - on { getGrantedAuthorities(fakeUser.roles) } doReturn mutableSetOf(SimpleGrantedAuthority("USER")) - } @Test fun `can authenticate and issue token`() { - val sut = GithubAPILoginService(mockConfig, mockIssuer, mockGithubUserClient, mockUserService, mockRoleService) + val sut = GithubAPILoginService(mockConfig, mockIssuer, mockGithubUserClient, mockUserService) val token = sut.authenticateAndIssueToken(LoginWithToken("fake token")) @@ -70,7 +67,7 @@ class GithubAPILoginServiceTest @Test fun `throws exception if token is empty`() { - val sut = GithubAPILoginService(mockConfig, mockIssuer, mockGithubUserClient, mockUserService, mockRoleService) + val sut = GithubAPILoginService(mockConfig, mockIssuer, mockGithubUserClient, mockUserService) val ex = assertThrows { sut.authenticateAndIssueToken(LoginWithToken("")) } assertEquals("emptyGitToken", ex.key) assertEquals(HttpStatus.BAD_REQUEST, ex.httpStatus) diff --git a/api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt new file mode 100644 index 00000000..1a3e14ab --- /dev/null +++ b/api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt @@ -0,0 +1,439 @@ +package packit.unit.service +import com.nimbusds.jose.proc.SecurityContext +import com.nimbusds.jwt.SignedJWT +import com.nimbusds.jwt.proc.DefaultJWTProcessor +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.assertDoesNotThrow +import org.junit.jupiter.api.assertThrows +import org.mockito.kotlin.* +import org.springframework.http.HttpMethod +import org.springframework.http.HttpStatus +import org.springframework.http.MediaType +import org.springframework.security.core.authority.SimpleGrantedAuthority +import org.springframework.security.oauth2.jwt.Jwt +import org.springframework.security.oauth2.jwt.JwtClaimsSet +import org.springframework.security.oauth2.jwt.NimbusJwtDecoder +import org.springframework.test.web.client.ExpectedCount +import org.springframework.test.web.client.MockRestServiceServer +import org.springframework.test.web.client.match.MockRestRequestMatchers.* +import org.springframework.test.web.client.response.DefaultResponseCreator.* +import org.springframework.test.web.client.response.MockRestResponseCreators.* +import org.springframework.web.client.RestTemplate +import packit.AppConfig +import packit.config.JwtLoginConfig +import packit.config.JwtPolicy +import packit.exceptions.PackitException +import packit.model.User +import packit.model.dto.LoginWithToken +import packit.security.profile.UserPrincipal +import packit.security.provider.TokenProvider +import packit.service.JwtLoginService +import packit.service.UserService +import packit.testing.TestJwtIssuer +import java.time.Duration +import java.time.Instant +import java.time.temporal.ChronoUnit +import kotlin.test.Test +import kotlin.test.assertEquals + +class JwtLoginServiceTest +{ + private val restTemplate = RestTemplate() + private val server = MockRestServiceServer.bindTo(restTemplate).ignoreExpectOrder(true).build() + + private val serviceUser = User( + username = "service", + userSource = "service", + displayName = null, + email = null, + disabled = false, + ) + + private val serviceUserPrincipal = UserPrincipal( + name = "service", + displayName = "Service Account", + authorities = listOf( + "packet.read", + "packet.run", + "outpack.read", + "outpack.write", + "user.manage", + ).map { SimpleGrantedAuthority(it) }.toMutableList(), + attributes = mutableMapOf(), + ) + + private val mockAppConfig = mock() { + on { authJWTSecret } doAnswer { "secret" } + on { authExpiryDays } doAnswer { 1 } + } + private val mockUserService = mock() { + on { getServiceUser() } doAnswer { serviceUser } + on { getUserPrincipal(serviceUser) } doAnswer { serviceUserPrincipal } + } + + lateinit var defaultIssuer: TestJwtIssuer + + // Create a TestJwtIssuer and register its JWK set on the mock rest template. + fun createIssuer(url: String): TestJwtIssuer { + val issuer = TestJwtIssuer() + server.expect(ExpectedCount.between(0, Integer.MAX_VALUE), requestTo(url)) + .andExpect(method(HttpMethod.GET)) + .andRespond(withSuccess(issuer.jwkSet.toString(), MediaType.APPLICATION_JSON)) + return issuer + } + + fun exchangeTokens(config: JwtLoginConfig, token: String): Jwt { + val sut = JwtLoginService(TokenProvider(mockAppConfig), mockUserService, config, restTemplate) + val result = sut.authenticateAndIssueToken(LoginWithToken(token)) + + val processor = object : DefaultJWTProcessor() { + // The default implementation enforces signature verification, but in this + // testing context we don't care. + override fun process(jwt: SignedJWT, context: SecurityContext?) = jwt.getJWTClaimsSet() + } + return NimbusJwtDecoder(processor).decode(result["token"]!!) + } + + fun exchangeTokens(config: JwtLoginConfig, f: (JwtClaimsSet.Builder) -> Unit): Jwt { + return exchangeTokens(config, defaultIssuer.issue(f)) + } + + @BeforeEach + fun setup() { + defaultIssuer = createIssuer("http://issuer/jwks.json") + } + + @AfterEach + fun reset() { + server.reset() + } + + @Test + fun `can exchange tokens`() + { + val config = JwtLoginConfig( + audience = "packit", + policy = listOf(JwtPolicy(jwkSetURI = "http://issuer/jwks.json", issuer = "issuer")) + ) + + exchangeTokens(config, { builder -> + builder.issuer("issuer") + builder.audience(listOf("packit")) + }) + } + + @Test + fun `provided token must satisfy all required claims`() + { + val requiredIssuer = "issuer" + val requiredAudience = "packit" + data class TestCase( + val providedClaims: Map = mapOf(), + val providedIssuer: String? = requiredIssuer, + val providedAudience: List? = listOf(requiredAudience), + val requiredClaims: Map = mapOf(), + val expectSuccess: Boolean, + ) + val testCases = listOf( + TestCase(providedAudience = listOf("packit"), expectSuccess = true), + TestCase(providedAudience = listOf("packit", "other"), expectSuccess = true), + TestCase(providedAudience = listOf("other", "packit"), expectSuccess = true), + + TestCase(providedAudience = null, expectSuccess = false), + TestCase(providedAudience = listOf(), expectSuccess = false), + TestCase(providedAudience = listOf("other1", "other2"), expectSuccess = false), + TestCase(providedAudience = listOf("other"), expectSuccess = false), + + TestCase(providedIssuer = "issuer", expectSuccess = true), + TestCase(providedIssuer = null, expectSuccess = false), + TestCase(providedIssuer = "wrong-issuer", expectSuccess = false), + + TestCase( + requiredClaims = mapOf("repository" to "mrc-ide/packit"), + providedClaims = mapOf("repository" to "mrc-ide/packit"), + expectSuccess = true, + ), + TestCase( + requiredClaims = mapOf("repository" to "mrc-ide/packit"), + providedClaims = mapOf("repository" to "mrc-ide/not-packit"), + expectSuccess = false, + ), + TestCase( + requiredClaims = mapOf("repository" to "mrc-ide/packit"), + providedClaims = mapOf(), + expectSuccess = false, + ), + TestCase( + requiredClaims = mapOf("repository" to "mrc-ide/packit"), + providedClaims = mapOf("repository" to listOf("mrc-ide/not-packit")), + expectSuccess = false, + ), + TestCase( + requiredClaims = mapOf("owner" to "mrc-ide", "repository" to "packit"), + providedClaims = mapOf("owner" to "mrc-ide", "repository" to "packit"), + expectSuccess = true, + ), + TestCase( + requiredClaims = mapOf("owner" to "mrc-ide", "repository" to "packit"), + providedClaims = mapOf("owner" to "mrc-ide"), + expectSuccess = false, + ), + TestCase( + requiredClaims = mapOf("owner" to "mrc-ide", "repository" to "packit"), + providedClaims = mapOf("repository" to "packit"), + expectSuccess = false, + ), + ) + + for (entry in testCases) { + val config = JwtLoginConfig( + audience = requiredAudience, + policy = listOf( + JwtPolicy( + jwkSetURI = "http://issuer/jwks.json", + issuer = requiredIssuer, + requiredClaims = entry.requiredClaims, + ) + ) + ) + + val fn = { builder: JwtClaimsSet.Builder -> + if (entry.providedIssuer != null) { + builder.issuer(entry.providedIssuer) + } + if (entry.providedAudience != null) { + builder.audience(entry.providedAudience) + } + for ((name, value) in entry.providedClaims) { + builder.claim(name, value) + } + } + + if (entry.expectSuccess) { + assertDoesNotThrow("$entry") { + exchangeTokens(config, fn) + } + } else { + val ex = assertThrows("$entry") { + exchangeTokens(config, fn) + } + assertEquals(ex.key, "externalJwtTokenInvalid") + assertEquals(ex.httpStatus, HttpStatus.UNAUTHORIZED) + } + } + } + + @Test + fun `issued token is granted permissions from the policy`() + { + class TestCase( + val policyPermissions: List, + val expectedPermissions: Set, + ) + val testCases = listOf( + TestCase(listOf(), setOf()), + TestCase(listOf("outpack.read"), setOf("outpack.read")), + TestCase(listOf("outpack.read", "outpack.write"), setOf("outpack.read", "outpack.write")), + TestCase(listOf("not.a.real.permission"), setOf()), + + // TODO: this currently returns an empty set, because the JwtIssuer doesn't support narrowing permissions + // from a global permission to a fine-grained one. + TestCase(listOf("packet.read:packetGroup:packet-name"), setOf()) + ) + + for (entry in testCases) { + val config = JwtLoginConfig( + audience = "packit", + policy = listOf( + JwtPolicy( + jwkSetURI = "http://issuer/jwks.json", + issuer = "issuer", + grantedPermissions = entry.policyPermissions, + ) + ) + ) + val token = exchangeTokens(config, { builder -> + builder.issuer("issuer") + builder.audience(listOf("packit")) + }) + assertEquals(token.getClaimAsStringList("au").toSet(), entry.expectedPermissions) + } + } + + @Test + fun `issued tokens have duration limited by shortest of application configuration, policy and provided token`() + { + whenever(mockAppConfig.authExpiryDays).thenReturn(7) + + class TestCase( + val policyDuration: Duration? = null, + val tokenDuration: Duration? = null, + val expectedDuration: Duration + ) + + val testCases = listOf( + TestCase( + policyDuration = null, + expectedDuration = Duration.ofDays(7) + ), + TestCase( + policyDuration = Duration.ofDays(365), + expectedDuration = Duration.ofDays(7) + ), + TestCase( + policyDuration = Duration.ofMinutes(60), + expectedDuration = Duration.ofMinutes(60) + ), + TestCase( + tokenDuration = Duration.ofMinutes(30), + policyDuration = Duration.ofMinutes(60), + expectedDuration = Duration.ofMinutes(30), + ), + TestCase( + tokenDuration = Duration.ofMinutes(60), + policyDuration = Duration.ofMinutes(30), + expectedDuration = Duration.ofMinutes(30), + ), + TestCase( + tokenDuration = Duration.ofDays(365), + expectedDuration = Duration.ofDays(7), + ), + ) + + for (entry in testCases) { + val config = JwtLoginConfig( + audience = "packit", + policy = listOf( + JwtPolicy( + jwkSetURI = "http://issuer/jwks.json", + issuer = "issuer", + tokenDuration = entry.policyDuration + ) + ) + ) + + val start = Instant.now().truncatedTo(ChronoUnit.SECONDS) + val token = exchangeTokens(config, { builder -> + builder.issuer("issuer") + builder.audience(listOf("packit")) + if (entry.tokenDuration != null) { + builder.expiresAt(Instant.now() + entry.tokenDuration) + } + }) + val end = Instant.now() + + val lifespan = Duration.between(token.issuedAt, token.expiresAt) + + assertThat(token.issuedAt).isBetween(start, end) + assertEquals(lifespan, entry.expectedDuration) + } + } + + @Test + fun `can define multiple policies for the same issuer`() + { + val config = JwtLoginConfig( + audience = "packit", + policy = listOf( + JwtPolicy( + jwkSetURI = "http://issuer/jwks.json", + issuer = "issuer", + requiredClaims = mapOf("name" to "read-only"), + grantedPermissions = listOf("outpack.read"), + ), + JwtPolicy( + jwkSetURI = "http://issuer/jwks.json", + issuer = "issuer", + requiredClaims = mapOf("name" to "read-write"), + grantedPermissions = listOf("outpack.read", "outpack.write"), + ) + ) + ) + + val readToken = exchangeTokens(config, { builder -> + builder.issuer("issuer") + builder.audience(listOf("packit")) + builder.claim("name", "read-only") + }) + val writeToken = exchangeTokens(config, { builder -> + builder.issuer("issuer") + builder.audience(listOf("packit")) + builder.claim("name", "read-write") + }) + + assertEquals(readToken.getClaimAsStringList("au"), listOf("outpack.read")) + assertEquals(writeToken.getClaimAsStringList("au"), listOf("outpack.read", "outpack.write")) + } + + @Test + fun `can define multiple issuers`() + { + val issuer1 = createIssuer("http://issuer1/jwks.json") + val issuer2 = createIssuer("http://issuer2/jwks.json") + + val config = JwtLoginConfig( + audience = "packit", + policy = listOf( + JwtPolicy( + jwkSetURI = "http://issuer1/jwks.json", + issuer = "issuer1", + grantedPermissions = listOf("outpack.read"), + ), + JwtPolicy( + jwkSetURI = "http://issuer2/jwks.json", + issuer = "issuer2", + grantedPermissions = listOf("outpack.read", "outpack.write"), + ) + ) + ) + + val readToken = exchangeTokens( + config, + issuer1.issue { builder -> + builder.issuer("issuer1") + builder.audience(listOf("packit")) + } + ) + val writeToken = exchangeTokens( + config, + issuer2.issue { builder -> + builder.issuer("issuer2") + builder.audience(listOf("packit")) + } + ) + + assertEquals(readToken.getClaimAsStringList("au"), listOf("outpack.read")) + assertEquals(writeToken.getClaimAsStringList("au"), listOf("outpack.read", "outpack.write")) + } + + @Test + fun `throws unauthorized if token is signed with wrong key`() + { + val untrustedIssuer = createIssuer("http://issuer/jwks.json") + + val config = JwtLoginConfig( + audience = "packit", + policy = listOf( + JwtPolicy( + jwkSetURI = "http://issuer/jwks.json", + issuer = "issuer", + grantedPermissions = listOf("outpack.read"), + ), + ) + ) + + val ex = assertThrows { + exchangeTokens( + config, + untrustedIssuer.issue { builder -> + builder.issuer("issuer") + builder.audience(listOf("packit")) + } + ) + } + assertEquals(ex.key, "externalJwtTokenInvalid") + assertEquals(ex.httpStatus, HttpStatus.UNAUTHORIZED) + } +} diff --git a/app/src/types.ts b/app/src/types.ts index ca4e61bb..59a62477 100644 --- a/app/src/types.ts +++ b/app/src/types.ts @@ -94,7 +94,6 @@ export interface FileMetadata { export interface PacketJwtPayload extends JwtPayload { displayName: string; userName: string; - datetime: number; au: string[]; } diff --git a/docs/auth.md b/docs/auth.md index 2314453f..55a610fc 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -113,3 +113,41 @@ The GitApiLoginService class is invoked by the LoginController to: as a Bearer token in the Authorization header in all subsequent requests to the API. - return an error response if authentication requirements not satisfied, e.g. empty PAT provided or user not member of an authorized org. + +### External JWT Authentication + +In addition to interactive authentication for humans, Packit support authentication using a JWT from an external +provider. The client must exchange this 3rd-party token for a Packit-issued JWT, which may be used in the application. + +This scenario is useful for automated jobs that need to connect to Packit, such as +[GitHub Actions workflows][github-oidc] or [Buildkite pipelines][buildkite-oidc]. In either case, the continuous +integration environment provides a way for the job to obtain a signed JWT identifying it. Packit verifies the JWT +against a list of policies to determine whether the token is accepted. + +These are the properties that configure the external JWT authentication: +- `auth.externalJwt.audience`: the required `aud` claim. If this property is missing, authentication from external JWTs is disabled. +- `auth.externalJwt.policy`: the list of policies +- `auth.externalJwt.policy[i].jwk-set-uri`: the URL to the set of public keys used by the issuer +- `auth.externalJwt.policy[i].issuer`: the required `iss` claim +- `auth.externalJwt.policy[i].required-claims.${name}`: custom required claims +- `auth.externalJwt.policy[i].granted-permissions`: scopes that are granted through the authentication process +- `auth.externalJwt.policy[i].token-duration`: the lifetime of tokens issued in exchange + +The claims which may be referenced by the `requiredClaims` property depend on the token issuer. For example, GitHub +provides claims such as `repository`, `repository_owner`, `workflow`, `event_name` or `ref` (the Git reference). +Buildkite provides claims such as `organization_slug`, `pipeline_slug` or `build_branch`. At the moment, only +string-valued claims are supported. + +For example, the following configuration snippet would allow any GitHub actions workflow running in the `mrc-ide/packit` +repository to read and write packets. + +``` +auth.external-jwt.audience=https://packit.dide.ic.ac.uk/reside +auth.external-jwt.policy[0].issuer=https://token.actions.githubusercontent.com +auth.external-jwt.policy[0].jwk-set-uri=https://token.actions.githubusercontent.com/.well-known/jwks +auth.external-jwt.policy[0].required-claims.repository=mrc-ide/packit +auth.external-jwt.policy[0].granted-permissions=outpack.read,outpack.write +``` + +[github-oidc]: https://docs.github.com/en/actions/security-for-github-actions/security-hardening-your-deployments/about-security-hardening-with-openid-connect +[buildkite-oidc]: https://buildkite.com/docs/pipelines/security/oidc From d8ce99d65d5a68b539d9813054ad883816411091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Li=C3=A9tar?= Date: Thu, 26 Sep 2024 12:21:15 +0100 Subject: [PATCH 2/7] Code review --- .../kotlin/packit/config/JwtLoginConfig.kt | 2 +- .../packit/controllers/LoginController.kt | 4 +- .../packit/security/provider/TokenProvider.kt | 90 +++++++++---------- .../kotlin/packit/service/JwtLoginService.kt | 3 +- .../controllers/LoginControllerTest.kt | 6 +- .../unit/service/JwtLoginServiceTest.kt | 14 +-- docs/auth.md | 22 ++--- 7 files changed, 71 insertions(+), 70 deletions(-) diff --git a/api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt b/api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt index 97c6db47..61638596 100644 --- a/api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt +++ b/api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt @@ -18,5 +18,5 @@ data class JwtPolicy( @ConfigurationProperties(prefix = "auth.external-jwt") data class JwtLoginConfig( val audience: String?, - val policy: List = listOf() + val policies: List = listOf() ) diff --git a/api/app/src/main/kotlin/packit/controllers/LoginController.kt b/api/app/src/main/kotlin/packit/controllers/LoginController.kt index 96e01933..2172a47d 100644 --- a/api/app/src/main/kotlin/packit/controllers/LoginController.kt +++ b/api/app/src/main/kotlin/packit/controllers/LoginController.kt @@ -58,7 +58,7 @@ class LoginController( @RequestBody @Validated user: LoginWithToken, ): ResponseEntity> { - if (jwtLoginService.audience == null) { + if (!jwtLoginService.isEnabled()) { throw PackitException("jwtLoginDisabled", HttpStatus.FORBIDDEN) } @@ -70,7 +70,7 @@ class LoginController( @ResponseBody fun jwtAudience(): ResponseEntity> { - if (jwtLoginService.audience == null) { + if (!jwtLoginService.isEnabled()) { throw PackitException("jwtLoginDisabled", HttpStatus.FORBIDDEN) } else { return ResponseEntity.ok(mapOf("audience" to jwtLoginService.audience!!)) diff --git a/api/app/src/main/kotlin/packit/security/provider/TokenProvider.kt b/api/app/src/main/kotlin/packit/security/provider/TokenProvider.kt index 82ebd8d3..ec1ac821 100644 --- a/api/app/src/main/kotlin/packit/security/provider/TokenProvider.kt +++ b/api/app/src/main/kotlin/packit/security/provider/TokenProvider.kt @@ -30,68 +30,68 @@ interface JwtIssuer fun builder(userPrincipal: UserPrincipal): JwtBuilder } -@Component -class TokenProvider(val config: AppConfig) : JwtIssuer -{ +class TokenProviderBuilder(val config: AppConfig, val userPrincipal: UserPrincipal) : JwtBuilder { companion object { const val TOKEN_ISSUER = "packit-api" const val TOKEN_AUDIENCE = "packit" } - inner class Builder(val userPrincipal: UserPrincipal) : JwtBuilder { - var duration: Duration = Duration.of(config.authExpiryDays, ChronoUnit.DAYS) - var expiry: Instant? = null - var permissions: MutableSet? = null + private var duration: Duration = Duration.of(config.authExpiryDays, ChronoUnit.DAYS) + private var expiry: Instant? = null + private var permissions: MutableSet? = null - override fun withExpiresAt(expiry: Instant): JwtBuilder { - if (this.expiry == null || expiry > this.expiry) { - this.expiry = expiry - } - return this + override fun withExpiresAt(expiry: Instant): JwtBuilder { + if (this.expiry == null || expiry > this.expiry) { + this.expiry = expiry } + return this + } - override fun withDuration(duration: Duration): JwtBuilder { - if (duration < this.duration) { - this.duration = duration - } - return this + override fun withDuration(duration: Duration): JwtBuilder { + if (duration < this.duration) { + this.duration = duration } + return this + } - override fun withPermissions(permissions: Collection): JwtBuilder { - if (this.permissions == null) { - this.permissions = permissions.toMutableSet() - } else { - this.permissions!!.retainAll(permissions) - } - return this + override fun withPermissions(permissions: Collection): JwtBuilder { + if (this.permissions == null) { + this.permissions = permissions.toMutableSet() + } else { + this.permissions!!.retainAll(permissions) } + return this + } - override fun issue(): String { - val issuedAt = Instant.now() - var expiresAt = issuedAt.plus(duration) - if (expiry != null && expiry!! < expiresAt) { - expiresAt = expiry - } - - val permissions = userPrincipal.authorities.map { it.authority }.toMutableList() - if (this.permissions != null) { - permissions.retainAll(this.permissions!!) - } + override fun issue(): String { + val issuedAt = Instant.now() + var expiresAt = issuedAt.plus(duration) + if (expiry != null && expiry!! < expiresAt) { + expiresAt = expiry + } - return JWT.create() - .withAudience(TOKEN_AUDIENCE) - .withIssuer(TOKEN_ISSUER) - .withClaim("userName", userPrincipal.name) - .withClaim("displayName", userPrincipal.displayName) - .withClaim("au", permissions) - .withIssuedAt(issuedAt) - .withExpiresAt(expiresAt) - .sign(Algorithm.HMAC256(config.authJWTSecret)) + val permissions = userPrincipal.authorities.map { it.authority }.toMutableList() + if (this.permissions != null) { + permissions.retainAll(this.permissions!!) } + + return JWT.create() + .withAudience(TOKEN_AUDIENCE) + .withIssuer(TOKEN_ISSUER) + .withClaim("userName", userPrincipal.name) + .withClaim("displayName", userPrincipal.displayName) + .withClaim("au", permissions) + .withIssuedAt(issuedAt) + .withExpiresAt(expiresAt) + .sign(Algorithm.HMAC256(config.authJWTSecret)) } +} +@Component +class TokenProvider(val config: AppConfig) : JwtIssuer +{ override fun builder(userPrincipal: UserPrincipal): JwtBuilder { - return Builder(userPrincipal) + return TokenProviderBuilder(config, userPrincipal) } } diff --git a/api/app/src/main/kotlin/packit/service/JwtLoginService.kt b/api/app/src/main/kotlin/packit/service/JwtLoginService.kt index 77101dcd..2954cb24 100644 --- a/api/app/src/main/kotlin/packit/service/JwtLoginService.kt +++ b/api/app/src/main/kotlin/packit/service/JwtLoginService.kt @@ -35,13 +35,14 @@ class JwtLoginService( lateinit var policies: List val audience: String? = jwtLoginConfig.audience + fun isEnabled(): Boolean = audience != null && !audience!!.isEmpty() init { if (audience != null) { val audValidator = JwtClaimValidator>( IdTokenClaimNames.AUD, { claimValue -> claimValue != null && claimValue.contains(audience) } ) - policies = jwtLoginConfig.policy.map { policy -> + policies = jwtLoginConfig.policies.map { policy -> val validators = mutableListOf( JwtTimestampValidator(), JwtIssuerValidator(policy.issuer), diff --git a/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt index 43e2674f..74730281 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt @@ -158,9 +158,9 @@ class LoginControllerTestBasic : IntegrationTest() @TestPropertySource( properties = [ "auth.external-jwt.audience=packit", - "auth.external-jwt.policy[0].jwk-set-uri=http://127.0.0.1:8787/jwks.json", - "auth.external-jwt.policy[0].issuer=issuer", - "auth.external-jwt.policy[0].granted-permissions=outpack.read,outpack.write", + "auth.external-jwt.policies[0].jwk-set-uri=http://127.0.0.1:8787/jwks.json", + "auth.external-jwt.policies[0].issuer=issuer", + "auth.external-jwt.policies[0].granted-permissions=outpack.read,outpack.write", ] ) class LoginControllerTestJwt(val jwksServer: ClientAndServer) : IntegrationTest() diff --git a/api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt index 1a3e14ab..ce383aca 100644 --- a/api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt @@ -115,7 +115,7 @@ class JwtLoginServiceTest { val config = JwtLoginConfig( audience = "packit", - policy = listOf(JwtPolicy(jwkSetURI = "http://issuer/jwks.json", issuer = "issuer")) + policies = listOf(JwtPolicy(jwkSetURI = "http://issuer/jwks.json", issuer = "issuer")) ) exchangeTokens(config, { builder -> @@ -190,7 +190,7 @@ class JwtLoginServiceTest for (entry in testCases) { val config = JwtLoginConfig( audience = requiredAudience, - policy = listOf( + policies = listOf( JwtPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = requiredIssuer, @@ -246,7 +246,7 @@ class JwtLoginServiceTest for (entry in testCases) { val config = JwtLoginConfig( audience = "packit", - policy = listOf( + policies = listOf( JwtPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = "issuer", @@ -305,7 +305,7 @@ class JwtLoginServiceTest for (entry in testCases) { val config = JwtLoginConfig( audience = "packit", - policy = listOf( + policies = listOf( JwtPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = "issuer", @@ -336,7 +336,7 @@ class JwtLoginServiceTest { val config = JwtLoginConfig( audience = "packit", - policy = listOf( + policies = listOf( JwtPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = "issuer", @@ -375,7 +375,7 @@ class JwtLoginServiceTest val config = JwtLoginConfig( audience = "packit", - policy = listOf( + policies = listOf( JwtPolicy( jwkSetURI = "http://issuer1/jwks.json", issuer = "issuer1", @@ -415,7 +415,7 @@ class JwtLoginServiceTest val config = JwtLoginConfig( audience = "packit", - policy = listOf( + policies = listOf( JwtPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = "issuer", diff --git a/docs/auth.md b/docs/auth.md index 55a610fc..a6b7a404 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -125,13 +125,13 @@ integration environment provides a way for the job to obtain a signed JWT identi against a list of policies to determine whether the token is accepted. These are the properties that configure the external JWT authentication: -- `auth.externalJwt.audience`: the required `aud` claim. If this property is missing, authentication from external JWTs is disabled. -- `auth.externalJwt.policy`: the list of policies -- `auth.externalJwt.policy[i].jwk-set-uri`: the URL to the set of public keys used by the issuer -- `auth.externalJwt.policy[i].issuer`: the required `iss` claim -- `auth.externalJwt.policy[i].required-claims.${name}`: custom required claims -- `auth.externalJwt.policy[i].granted-permissions`: scopes that are granted through the authentication process -- `auth.externalJwt.policy[i].token-duration`: the lifetime of tokens issued in exchange +- `auth.external-jwt.audience`: the required `aud` claim. If this property is missing or empty, authentication from external JWTs is disabled. +- `auth.external-jwt.policies`: the list of policies +- `auth.external-jwt.policies[i].jwk-set-uri`: the URL to the set of public keys used by the issuer +- `auth.external-jwt.policies[i].issuer`: the required `iss` claim +- `auth.external-jwt.policies[i].required-claims.${name}`: custom required claims +- `auth.external-jwt.policies[i].granted-permissions`: scopes that are granted through the authentication process +- `auth.external-jwt.policies[i].token-duration`: the lifetime of tokens issued in exchange The claims which may be referenced by the `requiredClaims` property depend on the token issuer. For example, GitHub provides claims such as `repository`, `repository_owner`, `workflow`, `event_name` or `ref` (the Git reference). @@ -143,10 +143,10 @@ repository to read and write packets. ``` auth.external-jwt.audience=https://packit.dide.ic.ac.uk/reside -auth.external-jwt.policy[0].issuer=https://token.actions.githubusercontent.com -auth.external-jwt.policy[0].jwk-set-uri=https://token.actions.githubusercontent.com/.well-known/jwks -auth.external-jwt.policy[0].required-claims.repository=mrc-ide/packit -auth.external-jwt.policy[0].granted-permissions=outpack.read,outpack.write +auth.external-jwt.policies[0].issuer=https://token.actions.githubusercontent.com +auth.external-jwt.policies[0].jwk-set-uri=https://token.actions.githubusercontent.com/.well-known/jwks +auth.external-jwt.policies[0].required-claims.repository=mrc-ide/packit +auth.external-jwt.policies[0].granted-permissions=outpack.read,outpack.write ``` [github-oidc]: https://docs.github.com/en/actions/security-for-github-actions/security-hardening-your-deployments/about-security-hardening-with-openid-connect From d3508241e1f93e3209b66e70656705e92172b0f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Li=C3=A9tar?= Date: Thu, 26 Sep 2024 13:16:09 +0100 Subject: [PATCH 3/7] Rename Jwt to Service. --- ...wtLoginConfig.kt => ServiceLoginConfig.kt} | 8 ++-- .../packit/controllers/LoginController.kt | 22 ++++----- ...LoginService.kt => ServiceLoginService.kt} | 20 ++++---- ...viceTest.kt => ServiceLoginServiceTest.kt} | 46 +++++++++---------- .../packit/unit/service/UserServiceTest.kt | 2 +- 5 files changed, 49 insertions(+), 49 deletions(-) rename api/app/src/main/kotlin/packit/config/{JwtLoginConfig.kt => ServiceLoginConfig.kt} (74%) rename api/app/src/main/kotlin/packit/service/{JwtLoginService.kt => ServiceLoginService.kt} (85%) rename api/app/src/test/kotlin/packit/unit/service/{JwtLoginServiceTest.kt => ServiceLoginServiceTest.kt} (93%) diff --git a/api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt b/api/app/src/main/kotlin/packit/config/ServiceLoginConfig.kt similarity index 74% rename from api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt rename to api/app/src/main/kotlin/packit/config/ServiceLoginConfig.kt index 61638596..af564563 100644 --- a/api/app/src/main/kotlin/packit/config/JwtLoginConfig.kt +++ b/api/app/src/main/kotlin/packit/config/ServiceLoginConfig.kt @@ -5,7 +5,7 @@ import org.springframework.boot.convert.DurationUnit import java.time.Duration import java.time.temporal.ChronoUnit -data class JwtPolicy( +data class ServiceLoginPolicy( val jwkSetURI: String, val issuer: String, val requiredClaims: Map = mapOf(), @@ -15,8 +15,8 @@ data class JwtPolicy( val tokenDuration: Duration? = null, ) -@ConfigurationProperties(prefix = "auth.external-jwt") -data class JwtLoginConfig( +@ConfigurationProperties(prefix = "auth.service") +data class ServiceLoginConfig( val audience: String?, - val policies: List = listOf() + val policies: List = listOf() ) diff --git a/api/app/src/main/kotlin/packit/controllers/LoginController.kt b/api/app/src/main/kotlin/packit/controllers/LoginController.kt index 2172a47d..f3ca66fe 100644 --- a/api/app/src/main/kotlin/packit/controllers/LoginController.kt +++ b/api/app/src/main/kotlin/packit/controllers/LoginController.kt @@ -11,7 +11,7 @@ import packit.model.dto.LoginWithToken import packit.model.dto.UpdatePassword import packit.service.BasicLoginService import packit.service.GithubAPILoginService -import packit.service.JwtLoginService +import packit.service.ServiceLoginService import packit.service.UserService @RestController @@ -19,7 +19,7 @@ import packit.service.UserService class LoginController( val gitApiLoginService: GithubAPILoginService, val basicLoginService: BasicLoginService, - val jwtLoginService: JwtLoginService, + val serviceLoginService: ServiceLoginService, val config: AppConfig, val userService: UserService, ) @@ -52,28 +52,28 @@ class LoginController( return ResponseEntity.ok(token) } - @PostMapping("/login/jwt") + @PostMapping("/login/service") @ResponseBody fun loginJWT( @RequestBody @Validated user: LoginWithToken, ): ResponseEntity> { - if (!jwtLoginService.isEnabled()) { - throw PackitException("jwtLoginDisabled", HttpStatus.FORBIDDEN) + if (!serviceLoginService.isEnabled()) { + throw PackitException("serviceLoginDisabled", HttpStatus.FORBIDDEN) } - val token = jwtLoginService.authenticateAndIssueToken(user) + val token = serviceLoginService.authenticateAndIssueToken(user) return ResponseEntity.ok(token) } - @GetMapping("/login/jwt/audience") + @GetMapping("/login/service/audience") @ResponseBody - fun jwtAudience(): ResponseEntity> + fun serviceAudience(): ResponseEntity> { - if (!jwtLoginService.isEnabled()) { - throw PackitException("jwtLoginDisabled", HttpStatus.FORBIDDEN) + if (!serviceLoginService.isEnabled()) { + throw PackitException("serviceLoginDisabled", HttpStatus.FORBIDDEN) } else { - return ResponseEntity.ok(mapOf("audience" to jwtLoginService.audience!!)) + return ResponseEntity.ok(mapOf("audience" to serviceLoginService.audience!!)) } } diff --git a/api/app/src/main/kotlin/packit/service/JwtLoginService.kt b/api/app/src/main/kotlin/packit/service/ServiceLoginService.kt similarity index 85% rename from api/app/src/main/kotlin/packit/service/JwtLoginService.kt rename to api/app/src/main/kotlin/packit/service/ServiceLoginService.kt index 2954cb24..75122911 100644 --- a/api/app/src/main/kotlin/packit/service/JwtLoginService.kt +++ b/api/app/src/main/kotlin/packit/service/ServiceLoginService.kt @@ -13,8 +13,8 @@ import org.springframework.security.oauth2.jwt.NimbusJwtDecoder import org.springframework.stereotype.Component import org.springframework.web.client.RestOperations import org.springframework.web.client.RestTemplate -import packit.config.JwtLoginConfig -import packit.config.JwtPolicy +import packit.config.ServiceLoginConfig +import packit.config.ServiceLoginPolicy import packit.exceptions.PackitException import packit.model.dto.LoginWithToken import packit.security.provider.JwtIssuer @@ -22,19 +22,19 @@ import java.util.function.Predicate data class TokenPolicy( val decoder: NimbusJwtDecoder, - val config: JwtPolicy, + val config: ServiceLoginPolicy, ) @Component -class JwtLoginService( +class ServiceLoginService( val jwtIssuer: JwtIssuer, val userService: UserService, - val jwtLoginConfig: JwtLoginConfig, + val serviceLoginConfig: ServiceLoginConfig, val restOperations: RestOperations = RestTemplate(), ) { - lateinit var policies: List + private lateinit var policies: List - val audience: String? = jwtLoginConfig.audience + val audience: String? = serviceLoginConfig.audience fun isEnabled(): Boolean = audience != null && !audience!!.isEmpty() init { @@ -42,7 +42,7 @@ class JwtLoginService( val audValidator = JwtClaimValidator>( IdTokenClaimNames.AUD, { claimValue -> claimValue != null && claimValue.contains(audience) } ) - policies = jwtLoginConfig.policies.map { policy -> + policies = serviceLoginConfig.policies.map { policy -> val validators = mutableListOf( JwtTimestampValidator(), JwtIssuerValidator(policy.issuer), @@ -65,7 +65,7 @@ class JwtLoginService( } } - private fun issueToken(verifiedToken: Jwt, policy: JwtPolicy): String { + private fun issueToken(verifiedToken: Jwt, policy: ServiceLoginPolicy): String { val user = userService.getServiceUser() val userPrincipal = userService.getUserPrincipal(user) val tokenBuilder = jwtIssuer.builder(userPrincipal) @@ -99,6 +99,6 @@ class JwtLoginService( companion object { - private val log = LoggerFactory.getLogger(JwtLoginService::class.java) + private val log = LoggerFactory.getLogger(ServiceLoginService::class.java) } } diff --git a/api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/ServiceLoginServiceTest.kt similarity index 93% rename from api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt rename to api/app/src/test/kotlin/packit/unit/service/ServiceLoginServiceTest.kt index ce383aca..c5ea5536 100644 --- a/api/app/src/test/kotlin/packit/unit/service/JwtLoginServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/ServiceLoginServiceTest.kt @@ -22,14 +22,14 @@ import org.springframework.test.web.client.response.DefaultResponseCreator.* import org.springframework.test.web.client.response.MockRestResponseCreators.* import org.springframework.web.client.RestTemplate import packit.AppConfig -import packit.config.JwtLoginConfig -import packit.config.JwtPolicy +import packit.config.ServiceLoginConfig +import packit.config.ServiceLoginPolicy import packit.exceptions.PackitException import packit.model.User import packit.model.dto.LoginWithToken import packit.security.profile.UserPrincipal import packit.security.provider.TokenProvider -import packit.service.JwtLoginService +import packit.service.ServiceLoginService import packit.service.UserService import packit.testing.TestJwtIssuer import java.time.Duration @@ -38,7 +38,7 @@ import java.time.temporal.ChronoUnit import kotlin.test.Test import kotlin.test.assertEquals -class JwtLoginServiceTest +class ServiceLoginServiceTest { private val restTemplate = RestTemplate() private val server = MockRestServiceServer.bindTo(restTemplate).ignoreExpectOrder(true).build() @@ -84,8 +84,8 @@ class JwtLoginServiceTest return issuer } - fun exchangeTokens(config: JwtLoginConfig, token: String): Jwt { - val sut = JwtLoginService(TokenProvider(mockAppConfig), mockUserService, config, restTemplate) + fun exchangeTokens(config: ServiceLoginConfig, token: String): Jwt { + val sut = ServiceLoginService(TokenProvider(mockAppConfig), mockUserService, config, restTemplate) val result = sut.authenticateAndIssueToken(LoginWithToken(token)) val processor = object : DefaultJWTProcessor() { @@ -96,7 +96,7 @@ class JwtLoginServiceTest return NimbusJwtDecoder(processor).decode(result["token"]!!) } - fun exchangeTokens(config: JwtLoginConfig, f: (JwtClaimsSet.Builder) -> Unit): Jwt { + fun exchangeTokens(config: ServiceLoginConfig, f: (JwtClaimsSet.Builder) -> Unit): Jwt { return exchangeTokens(config, defaultIssuer.issue(f)) } @@ -113,9 +113,9 @@ class JwtLoginServiceTest @Test fun `can exchange tokens`() { - val config = JwtLoginConfig( + val config = ServiceLoginConfig( audience = "packit", - policies = listOf(JwtPolicy(jwkSetURI = "http://issuer/jwks.json", issuer = "issuer")) + policies = listOf(ServiceLoginPolicy(jwkSetURI = "http://issuer/jwks.json", issuer = "issuer")) ) exchangeTokens(config, { builder -> @@ -188,10 +188,10 @@ class JwtLoginServiceTest ) for (entry in testCases) { - val config = JwtLoginConfig( + val config = ServiceLoginConfig( audience = requiredAudience, policies = listOf( - JwtPolicy( + ServiceLoginPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = requiredIssuer, requiredClaims = entry.requiredClaims, @@ -244,10 +244,10 @@ class JwtLoginServiceTest ) for (entry in testCases) { - val config = JwtLoginConfig( + val config = ServiceLoginConfig( audience = "packit", policies = listOf( - JwtPolicy( + ServiceLoginPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = "issuer", grantedPermissions = entry.policyPermissions, @@ -303,10 +303,10 @@ class JwtLoginServiceTest ) for (entry in testCases) { - val config = JwtLoginConfig( + val config = ServiceLoginConfig( audience = "packit", policies = listOf( - JwtPolicy( + ServiceLoginPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = "issuer", tokenDuration = entry.policyDuration @@ -334,16 +334,16 @@ class JwtLoginServiceTest @Test fun `can define multiple policies for the same issuer`() { - val config = JwtLoginConfig( + val config = ServiceLoginConfig( audience = "packit", policies = listOf( - JwtPolicy( + ServiceLoginPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = "issuer", requiredClaims = mapOf("name" to "read-only"), grantedPermissions = listOf("outpack.read"), ), - JwtPolicy( + ServiceLoginPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = "issuer", requiredClaims = mapOf("name" to "read-write"), @@ -373,15 +373,15 @@ class JwtLoginServiceTest val issuer1 = createIssuer("http://issuer1/jwks.json") val issuer2 = createIssuer("http://issuer2/jwks.json") - val config = JwtLoginConfig( + val config = ServiceLoginConfig( audience = "packit", policies = listOf( - JwtPolicy( + ServiceLoginPolicy( jwkSetURI = "http://issuer1/jwks.json", issuer = "issuer1", grantedPermissions = listOf("outpack.read"), ), - JwtPolicy( + ServiceLoginPolicy( jwkSetURI = "http://issuer2/jwks.json", issuer = "issuer2", grantedPermissions = listOf("outpack.read", "outpack.write"), @@ -413,10 +413,10 @@ class JwtLoginServiceTest { val untrustedIssuer = createIssuer("http://issuer/jwks.json") - val config = JwtLoginConfig( + val config = ServiceLoginConfig( audience = "packit", policies = listOf( - JwtPolicy( + ServiceLoginPolicy( jwkSetURI = "http://issuer/jwks.json", issuer = "issuer", grantedPermissions = listOf("outpack.read"), diff --git a/api/app/src/test/kotlin/packit/unit/service/UserServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/UserServiceTest.kt index 4298f5fa..b323a2fa 100644 --- a/api/app/src/test/kotlin/packit/unit/service/UserServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/UserServiceTest.kt @@ -249,7 +249,7 @@ class UserServiceTest val service = BaseUserService(mockUserRepository, mockRoleService, passwordEncoder) - val result = service.createBasicUser( + service.createBasicUser( CreateBasicUser( email = "email", password = "password", From c1c4b64c765bc4d203cbf77a4cef183006d6729e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Li=C3=A9tar?= Date: Thu, 26 Sep 2024 14:38:07 +0100 Subject: [PATCH 4/7] fix tests --- .../packit/integration/controllers/LoginControllerTest.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt index 74730281..4d95599e 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt @@ -157,10 +157,10 @@ class LoginControllerTestBasic : IntegrationTest() @MockServerSettings(ports = [8787]) @TestPropertySource( properties = [ - "auth.external-jwt.audience=packit", - "auth.external-jwt.policies[0].jwk-set-uri=http://127.0.0.1:8787/jwks.json", - "auth.external-jwt.policies[0].issuer=issuer", - "auth.external-jwt.policies[0].granted-permissions=outpack.read,outpack.write", + "auth.service.audience=packit", + "auth.service.policies[0].jwk-set-uri=http://127.0.0.1:8787/jwks.json", + "auth.service.policies[0].issuer=issuer", + "auth.service.policies[0].granted-permissions=outpack.read,outpack.write", ] ) class LoginControllerTestJwt(val jwksServer: ClientAndServer) : IntegrationTest() From b017d8d1666d798b9b4e5dac0a15625cc0cd9d7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Li=C3=A9tar?= Date: Thu, 26 Sep 2024 15:05:46 +0100 Subject: [PATCH 5/7] fix tests again --- .../integration/controllers/LoginControllerTest.kt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt index 4d95599e..5f77fb5b 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/LoginControllerTest.kt @@ -163,7 +163,7 @@ class LoginControllerTestBasic : IntegrationTest() "auth.service.policies[0].granted-permissions=outpack.read,outpack.write", ] ) -class LoginControllerTestJwt(val jwksServer: ClientAndServer) : IntegrationTest() +class LoginControllerTestService(val jwksServer: ClientAndServer) : IntegrationTest() { val trustedIssuer = TestJwtIssuer() @@ -186,7 +186,7 @@ class LoginControllerTestJwt(val jwksServer: ClientAndServer) : IntegrationTest( @Test fun `can get expected audience`() { - val result = restTemplate.getForEntity("/auth/login/jwt/audience", JsonNode::class.java) + val result = restTemplate.getForEntity("/auth/login/service/audience", JsonNode::class.java) assertSuccess(result) assertEquals(result.body?.required("audience")?.textValue(), "packit") } @@ -199,7 +199,7 @@ class LoginControllerTestJwt(val jwksServer: ClientAndServer) : IntegrationTest( builder.audience(listOf("packit")) } val result = restTemplate.postForEntity( - "/auth/login/jwt", + "/auth/login/service", LoginWithToken(externalToken), JsonNode::class.java, ) @@ -218,7 +218,11 @@ class LoginControllerTestJwt(val jwksServer: ClientAndServer) : IntegrationTest( builder.issuer("issuer") builder.audience(listOf("packit")) } - val result = restTemplate.postForEntity("/auth/login/jwt", LoginWithToken(externalToken), JsonNode::class.java) + val result = restTemplate.postForEntity( + "/auth/login/service", + LoginWithToken(externalToken), + JsonNode::class.java + ) assertUnauthorized(result) } @@ -229,7 +233,7 @@ class LoginControllerTestJwt(val jwksServer: ClientAndServer) : IntegrationTest( builder.issuer("issuer") builder.audience(listOf("not-packit")) } - val result = restTemplate.postForEntity("/auth/login/jwt", LoginWithToken(token), String::class.java) + val result = restTemplate.postForEntity("/auth/login/service", LoginWithToken(token), String::class.java) assertUnauthorized(result) } } From b932e877b7c9e02f3b6737d2c45272fc6618ba6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Li=C3=A9tar?= Date: Fri, 27 Sep 2024 09:54:46 +0100 Subject: [PATCH 6/7] Update api/app/src/main/kotlin/packit/controllers/LoginController.kt Co-authored-by: Anmol Thapar --- api/app/src/main/kotlin/packit/controllers/LoginController.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/app/src/main/kotlin/packit/controllers/LoginController.kt b/api/app/src/main/kotlin/packit/controllers/LoginController.kt index f3ca66fe..5cdcf0c5 100644 --- a/api/app/src/main/kotlin/packit/controllers/LoginController.kt +++ b/api/app/src/main/kotlin/packit/controllers/LoginController.kt @@ -54,7 +54,7 @@ class LoginController( @PostMapping("/login/service") @ResponseBody - fun loginJWT( + fun loginService( @RequestBody @Validated user: LoginWithToken, ): ResponseEntity> { From 8f2cb21a5d092e0dfd027341ef3332a08a6c1556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Li=C3=A9tar?= Date: Fri, 27 Sep 2024 12:01:22 +0100 Subject: [PATCH 7/7] Make policies a val --- api/app/src/main/kotlin/packit/service/ServiceLoginService.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/app/src/main/kotlin/packit/service/ServiceLoginService.kt b/api/app/src/main/kotlin/packit/service/ServiceLoginService.kt index 75122911..78de07d0 100644 --- a/api/app/src/main/kotlin/packit/service/ServiceLoginService.kt +++ b/api/app/src/main/kotlin/packit/service/ServiceLoginService.kt @@ -32,11 +32,11 @@ class ServiceLoginService( val serviceLoginConfig: ServiceLoginConfig, val restOperations: RestOperations = RestTemplate(), ) { - private lateinit var policies: List - val audience: String? = serviceLoginConfig.audience fun isEnabled(): Boolean = audience != null && !audience!!.isEmpty() + private val policies: List + init { if (audience != null) { val audValidator = JwtClaimValidator>(