From f484bcadc3049595ea64f93c7258c0dc7c97a1cd Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Thu, 14 Oct 2021 10:48:04 -0300
Subject: [PATCH] Separate authentication and authorization
this also stubs a FileManager to show how authorization in the route endpoints could look like
---
.../mailbox/core/files/FileManager.kt | 112 ++++++++++++++++++
.../mailbox/core/server/AuthManager.kt | 60 ++++++++++
.../core/server/AuthenticationManager.kt | 23 ----
.../server/BearerAuthenticationProvider.kt | 19 +--
.../mailbox/core/server/Credentials.kt | 24 ----
.../mailbox/core/server/Routing.kt | 68 ++++++-----
.../mailbox/core/server/WebServerManager.kt | 35 ++----
.../mailbox/core/system/RandomIdManager.kt | 7 ++
8 files changed, 233 insertions(+), 115 deletions(-)
create mode 100644 mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt
create mode 100644 mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthManager.kt
delete mode 100644 mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthenticationManager.kt
delete mode 100644 mailbox-core/src/main/java/org/briarproject/mailbox/core/server/Credentials.kt
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt
new file mode 100644
index 00000000..e582dbc9
--- /dev/null
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt
@@ -0,0 +1,112 @@
+package org.briarproject.mailbox.core.files
+
+import io.ktor.application.ApplicationCall
+import io.ktor.auth.principal
+import io.ktor.http.HttpStatusCode
+import io.ktor.response.respond
+import org.briarproject.mailbox.core.db.Database
+import org.briarproject.mailbox.core.server.AuthManager
+import org.briarproject.mailbox.core.server.AuthenticationException
+import org.briarproject.mailbox.core.server.MailboxPrincipal
+import org.briarproject.mailbox.core.system.InvalidIdException
+import org.briarproject.mailbox.core.system.RandomIdManager
+import javax.inject.Inject
+
+class FileManager @Inject constructor(
+ private val db: Database,
+ private val authManager: AuthManager,
+ private val randomIdManager: RandomIdManager,
+) {
+
+ /**
+ * Used by contacts to send files to the owner and by the owner to send files to contacts.
+ *
+ * Checks if provided auth token is allowed to upload to given [folderId],
+ * Responds with 200 (OK) if upload was successful
+ * (no 201 as the uploader doesn't need to know the $fileId)
+ * The mailbox chooses a random ID string for the file ID.
+ */
+ @Throws(AuthenticationException::class, InvalidIdException::class)
+ suspend fun postFile(call: ApplicationCall, folderId: String) {
+ val principal: MailboxPrincipal? = call.principal()
+ randomIdManager.assertIsRandomId(folderId)
+ authManager.assertCanPostToFolder(principal, folderId)
+
+ // TODO implement
+
+ call.respond(HttpStatusCode.OK, "post: Not yet implemented. folderId: $folderId}")
+ }
+
+ /**
+ * Used by owner and contacts to list their files to retrieve.
+ *
+ * Checks if provided auth token is allowed to download from [folderId].
+ * Responds with 200 (OK) with the list of files in JSON.
+ */
+ suspend fun listFiles(call: ApplicationCall, folderId: String) {
+ val principal: MailboxPrincipal? = call.principal()
+ randomIdManager.assertIsRandomId(folderId)
+ authManager.assertCanDownloadFromFolder(principal, folderId)
+
+ // TODO implement
+
+ call.respond(HttpStatusCode.OK, "get: Not yet implemented. folderId: $folderId")
+ }
+
+ /**
+ * Used by owner and contacts to retrieve a file.
+ *
+ * Checks if provided auth token is allowed to download from $folderId
+ * Returns 200 (OK) if successful with the files' bytes in the response body
+ */
+ @Throws(AuthenticationException::class, InvalidIdException::class)
+ suspend fun getFile(call: ApplicationCall, folderId: String, fileId: String) {
+ val principal: MailboxPrincipal? = call.principal()
+ randomIdManager.assertIsRandomId(folderId)
+ randomIdManager.assertIsRandomId(fileId)
+ authManager.assertCanDownloadFromFolder(principal, folderId)
+
+ // TODO implement
+
+ call.respond(
+ HttpStatusCode.OK,
+ "get: Not yet implemented. folderId: $folderId fileId: $fileId"
+ )
+ }
+
+ /**
+ * Used by owner and contacts to delete files.
+ *
+ * Checks if provided auth token is allowed to download from [folderId].
+ * Responds with 200 (OK) if deletion was successful.
+ */
+ suspend fun deleteFile(call: ApplicationCall, folderId: String, fileId: String) {
+ val principal: MailboxPrincipal? = call.principal()
+ randomIdManager.assertIsRandomId(folderId)
+ randomIdManager.assertIsRandomId(fileId)
+ authManager.assertCanDownloadFromFolder(principal, folderId)
+
+ // TODO implement
+
+ call.respond(
+ HttpStatusCode.OK,
+ "delete: Not yet implemented. folderId: $folderId fileId: $fileId"
+ )
+ }
+
+ /**
+ * Used by owner only to list all folders that have files available for download.
+ *
+ * Checks if provided auth token is the owner.
+ * Responds with 200 (OK) with the list of folders with files in JSON.
+ */
+ suspend fun listFoldersWithFiles(call: ApplicationCall) {
+ val principal: MailboxPrincipal? = call.principal()
+ authManager.assertIsOwner(principal)
+
+ // TODO implement
+
+ call.respond(HttpStatusCode.OK, "get: Not yet implemented")
+ }
+
+}
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthManager.kt
new file mode 100644
index 00000000..a8d4cd46
--- /dev/null
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthManager.kt
@@ -0,0 +1,60 @@
+package org.briarproject.mailbox.core.server
+
+import io.ktor.auth.Principal
+import org.briarproject.mailbox.core.db.Database
+import javax.inject.Inject
+import javax.inject.Singleton
+
+@Singleton
+class AuthManager @Inject constructor(
+ private val db: Database,
+) {
+
+ /**
+ * Returns the principal the given token belongs to
+ * or null if this token doesn't belong to any principal.
+ */
+ fun getPrincipal(token: String): MailboxPrincipal? {
+ // TODO get real principal owning token from DB or null of token unknown
+ return MailboxPrincipal.Owner(token)
+ }
+
+ /**
+ * @throws [AuthenticationException] when given [principal] is NOT allowed
+ * to download or delete from the given [folderId].
+ */
+ @Throws(AuthenticationException::class)
+ fun assertCanDownloadFromFolder(principal: MailboxPrincipal?, folderId: String) {
+ if (principal == null) throw AuthenticationException()
+
+ // TODO check access of principal to folderId
+ }
+
+ /**
+ * @throws [AuthenticationException] when given [principal] is NOT allowed
+ * to post to the given [folderId].
+ */
+ @Throws(AuthenticationException::class)
+ fun assertCanPostToFolder(principal: MailboxPrincipal?, folderId: String) {
+ if (principal == null) throw AuthenticationException()
+ // TODO check access of principal to folderId
+ }
+
+ /**
+ * @throws [AuthenticationException] when given [principal] is NOT the mailbox owner.
+ */
+ @Throws(AuthenticationException::class)
+ fun assertIsOwner(principal: MailboxPrincipal?) {
+ if (principal !is MailboxPrincipal.Owner) throw AuthenticationException()
+ }
+
+}
+
+sealed class MailboxPrincipal(val token: String) : Principal {
+
+ class Owner(token: String) : MailboxPrincipal(token)
+ class Contact(token: String, val contactId: Int) : MailboxPrincipal(token)
+
+}
+
+class AuthenticationException : IllegalStateException()
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthenticationManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthenticationManager.kt
deleted file mode 100644
index 33da1663..00000000
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/AuthenticationManager.kt
+++ /dev/null
@@ -1,23 +0,0 @@
-package org.briarproject.mailbox.core.server
-
-import javax.inject.Inject
-import javax.inject.Singleton
-
-@Singleton
-class AuthenticationManager @Inject constructor() {
-
- fun canOwnerAccess(credentials: Credentials): Boolean {
- // TODO check credentials:
- // * token must be from owner
- // * if credentials.folderId is not null, must have accessType right to folderId
- return credentials.token == "test123"
- }
-
- fun canOwnerOrContactAccess(credentials: Credentials): Boolean {
- require(credentials.folderId != null)
- // TODO check credentials:
- // * token must have credentials.accessType right to folderId
- return credentials.token == "test123" || credentials.token == "test124"
- }
-
-}
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/BearerAuthenticationProvider.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/BearerAuthenticationProvider.kt
index 690844f0..808f7648 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/BearerAuthenticationProvider.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/BearerAuthenticationProvider.kt
@@ -8,7 +8,6 @@ import io.ktor.auth.AuthenticationFailedCause
import io.ktor.auth.AuthenticationFunction
import io.ktor.auth.AuthenticationPipeline
import io.ktor.auth.AuthenticationProvider
-import io.ktor.auth.Principal
import io.ktor.auth.UnauthorizedResponse
import io.ktor.auth.parseAuthorizationHeader
import io.ktor.http.auth.HttpAuthHeader
@@ -24,7 +23,7 @@ private val LOG = getLogger(BearerAuthenticationProvider::class.java)
internal class BearerAuthenticationProvider constructor(config: Configuration) :
AuthenticationProvider(config) {
- internal var realm: String = config.realm ?: "Ktor Server"
+ internal val realm: String = "Briar Mailbox"
internal val authHeader: (ApplicationCall) -> HttpAuthHeader? = { call ->
try {
call.request.parseAuthorizationHeader()
@@ -38,13 +37,11 @@ internal class BearerAuthenticationProvider constructor(config: Configuration) :
internal class Configuration internal constructor(name: String?) :
AuthenticationProvider.Configuration(name) {
- var realm: String? = null
-
/**
- * This function is applied to every call with [Credentials].
- * @return a principal (usually an instance of [Principal]) or `null`
+ * This function is applied to every call with a [String] auth token.
+ * @return a [MailboxPrincipal] or `null`
*/
- var authenticationFunction: AuthenticationFunction<Credentials> = {
+ var authenticationFunction: AuthenticationFunction<String> = {
throw NotImplementedError(
"Bearer auth authenticationFunction is not specified." +
"Use bearer { authenticationFunction = { ... } } to fix."
@@ -82,21 +79,17 @@ private suspend fun PipelineContext<AuthenticationContext, ApplicationCall>.auth
}
try {
- // TODO try faking accessType with X-Http-Method-Override header
- val accessType = call.request.httpMethod.toAccessType()
val token = (authHeader as? HttpAuthHeader.Single)?.blob
- if (accessType == null || token == null) {
+ if (token == null) {
context.unauthorizedResponse(AuthenticationFailedCause.InvalidCredentials, provider)
return
}
- val folderId = call.parameters["folderId"]
// TODO remove logging before release
LOG.debug { "name: $name" }
LOG.debug { "httpMethod: ${call.request.httpMethod}" }
- val credentials = Credentials(accessType, token, folderId)
- val principal = provider.authenticationFunction(call, credentials)
+ val principal = provider.authenticationFunction(call, token)
if (principal == null) {
context.unauthorizedResponse(AuthenticationFailedCause.InvalidCredentials, provider)
} else {
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/Credentials.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/Credentials.kt
deleted file mode 100644
index c06a49fe..00000000
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/Credentials.kt
+++ /dev/null
@@ -1,24 +0,0 @@
-package org.briarproject.mailbox.core.server
-
-import io.ktor.http.HttpMethod
-
-data class Credentials(
- val accessType: AccessType,
- val token: String,
- val folderId: String?,
-)
-
-enum class AccessType { UPLOAD, DOWNLOAD_DELETE }
-
-internal fun HttpMethod.toAccessType(): AccessType? = when (this) {
- HttpMethod.Get -> AccessType.DOWNLOAD_DELETE
- HttpMethod.Delete -> AccessType.DOWNLOAD_DELETE
- HttpMethod.Post -> AccessType.UPLOAD
- HttpMethod.Put -> AccessType.UPLOAD
- else -> null
-}
-
-internal object AuthContext {
- const val ownerOnly = "ownerOnly"
- const val ownerAndContacts = "ownerAndContacts"
-}
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/Routing.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/Routing.kt
index 62a8a2be..d0176946 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/Routing.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/Routing.kt
@@ -1,6 +1,7 @@
package org.briarproject.mailbox.core.server
import io.ktor.application.Application
+import io.ktor.application.ApplicationCall
import io.ktor.application.call
import io.ktor.auth.authenticate
import io.ktor.http.ContentType
@@ -13,6 +14,9 @@ import io.ktor.routing.post
import io.ktor.routing.put
import io.ktor.routing.route
import io.ktor.routing.routing
+import io.ktor.util.getOrFail
+import org.briarproject.mailbox.core.files.FileManager
+import org.briarproject.mailbox.core.system.InvalidIdException
internal const val V = "/" // TODO set to "/v1" for release
@@ -21,7 +25,7 @@ internal fun Application.configureBasicApi() = routing {
get {
call.respondText("Hello world!", ContentType.Text.Plain)
}
- authenticate(AuthContext.ownerOnly) {
+ authenticate {
delete {
call.respond(HttpStatusCode.OK, "delete: Not yet implemented")
}
@@ -33,7 +37,7 @@ internal fun Application.configureBasicApi() = routing {
}
internal fun Application.configureContactApi() = routing {
- authenticate(AuthContext.ownerOnly) {
+ authenticate {
route("$V/contacts") {
put("/{contactId}") {
call.respond(
@@ -56,47 +60,53 @@ internal fun Application.configureContactApi() = routing {
}
}
-internal fun Application.configureFilesApi() = routing {
+internal fun Application.configureFilesApi(fileManager: FileManager) = routing {
- authenticate(AuthContext.ownerAndContacts) {
+ authenticate {
route("$V/files/{folderId}") {
post {
- call.respond(
- HttpStatusCode.OK,
- "post: Not yet implemented. " +
- "folderId: ${call.parameters["folderId"]}"
- )
+ call.handle {
+ fileManager.postFile(call, call.parameters.getOrFail("folderId"))
+ }
}
get {
- call.respond(
- HttpStatusCode.OK,
- "get: Not yet implemented. " +
- "folderId: ${call.parameters["folderId"]}"
- )
+ call.handle {
+ fileManager.listFiles(call, call.parameters.getOrFail("folderId"))
+ }
}
route("/{fileId}") {
get {
- call.respond(
- HttpStatusCode.OK,
- "get: Not yet implemented. " +
- "folderId: ${call.parameters["folderId"]} " +
- "fileId: ${call.parameters["fileId"]}"
- )
+ val folderId = call.parameters.getOrFail("folderId")
+ val fileId = call.parameters.getOrFail("fileId")
+ call.handle {
+ fileManager.getFile(call, folderId, fileId)
+ }
}
delete {
- call.respond(
- HttpStatusCode.OK,
- "delete: Not yet implemented. " +
- "folderId: ${call.parameters["folderId"]} " +
- "fileId: ${call.parameters["fileId"]}"
- )
+ val folderId = call.parameters.getOrFail("folderId")
+ val fileId = call.parameters.getOrFail("fileId")
+ call.handle {
+ fileManager.deleteFile(call, folderId, fileId)
+ }
}
}
}
}
- authenticate(AuthContext.ownerOnly) {
- get("$V/mailboxes") {
- call.respond(HttpStatusCode.OK, "get: Not yet implemented")
+ authenticate {
+ get("$V/folders") {
+ call.handle {
+ fileManager.listFoldersWithFiles(call)
+ }
}
}
}
+
+private suspend fun ApplicationCall.handle(block: suspend () -> Unit) {
+ try {
+ block()
+ } catch (e: AuthenticationException) {
+ respond(HttpStatusCode.Unauthorized, HttpStatusCode.Unauthorized.description)
+ } catch (e: InvalidIdException) {
+ respond(HttpStatusCode.BadRequest, "Malformed ID: ${e.id}")
+ }
+}
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/WebServerManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/WebServerManager.kt
index a806bc3b..8cecb21a 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/WebServerManager.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/server/WebServerManager.kt
@@ -2,10 +2,10 @@ package org.briarproject.mailbox.core.server
import io.ktor.application.install
import io.ktor.auth.Authentication
-import io.ktor.auth.UserIdPrincipal
import io.ktor.features.CallLogging
import io.ktor.server.engine.embeddedServer
import io.ktor.server.netty.Netty
+import org.briarproject.mailbox.core.files.FileManager
import org.briarproject.mailbox.core.lifecycle.Service
import org.briarproject.mailbox.core.server.WebServerManager.Companion.PORT
import org.slf4j.Logger
@@ -21,7 +21,8 @@ interface WebServerManager : Service {
@Singleton
internal class WebServerManagerImpl @Inject constructor(
- private val authManager: AuthenticationManager,
+ private val authManager: AuthManager,
+ private val fileManager: FileManager,
) : WebServerManager {
internal companion object {
@@ -31,36 +32,18 @@ internal class WebServerManagerImpl @Inject constructor(
private val server by lazy {
embeddedServer(Netty, PORT, watchPaths = emptyList()) {
install(CallLogging)
- // TODO validate folderId and fileId somewhere
install(Authentication) {
- bearer(AuthContext.ownerOnly) {
- realm = "Briar Mailbox Owner"
- authenticationFunction = { credentials ->
- // TODO: Remove logging [of credentials] before release.
- LOG.error("credentials: $credentials")
- if (authManager.canOwnerAccess(credentials)) {
- UserIdPrincipal(AuthContext.ownerOnly)
- } else null // not authenticated
- }
- }
- bearer(AuthContext.ownerAndContacts) {
- realm = "Briar Mailbox"
- authenticationFunction = { credentials ->
- LOG.error("credentials: $credentials")
- val folderId = credentials.folderId
- // we must have a folderId for this AuthContext
- if (folderId == null) {
- LOG.warn("No folderId found in request")
- null
- } else if (authManager.canOwnerOrContactAccess(credentials)) {
- UserIdPrincipal(AuthContext.ownerAndContacts)
- } else null // not authenticated
+ bearer {
+ authenticationFunction = { token ->
+ // TODO: Remove logging of token before release.
+ LOG.error("token: $token")
+ authManager.getPrincipal(token)
}
}
}
configureBasicApi()
configureContactApi()
- configureFilesApi()
+ configureFilesApi(fileManager)
}
}
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/system/RandomIdManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/system/RandomIdManager.kt
index 15bcaa4c..4e8c89cd 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/system/RandomIdManager.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/system/RandomIdManager.kt
@@ -22,8 +22,15 @@ class RandomIdManager @Inject constructor() {
return idRegex.matches(id)
}
+ @Throws(InvalidIdException::class)
+ fun assertIsRandomId(id: String) {
+ if (!isValidRandomId(id)) throw InvalidIdException(id)
+ }
+
}
+class InvalidIdException(val id: String) : IllegalStateException()
+
fun ByteArray.toHex(): String = joinToString(separator = "") { eachByte ->
"%02x".format(eachByte)
}
--
GitLab