diff --git a/src/main/kotlin/com/wire/github/EventsHandler.kt b/src/main/kotlin/com/wire/github/EventsHandler.kt index 7c52803..1eb5d00 100644 --- a/src/main/kotlin/com/wire/github/EventsHandler.kt +++ b/src/main/kotlin/com/wire/github/EventsHandler.kt @@ -5,16 +5,15 @@ import com.wire.sdk.WireEventsHandlerSuspending import com.wire.sdk.model.Conversation import com.wire.sdk.model.ConversationMember import com.wire.sdk.model.WireMessage -import io.lettuce.core.api.StatefulRedisConnection import org.koin.core.context.GlobalContext import org.slf4j.LoggerFactory class EventsHandler : WireEventsHandlerSuspending() { private val logger = LoggerFactory.getLogger(this::class.java) - private val redisConnection = GlobalContext.get().get>() + private val redisService = GlobalContext.get().get() private val usageMetrics = GlobalContext.get().get() private val commandHandler = GitHubCommandHandler( - storage = redisConnection.sync(), + redisService = redisService, usageMetrics = usageMetrics ) diff --git a/src/main/kotlin/com/wire/github/GitHubCommandHandler.kt b/src/main/kotlin/com/wire/github/GitHubCommandHandler.kt index 6ff7118..d4ced02 100644 --- a/src/main/kotlin/com/wire/github/GitHubCommandHandler.kt +++ b/src/main/kotlin/com/wire/github/GitHubCommandHandler.kt @@ -4,13 +4,10 @@ import com.wire.github.metrics.UsageMetrics import com.wire.github.util.ENV_VAR_HOST import com.wire.github.util.ActionsTokenGenerator import com.wire.github.util.SessionIdentifierGenerator -import com.wire.github.util.toActionsTokenStorageKey -import com.wire.github.util.toStorageKey import com.wire.sdk.model.QualifiedId -import io.lettuce.core.api.sync.RedisCommands internal class GitHubCommandHandler( - private val storage: RedisCommands, + private val redisService: RedisService, private val usageMetrics: UsageMetrics, private val host: String = ENV_VAR_HOST, private val generateSecret: () -> String = SessionIdentifierGenerator::generate, @@ -64,15 +61,16 @@ internal class GitHubCommandHandler( response.also { usageMetrics.onHelpCommand() } private fun getOrCreateSecret(conversationId: QualifiedId): String = - getOrCreate(conversationId.toStorageKey(), generateSecret) + redisService.getWebhookSecret(conversationId) + ?: generateSecret().also { + redisService.setWebhookSecret(conversationId, it) + } private fun getOrCreateActionsToken(conversationId: QualifiedId): String = - getOrCreate(conversationId.toActionsTokenStorageKey(), generateActionsToken) - - private fun getOrCreate( - storageKey: String, - generate: () -> String - ): String = storage.get(storageKey) ?: generate().also { storage.set(storageKey, it) } + redisService.getActionSecret(conversationId) + ?: generateActionsToken().also { + redisService.setActionSecret(conversationId, it) + } private val normalizedHost: String get() = host.trimEnd('/') diff --git a/src/main/kotlin/com/wire/github/RedisService.kt b/src/main/kotlin/com/wire/github/RedisService.kt new file mode 100644 index 0000000..e259ee9 --- /dev/null +++ b/src/main/kotlin/com/wire/github/RedisService.kt @@ -0,0 +1,88 @@ +package com.wire.github + +import com.wire.sdk.model.QualifiedId +import io.lettuce.core.api.sync.RedisCommands +import java.io.IOException +import java.nio.charset.StandardCharsets +import java.security.MessageDigest +import java.util.Locale +import java.util.UUID +import javax.crypto.Mac +import javax.crypto.spec.SecretKeySpec + +class RedisService( + private val storage: RedisCommands +) { + fun getWebhookSecret(conversationId: QualifiedId): String? = + storage.get(conversationId.toWebhookStorageKey()) + + fun setWebhookSecret( + conversationId: QualifiedId, + secret: String + ) { + storage.set(conversationId.toWebhookStorageKey(), secret) + } + + fun getActionSecret(conversationId: QualifiedId): String? = + storage.get(conversationId.toActionStorageKey()) + + fun setActionSecret( + conversationId: QualifiedId, + secret: String + ) { + storage.set(conversationId.toActionStorageKey(), secret) + } + + fun isActionsTokenValid( + conversationId: QualifiedId, + token: String + ): Boolean { + val storedToken = getActionSecret(conversationId) ?: return false + return MessageDigest.isEqual( + storedToken.toByteArray(StandardCharsets.UTF_8), + token.toByteArray(StandardCharsets.UTF_8) + ) + } + + @Throws(Exception::class) + fun isWebhookSignatureValid( + conversationId: String, + conversationDomain: String, + signature: String, + payload: String + ): Boolean { + val qualifiedConversationId = QualifiedId( + UUID.fromString(conversationId), + conversationDomain + ) + val secret = getWebhookSecret(qualifiedConversationId) ?: throw IOException( + "Missing secret for Conversation: $conversationId" + ) + val generatedHmacSha1 = generateHmacSha1(payload, secret) + val challenge = String.format(Locale.getDefault(), "sha1=%s", generatedHmacSha1) + + return challenge == signature + } + + internal fun generateHmacSha1( + data: String, + secret: String + ): String { + val keySpec = SecretKeySpec(secret.toByteArray(), HMAC_ALGORITHM) + val mac = Mac.getInstance(HMAC_ALGORITHM) + mac.init(keySpec) + val digest = mac.doFinal(data.toByteArray()) + + return digest.joinToString("") { "%02x".format(it) } + } + + private fun QualifiedId.toWebhookStorageKey() = "$STORAGE_KEY_PREFIX$id@$domain" + + private fun QualifiedId.toActionStorageKey() = "${toWebhookStorageKey()}$ACTION_TOKEN_SUFFIX" + + private companion object { + const val STORAGE_KEY_PREFIX = "github-app:" + const val ACTION_TOKEN_SUFFIX = ":actions-token" + const val HMAC_ALGORITHM = "HmacSHA1" + } +} diff --git a/src/main/kotlin/com/wire/github/Routing.kt b/src/main/kotlin/com/wire/github/Routing.kt index 0d5e678..66293e1 100644 --- a/src/main/kotlin/com/wire/github/Routing.kt +++ b/src/main/kotlin/com/wire/github/Routing.kt @@ -3,9 +3,7 @@ package com.wire.github import com.wire.github.metrics.UsageMetrics import com.wire.github.request.model.ActionsNotificationRequest import com.wire.github.response.model.GitHubResponse -import com.wire.github.util.ActionsTokenValidator import com.wire.github.util.KtxSerializer -import com.wire.github.util.SignatureValidator import com.wire.github.util.TemplateHandler import com.wire.sdk.WireAppSdk import com.wire.sdk.model.QualifiedId @@ -44,8 +42,7 @@ fun Application.configureRouting() { } val wireAppSdk = GlobalContext.get().get() - val actionsTokenValidator = GlobalContext.get().get() - val signatureValidator = GlobalContext.get().get() + val redisService = GlobalContext.get().get() val templateHandler = GlobalContext.get().get() val usageMetrics = GlobalContext.get().get() @@ -86,7 +83,7 @@ fun Application.configureRouting() { // Validation of received signature val isSignatureValid = try { - signatureValidator.isValid( + redisService.isWebhookSignatureValid( conversationId = conversationId, conversationDomain = conversationDomain, signature = signature, @@ -156,7 +153,7 @@ fun Application.configureRouting() { configureActionsRoute( wireAppSdk = wireAppSdk, - actionsTokenValidator = actionsTokenValidator + redisService = redisService ) } } @@ -166,7 +163,7 @@ private const val PARAM_CONVERSATION_DOMAIN = "conversationDomain" private fun Route.configureActionsRoute( wireAppSdk: WireAppSdk, - actionsTokenValidator: ActionsTokenValidator + redisService: RedisService ) { post("/actions/{$PARAM_CONVERSATION_ID}/{$PARAM_CONVERSATION_DOMAIN}") { val conversationId = call.parameters[PARAM_CONVERSATION_ID] @@ -199,7 +196,7 @@ private fun Route.configureActionsRoute( "Bearer authorization is required" ) - if (!actionsTokenValidator.isValid(qualifiedConversationId, bearerToken)) { + if (!redisService.isActionsTokenValid(qualifiedConversationId, bearerToken)) { application.log.warn( "Invalid Actions token for conversation $conversationId@$conversationDomain" ) diff --git a/src/main/kotlin/com/wire/github/config/Modules.kt b/src/main/kotlin/com/wire/github/config/Modules.kt index 7334835..94b1c97 100644 --- a/src/main/kotlin/com/wire/github/config/Modules.kt +++ b/src/main/kotlin/com/wire/github/config/Modules.kt @@ -1,13 +1,12 @@ package com.wire.github.config import com.wire.github.EventsHandler +import com.wire.github.RedisService import com.wire.github.metrics.UsageMetrics -import com.wire.github.util.ActionsTokenValidator import com.wire.github.util.ENV_VAR_API_HOST import com.wire.github.util.ENV_VAR_API_TOKEN import com.wire.github.util.ENV_VAR_CRYPTOGRAPHY_STORAGE_KEY import com.wire.github.util.ENV_VAR_REDIS_URL -import com.wire.github.util.SignatureValidator import com.wire.github.util.TemplateHandler import com.wire.sdk.WireAppSdk import io.ktor.utils.io.core.toByteArray @@ -23,15 +22,10 @@ val projectModules = module { startListening() } } - single { SignatureValidator() } - single { - ActionsTokenValidator( - storage = get>().sync() - ) - } single { TemplateHandler() } single { RedisClient.create(ENV_VAR_REDIS_URL) } single> { get().connect() } + single { RedisService(storage = get>().sync()) } single { PrometheusMeterRegistry(PrometheusConfig.DEFAULT) } single { UsageMetrics(registry = get()) } } diff --git a/src/main/kotlin/com/wire/github/util/ActionsTokenValidator.kt b/src/main/kotlin/com/wire/github/util/ActionsTokenValidator.kt deleted file mode 100644 index 70754b3..0000000 --- a/src/main/kotlin/com/wire/github/util/ActionsTokenValidator.kt +++ /dev/null @@ -1,22 +0,0 @@ -package com.wire.github.util - -import com.wire.sdk.model.QualifiedId -import io.lettuce.core.api.sync.RedisCommands -import java.nio.charset.StandardCharsets -import java.security.MessageDigest - -class ActionsTokenValidator( - private val storage: RedisCommands -) { - fun isValid( - conversationId: QualifiedId, - token: String - ): Boolean { - val storageKey = conversationId.toActionsTokenStorageKey() - val storedToken = storage.get(storageKey) ?: return false - return MessageDigest.isEqual( - storedToken.toByteArray(StandardCharsets.UTF_8), - token.toByteArray(StandardCharsets.UTF_8) - ) - } -} diff --git a/src/main/kotlin/com/wire/github/util/Extensions.kt b/src/main/kotlin/com/wire/github/util/Extensions.kt deleted file mode 100644 index 52cd19a..0000000 --- a/src/main/kotlin/com/wire/github/util/Extensions.kt +++ /dev/null @@ -1,12 +0,0 @@ -package com.wire.github.util - -import com.wire.sdk.model.QualifiedId - -private const val STORAGE_KEY_PREFIX = "github-app:" -private const val ACTIONS_TOKEN_SUFFIX = ":actions-token" - -fun QualifiedId.toStorageKey() = "$STORAGE_KEY_PREFIX${this.id}@${this.domain}" - -fun String.toStorageKey(domain: String) = "$STORAGE_KEY_PREFIX$this@$domain" - -fun QualifiedId.toActionsTokenStorageKey() = "${toStorageKey()}$ACTIONS_TOKEN_SUFFIX" diff --git a/src/main/kotlin/com/wire/github/util/SignatureValidator.kt b/src/main/kotlin/com/wire/github/util/SignatureValidator.kt deleted file mode 100644 index 792b73f..0000000 --- a/src/main/kotlin/com/wire/github/util/SignatureValidator.kt +++ /dev/null @@ -1,50 +0,0 @@ -package com.wire.github.util - -import io.lettuce.core.api.StatefulRedisConnection -import java.io.IOException -import java.util.Locale -import javax.crypto.Mac -import javax.crypto.spec.SecretKeySpec -import org.koin.core.context.GlobalContext - -class SignatureValidator { - private val redisConnection = GlobalContext.get().get>() - private val storage = redisConnection.sync() - - @Throws(Exception::class) - fun isValid( - conversationId: String, - conversationDomain: String, - signature: String, - payload: String - ): Boolean { - val storageKey = conversationId.toStorageKey(domain = conversationDomain) - val secret = storage.get(storageKey) - - if (secret == null) { - throw IOException("Missing secret for Conversation: $conversationId") - } - - val generatedHmacSha1: String = generateHmacSha1(payload, secret) - val challenge = String.format(Locale.getDefault(), "sha1=%s", generatedHmacSha1) - - return challenge == signature - } - - internal fun generateHmacSha1( - data: String, - secret: String - ): String { - val keySpec = SecretKeySpec(secret.toByteArray(), ALGORITHM) - val mac = Mac.getInstance(ALGORITHM) - mac.init(keySpec) - val digest = mac.doFinal(data.toByteArray()) - - // Convert to hex string - return digest.joinToString("") { "%02x".format(it) } - } - - private companion object { - const val ALGORITHM = "HmacSHA1" - } -} diff --git a/src/test/kotlin/com/wire/github/ApplicationTest.kt b/src/test/kotlin/com/wire/github/ApplicationTest.kt index 3db354b..0283193 100644 --- a/src/test/kotlin/com/wire/github/ApplicationTest.kt +++ b/src/test/kotlin/com/wire/github/ApplicationTest.kt @@ -1,8 +1,6 @@ package com.wire.github import com.wire.github.metrics.UsageMetrics -import com.wire.github.util.ActionsTokenValidator -import com.wire.github.util.SignatureValidator import com.wire.github.util.TemplateHandler import com.wire.sdk.WireAppSdk import com.wire.sdk.model.QualifiedId @@ -19,9 +17,6 @@ import kotlin.test.Test import kotlin.test.assertTrue import kotlin.test.assertEquals import io.ktor.client.request.header -import io.lettuce.core.RedisClient -import io.lettuce.core.api.StatefulRedisConnection -import io.lettuce.core.api.sync.RedisCommands import io.micrometer.prometheusmetrics.PrometheusConfig import io.micrometer.prometheusmetrics.PrometheusMeterRegistry import io.mockk.every @@ -40,22 +35,14 @@ import org.koin.dsl.module class ApplicationTest { @BeforeTest fun setupKoin() { - val mockRedisClient = mockk() - val mockRedisCommands = mockk>() - val mockRedisConnection = mockk>() + val redisService = mockk() val mockWireAppSdk = mockk(relaxed = true) - // Configure the Redis connection mock to return the sync commands - every { mockRedisConnection.sync() } returns mockRedisCommands - startKoin { modules( module { - single { SignatureValidator() } - single { ActionsTokenValidator(storage = mockRedisCommands) } + single { redisService } single { TemplateHandler() } - single { mockRedisClient } - single> { mockRedisConnection } single { mockWireAppSdk } single { PrometheusMeterRegistry(PrometheusConfig.DEFAULT) } single { UsageMetrics(registry = get()) } @@ -116,8 +103,8 @@ class ApplicationTest { @Test fun `given a delivery is rendered, when GET metrics, then funnel counters are set`() { - val signatureValidator = mockk() - every { signatureValidator.isValid(any(), any(), any(), any()) } returns true + val redisService = mockk() + every { redisService.isWebhookSignatureValid(any(), any(), any(), any()) } returns true val wireAppSdk = mockk() every { @@ -131,7 +118,7 @@ class ApplicationTest { loadKoinModules( module { - single { signatureValidator } + single { redisService } single { wireAppSdk } single { templateHandler } } @@ -169,8 +156,8 @@ class ApplicationTest { @Test fun `given no template for the event, when GET metrics, then unsupported counter is exposed`() { - val signatureValidator = mockk() - every { signatureValidator.isValid(any(), any(), any(), any()) } returns true + val redisService = mockk() + every { redisService.isWebhookSignatureValid(any(), any(), any(), any()) } returns true val wireAppSdk = mockk(relaxed = true) @@ -181,7 +168,7 @@ class ApplicationTest { loadKoinModules( module { - single { signatureValidator } + single { redisService } single { wireAppSdk } single { templateHandler } } @@ -222,15 +209,9 @@ class ApplicationTest { @Test fun `given received event, when pull_request is created, then validations are passing`() { - val signatureValidator = mockk() - every { - signatureValidator.generateHmacSha1( - data = DUMMY_PAYLOAD, - secret = CONVERSATION_ID.id.toString() - ) - } returns DUMMY_SIGNATURE + val redisService = mockk() every { - signatureValidator.isValid( + redisService.isWebhookSignatureValid( conversationId = CONVERSATION_ID.id.toString(), conversationDomain = CONVERSATION_ID.domain, signature = "sha1=$DUMMY_SIGNATURE", @@ -255,7 +236,7 @@ class ApplicationTest { loadKoinModules( module { - single { signatureValidator } + single { redisService } single { wireAppSdk } single { templateHandler } } @@ -266,20 +247,12 @@ class ApplicationTest { configureRouting() } - val signature = String.format( - "sha1=%s", - signatureValidator.generateHmacSha1( - data = DUMMY_PAYLOAD, - secret = CONVERSATION_ID.id.toString() - ) - ) - // Test the real route with mocked service val response = client.post("/${CONVERSATION_ID.id}/${CONVERSATION_ID.domain}") { contentType(ContentType.Application.Json) header("X-GitHub-Event", DUMMY_EVENT) - header("X-Hub-Signature", signature) + header("X-Hub-Signature", "sha1=$DUMMY_SIGNATURE") header("X-GitHub-Delivery", "delivery") setBody(DUMMY_PAYLOAD) @@ -296,12 +269,12 @@ class ApplicationTest { @Test fun `given invalid signature, when webhook is posted, then forbidden without sending`() { - val signatureValidator = mockk() - every { signatureValidator.isValid(any(), any(), any(), any()) } returns false + val redisService = mockk() + every { redisService.isWebhookSignatureValid(any(), any(), any(), any()) } returns false val wireAppSdk = mockk(relaxed = true) loadKoinModules( module { - single { signatureValidator } + single { redisService } single { wireAppSdk } } ) @@ -325,12 +298,12 @@ class ApplicationTest { @Test fun `given malformed payload, when webhook is posted, then bad request without sending`() { - val signatureValidator = mockk() - every { signatureValidator.isValid(any(), any(), any(), any()) } returns true + val redisService = mockk() + every { redisService.isWebhookSignatureValid(any(), any(), any(), any()) } returns true val wireAppSdk = mockk(relaxed = true) loadKoinModules( module { - single { signatureValidator } + single { redisService } single { wireAppSdk } } ) @@ -354,8 +327,8 @@ class ApplicationTest { @Test fun `given completed GitHub Actions events, when received, then messages are sent`() { - val signatureValidator = mockk() - every { signatureValidator.isValid(any(), any(), any(), any()) } returns true + val redisService = mockk() + every { redisService.isWebhookSignatureValid(any(), any(), any(), any()) } returns true val wireAppSdk = mockk() every { @@ -364,7 +337,7 @@ class ApplicationTest { loadKoinModules( module { - single { signatureValidator } + single { redisService } single { wireAppSdk } } ) @@ -394,7 +367,7 @@ class ApplicationTest { @Test fun `given valid bearer token, when Actions message is posted, then message is sent`() { - val (actionsTokenValidator, wireAppSdk) = loadActionsDependencies(tokenValid = true) + val (redisService, wireAppSdk) = loadActionsDependencies(tokenValid = true) val sentMessage = slot() every { wireAppSdk.getApplicationManager().sendMessage(message = capture(sentMessage)) @@ -413,7 +386,7 @@ class ApplicationTest { assertEquals(CONVERSATION_ID, sentMessage.captured.conversationId) assertEquals("CI failed", sentMessage.captured.text) verify(exactly = 1) { - actionsTokenValidator.isValid(CONVERSATION_ID, ACTIONS_TOKEN) + redisService.isActionsTokenValid(CONVERSATION_ID, ACTIONS_TOKEN) } verify(exactly = 1) { wireAppSdk.getApplicationManager().sendMessage(message = any()) } } @@ -427,7 +400,7 @@ class ApplicationTest { "Bearer ", "Bearer token with-spaces" ).forEach { authorization -> - val (actionsTokenValidator, wireAppSdk) = loadActionsDependencies(tokenValid = true) + val (redisService, wireAppSdk) = loadActionsDependencies(tokenValid = true) testApplication { application { configureRouting() } @@ -441,7 +414,7 @@ class ApplicationTest { assertEquals(HttpStatusCode.Unauthorized, response.status) assertEquals("Bearer authorization is required", response.bodyAsText()) - verify(exactly = 0) { actionsTokenValidator.isValid(any(), any()) } + verify(exactly = 0) { redisService.isActionsTokenValid(any(), any()) } verify(exactly = 0) { wireAppSdk.getApplicationManager().sendMessage(message = any()) } @@ -499,7 +472,7 @@ class ApplicationTest { @Test fun `Actions message with non-json content type is unsupported`() { - val (actionsTokenValidator, wireAppSdk) = loadActionsDependencies(tokenValid = true) + val (redisService, wireAppSdk) = loadActionsDependencies(tokenValid = true) testApplication { application { configureRouting() } @@ -513,7 +486,7 @@ class ApplicationTest { assertEquals(HttpStatusCode.UnsupportedMediaType, response.status) assertEquals("Content-Type must be application/json", response.bodyAsText()) - verify(exactly = 0) { actionsTokenValidator.isValid(any(), any()) } + verify(exactly = 0) { redisService.isActionsTokenValid(any(), any()) } verify(exactly = 0) { wireAppSdk.getApplicationManager().sendMessage(message = any()) } @@ -522,7 +495,7 @@ class ApplicationTest { @Test fun `given invalid conversation ID, when Actions message is posted, then bad request`() { - val (actionsTokenValidator, wireAppSdk) = loadActionsDependencies(tokenValid = true) + val (redisService, wireAppSdk) = loadActionsDependencies(tokenValid = true) testApplication { application { configureRouting() } @@ -534,24 +507,22 @@ class ApplicationTest { assertEquals(HttpStatusCode.BadRequest, response.status) assertEquals("Invalid conversation ID", response.bodyAsText()) - verify(exactly = 0) { actionsTokenValidator.isValid(any(), any()) } + verify(exactly = 0) { redisService.isActionsTokenValid(any(), any()) } verify(exactly = 0) { wireAppSdk.getApplicationManager().sendMessage(message = any()) } } } - private fun loadActionsDependencies( - tokenValid: Boolean - ): Pair { - val actionsTokenValidator = mockk() - every { actionsTokenValidator.isValid(any(), any()) } returns tokenValid + private fun loadActionsDependencies(tokenValid: Boolean): Pair { + val redisService = mockk() + every { redisService.isActionsTokenValid(any(), any()) } returns tokenValid val wireAppSdk = mockk(relaxed = true) loadKoinModules( module { - single { actionsTokenValidator } + single { redisService } single { wireAppSdk } } ) - return actionsTokenValidator to wireAppSdk + return redisService to wireAppSdk } private suspend fun io.ktor.client.HttpClient.postActionsMessage( diff --git a/src/test/kotlin/com/wire/github/GitHubCommandHandlerTest.kt b/src/test/kotlin/com/wire/github/GitHubCommandHandlerTest.kt index 218622c..ab81b34 100644 --- a/src/test/kotlin/com/wire/github/GitHubCommandHandlerTest.kt +++ b/src/test/kotlin/com/wire/github/GitHubCommandHandlerTest.kt @@ -1,10 +1,7 @@ package com.wire.github import com.wire.github.metrics.UsageMetrics -import com.wire.github.util.toActionsTokenStorageKey -import com.wire.github.util.toStorageKey import com.wire.sdk.model.QualifiedId -import io.lettuce.core.api.sync.RedisCommands import io.mockk.every import io.mockk.mockk import io.mockk.verify @@ -14,10 +11,10 @@ import kotlin.test.assertContains import kotlin.test.assertNull class GitHubCommandHandlerTest { - private val storage = mockk>() + private val redisService = mockk() private val usageMetrics = mockk(relaxed = true) private val handler = GitHubCommandHandler( - storage = storage, + redisService = redisService, usageMetrics = usageMetrics, host = "https://github-app.example.com/", generateSecret = { GENERATED_SECRET }, @@ -35,7 +32,7 @@ class GitHubCommandHandlerTest { @Test fun `webhook help includes webhook URL and token`() { - every { storage.get(CONVERSATION_ID.toStorageKey()) } returns EXISTING_SECRET + every { redisService.getWebhookSecret(CONVERSATION_ID) } returns EXISTING_SECRET val response = handler.response(GitHubCommandHandler.WEBHOOK_HELP_COMMAND, CONVERSATION_ID) @@ -50,7 +47,7 @@ class GitHubCommandHandlerTest { @Test fun `Actions help includes endpoint token and reusable action`() { every { - storage.get(CONVERSATION_ID.toActionsTokenStorageKey()) + redisService.getActionSecret(CONVERSATION_ID) } returns EXISTING_ACTIONS_TOKEN val response = handler.response(GitHubCommandHandler.ACTIONS_HELP_COMMAND, CONVERSATION_ID) @@ -68,13 +65,44 @@ class GitHubCommandHandlerTest { @Test fun `webhook setup generated for app onboarding is not counted as a help command`() { - every { storage.get(CONVERSATION_ID.toStorageKey()) } returns EXISTING_SECRET + every { redisService.getWebhookSecret(CONVERSATION_ID) } returns EXISTING_SECRET handler.webhookHelp(CONVERSATION_ID) verify(exactly = 0) { usageMetrics.onHelpCommand() } } + @Test + fun `webhook help stores and returns a generated secret when none exists`() { + every { redisService.getWebhookSecret(CONVERSATION_ID) } returns null + every { redisService.setWebhookSecret(CONVERSATION_ID, GENERATED_SECRET) } returns Unit + + val response = handler.webhookHelp(CONVERSATION_ID) + + assertContains(response, GENERATED_SECRET) + verify(exactly = 1) { + redisService.setWebhookSecret(CONVERSATION_ID, GENERATED_SECRET) + } + } + + @Test + fun `Actions help stores and returns a generated secret when none exists`() { + every { redisService.getActionSecret(CONVERSATION_ID) } returns null + every { + redisService.setActionSecret( + CONVERSATION_ID, + GENERATED_ACTIONS_TOKEN + ) + } returns Unit + + val response = handler.response(GitHubCommandHandler.ACTIONS_HELP_COMMAND, CONVERSATION_ID) + + assertContains(response.orEmpty(), GENERATED_ACTIONS_TOKEN) + verify(exactly = 1) { + redisService.setActionSecret(CONVERSATION_ID, GENERATED_ACTIONS_TOKEN) + } + } + @Test fun `old singular token command is ignored`() { assertNull(handler.response("/github token", CONVERSATION_ID)) diff --git a/src/test/kotlin/com/wire/github/RedisServiceTest.kt b/src/test/kotlin/com/wire/github/RedisServiceTest.kt new file mode 100644 index 0000000..9608805 --- /dev/null +++ b/src/test/kotlin/com/wire/github/RedisServiceTest.kt @@ -0,0 +1,129 @@ +package com.wire.github + +import com.wire.sdk.model.QualifiedId +import io.lettuce.core.api.sync.RedisCommands +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import java.io.IOException +import java.util.UUID +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class RedisServiceTest { + private val storage = mockk>() + private val service = RedisService(storage) + + @Test + fun `get webhook secret reads the conversation storage key`() { + every { storage.get(WEBHOOK_STORAGE_KEY) } returns WEBHOOK_SECRET + + assertEquals(WEBHOOK_SECRET, service.getWebhookSecret(CONVERSATION_ID)) + } + + @Test + fun `set webhook secret writes the conversation storage key`() { + every { storage.set(WEBHOOK_STORAGE_KEY, WEBHOOK_SECRET) } returns "OK" + + service.setWebhookSecret(CONVERSATION_ID, WEBHOOK_SECRET) + + verify(exactly = 1) { storage.set(WEBHOOK_STORAGE_KEY, WEBHOOK_SECRET) } + } + + @Test + fun `get action secret reads the Actions storage key`() { + every { storage.get(ACTION_STORAGE_KEY) } returns ACTION_SECRET + + assertEquals(ACTION_SECRET, service.getActionSecret(CONVERSATION_ID)) + } + + @Test + fun `set action secret writes the Actions storage key`() { + every { storage.set(ACTION_STORAGE_KEY, ACTION_SECRET) } returns "OK" + + service.setActionSecret(CONVERSATION_ID, ACTION_SECRET) + + verify(exactly = 1) { storage.set(ACTION_STORAGE_KEY, ACTION_SECRET) } + } + + @Test + fun `matching stored Actions token is valid`() { + every { storage.get(ACTION_STORAGE_KEY) } returns ACTION_SECRET + + assertTrue(service.isActionsTokenValid(CONVERSATION_ID, ACTION_SECRET)) + } + + @Test + fun `wrong Actions token is invalid`() { + every { storage.get(ACTION_STORAGE_KEY) } returns ACTION_SECRET + + assertFalse(service.isActionsTokenValid(CONVERSATION_ID, "wrong-token")) + } + + @Test + fun `missing stored Actions token is invalid`() { + every { storage.get(ACTION_STORAGE_KEY) } returns null + + assertFalse(service.isActionsTokenValid(CONVERSATION_ID, ACTION_SECRET)) + } + + @Test + fun `signature generated with the stored webhook secret is valid`() { + every { storage.get(WEBHOOK_STORAGE_KEY) } returns WEBHOOK_SECRET + val signature = "sha1=${service.generateHmacSha1(PAYLOAD, WEBHOOK_SECRET)}" + + assertTrue( + service.isWebhookSignatureValid( + CONVERSATION_ID.id.toString(), + CONVERSATION_ID.domain, + signature, + PAYLOAD + ) + ) + } + + @Test + fun `signature generated with another secret is invalid`() { + every { storage.get(WEBHOOK_STORAGE_KEY) } returns WEBHOOK_SECRET + val signature = "sha1=${service.generateHmacSha1(PAYLOAD, "another-secret")}" + + assertFalse( + service.isWebhookSignatureValid( + CONVERSATION_ID.id.toString(), + CONVERSATION_ID.domain, + signature, + PAYLOAD + ) + ) + } + + @Test + fun `missing webhook secret throws`() { + every { storage.get(WEBHOOK_STORAGE_KEY) } returns null + + assertFailsWith { + service.isWebhookSignatureValid( + CONVERSATION_ID.id.toString(), + CONVERSATION_ID.domain, + "sha1=unused", + PAYLOAD + ) + } + } + + private companion object { + val CONVERSATION_ID = QualifiedId( + UUID.fromString("e37b6009-fbaa-4bb9-b345-7bdbf7d5462e"), + "conversation.example.com" + ) + const val WEBHOOK_STORAGE_KEY = + "github-app:e37b6009-fbaa-4bb9-b345-7bdbf7d5462e@conversation.example.com" + const val ACTION_STORAGE_KEY = "$WEBHOOK_STORAGE_KEY:actions-token" + const val WEBHOOK_SECRET = "webhook-secret" + const val ACTION_SECRET = "action-secret" + const val PAYLOAD = "payload" + } +} diff --git a/src/test/kotlin/com/wire/github/util/ActionsTokenValidatorTest.kt b/src/test/kotlin/com/wire/github/util/ActionsTokenValidatorTest.kt deleted file mode 100644 index 6f546db..0000000 --- a/src/test/kotlin/com/wire/github/util/ActionsTokenValidatorTest.kt +++ /dev/null @@ -1,52 +0,0 @@ -package com.wire.github.util - -import com.wire.sdk.model.QualifiedId -import io.lettuce.core.api.sync.RedisCommands -import io.mockk.every -import io.mockk.mockk -import java.util.UUID -import kotlin.test.Test -import kotlin.test.assertFalse -import kotlin.test.assertTrue - -class ActionsTokenValidatorTest { - private val storage = mockk>() - private val validator = ActionsTokenValidator(storage) - - @Test - fun `matching stored token is valid`() { - every { - storage.get(ACTIONS_STORAGE_KEY) - } returns TOKEN - - assertTrue(validator.isValid(CONVERSATION_ID, TOKEN)) - } - - @Test - fun `wrong token is invalid`() { - every { - storage.get(ACTIONS_STORAGE_KEY) - } returns TOKEN - - assertFalse(validator.isValid(CONVERSATION_ID, "wrong-token")) - } - - @Test - fun `missing stored token is invalid`() { - every { - storage.get(ACTIONS_STORAGE_KEY) - } returns null - - assertFalse(validator.isValid(CONVERSATION_ID, TOKEN)) - } - - private companion object { - val CONVERSATION_ID = QualifiedId( - UUID.randomUUID(), - "conversation.example.com" - ) - val ACTIONS_STORAGE_KEY = - "github-app:${CONVERSATION_ID.id}@${CONVERSATION_ID.domain}:actions-token" - const val TOKEN = "actions-token" - } -}