Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log redacted caller in AccessInterceptor #1117

Merged
merged 1 commit into from Jul 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions misk/src/main/kotlin/misk/MiskCaller.kt
Expand Up @@ -18,4 +18,13 @@ data class MiskCaller(

/** The identity of the calling principal, regardless of whether they are a service or a user */
val principal: String get() = service ?: user!!

/** We don't like to log usernames. */
override fun toString(): String {
return if (user != null) {
"user=${user.firstOrNull() ?: ""}███████, capabilities=$capabilities"
keeferrourke marked this conversation as resolved.
Show resolved Hide resolved
} else {
"service=$service"
}
}
}
6 changes: 6 additions & 0 deletions misk/src/main/kotlin/misk/security/authz/AccessInterceptor.kt
Expand Up @@ -6,6 +6,7 @@ import misk.Chain
import misk.MiskCaller
import misk.exceptions.UnauthenticatedException
import misk.exceptions.UnauthorizedException
import misk.logging.getLogger
import misk.scope.ActionScoped
import javax.inject.Inject
import kotlin.reflect.KClass
Expand All @@ -19,6 +20,7 @@ class AccessInterceptor private constructor(
override fun intercept(chain: Chain): Any {
val caller = caller.get() ?: throw UnauthenticatedException()
if (!isAllowed(caller)) {
logger.info { "$caller is not allowed to access ${chain.action}" }
throw UnauthorizedException()
}

Expand Down Expand Up @@ -96,4 +98,8 @@ class AccessInterceptor private constructor(
private inline fun <reified T : Annotation> Action.hasAnnotation() =
function.annotations.any { it.annotationClass == T::class }
}

companion object {
val logger = getLogger<AccessInterceptor>()
}
}
40 changes: 40 additions & 0 deletions misk/src/test/kotlin/misk/MiskCallerTest.kt
@@ -0,0 +1,40 @@
package misk

import misk.inject.KAbstractModule
import misk.testing.MiskTest
import misk.testing.MiskTestModule
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import javax.inject.Inject
import javax.inject.Qualifier

@MiskTest(startService = false)
internal class MiskCallerTest {
@MiskTestModule val module = MiskCallerTestModule()
@Inject @TestUser lateinit var testUser: MiskCaller
@Inject @TestService lateinit var testService: MiskCaller

@Test fun userNameIsRedactedFromToString() {
assertThat("$testUser").doesNotContain(testUser.user)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

}
@Test fun serviceNameIsNotRedactedFromToString() {
assertThat("$testService").contains("${testService.service}")
}
}

internal class MiskCallerTestModule : KAbstractModule() {
override fun configure() {
bind<MiskCaller>().annotatedWith<TestUser>()
.toInstance(MiskCaller(user = "Test user", capabilities = setOf("testing")))
bind<MiskCaller>().annotatedWith<TestService>()
.toInstance(MiskCaller(service = "Test service"))
}
}

@Qualifier
@Target(AnnotationTarget.FIELD, AnnotationTarget.FUNCTION)
annotation class TestUser

@Qualifier
@Target(AnnotationTarget.FIELD, AnnotationTarget.FUNCTION)
annotation class TestService