Skip to content

Commit

Permalink
fix: Disable importing with any project permission (#1818)
Browse files Browse the repository at this point in the history
  • Loading branch information
JanCizmar committed Jul 21, 2023
1 parent 02f04bb commit 4776cba
Show file tree
Hide file tree
Showing 10 changed files with 88 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import io.tolgee.model.Language
import io.tolgee.model.dataImport.ImportFile
import io.tolgee.model.dataImport.ImportLanguage
import io.tolgee.model.dataImport.ImportTranslation
import io.tolgee.model.enums.Scope
import io.tolgee.model.views.ImportFileIssueView
import io.tolgee.model.views.ImportLanguageView
import io.tolgee.model.views.ImportTranslationView
Expand All @@ -36,6 +37,7 @@ import io.tolgee.service.LanguageService
import io.tolgee.service.dataImport.ForceMode
import io.tolgee.service.dataImport.ImportService
import io.tolgee.service.key.NamespaceService
import io.tolgee.service.security.SecurityService
import org.springdoc.api.annotations.ParameterObject
import org.springframework.data.domain.PageRequest
import org.springframework.data.domain.Pageable
Expand Down Expand Up @@ -64,7 +66,10 @@ import javax.servlet.http.HttpServletRequest
@RestController
@CrossOrigin(origins = ["*"])
@RequestMapping(value = ["/v2/projects/{projectId:\\d+}/import", "/v2/projects/import"])
@Tag(name = "Import")
@Tag(
name = "Import",
description = "These endpoints handle multi-step data import"
)
class V2ImportController(
private val importService: ImportService,
private val authenticationFacade: AuthenticationFacade,
Expand All @@ -81,6 +86,7 @@ class V2ImportController(
private val projectHolder: ProjectHolder,
private val languageService: LanguageService,
private val namespaceService: NamespaceService,
private val securityService: SecurityService
) {
@PostMapping("", consumes = [MediaType.MULTIPART_FORM_DATA_VALUE])
@AccessWithAnyProjectPermission()
Expand All @@ -90,6 +96,7 @@ class V2ImportController(
@RequestPart("files") files: Array<MultipartFile>,
@ParameterObject params: ImportAddFilesParams
): ImportAddFilesResultModel {
checkBaseImportPermissions()
val fileDtos = files.map { ImportFileDto(it.originalFilename ?: "", it.inputStream) }
val errors = importService.addFiles(
files = fileDtos,
Expand Down Expand Up @@ -122,16 +129,22 @@ class V2ImportController(
forceMode: ForceMode,
) {
val projectId = projectHolder.project.id
checkBaseImportPermissions()
this.importService.import(projectId, authenticationFacade.userAccount.id, forceMode)
}

private fun checkBaseImportPermissions() {
securityService.checkProjectPermission(projectHolder.project.id, Scope.TRANSLATIONS_VIEW)
}

@GetMapping("/result")
@AccessWithAnyProjectPermission()
@AccessWithApiKey()
@Operation(description = "Returns the result of preparation.", summary = "Get result")
fun getImportResult(
@ParameterObject pageable: Pageable
): PagedModel<ImportLanguageModel> {
checkBaseImportPermissions()
val projectId = projectHolder.project.id
val userId = authenticationFacade.userAccount.id
val languages = importService.getResult(projectId, userId, pageable)
Expand All @@ -145,6 +158,7 @@ class V2ImportController(
fun getImportLanguage(
@PathVariable("languageId") languageId: Long,
): ImportLanguageModel {
checkBaseImportPermissions()
checkImportLanguageInProject(languageId)
val language = importService.findLanguageView(languageId) ?: throw NotFoundException()
return importLanguageModelAssembler.toModel(language)
Expand All @@ -171,6 +185,7 @@ class V2ImportController(
@RequestParam("search") search: String? = null,
@ParameterObject @SortDefault("keyName") pageable: Pageable
): PagedModel<ImportTranslationModel> {
checkBaseImportPermissions()
checkImportLanguageInProject(languageId)
val translations = importService.getTranslationsView(languageId, pageable, onlyConflicts, onlyUnresolved, search)
return pagedTranslationsResourcesAssembler.toModel(translations, importTranslationModelAssembler)
Expand All @@ -181,6 +196,7 @@ class V2ImportController(
@AccessWithApiKey()
@Operation(description = "Deletes prepared import data.", summary = "Delete")
fun cancelImport() {
checkBaseImportPermissions()
this.importService.deleteImport(projectHolder.project.id, authenticationFacade.userAccount.id)
}

Expand All @@ -189,6 +205,7 @@ class V2ImportController(
@AccessWithApiKey()
@Operation(description = "Deletes language prepared to import.", summary = "Delete language")
fun deleteLanguage(@PathVariable("languageId") languageId: Long) {
checkBaseImportPermissions()
val language = checkImportLanguageInProject(languageId)
this.importService.deleteLanguage(language)
}
Expand All @@ -204,6 +221,7 @@ class V2ImportController(
@PathVariable("languageId") languageId: Long,
@PathVariable("translationId") translationId: Long
) {
checkBaseImportPermissions()
resolveTranslation(languageId, translationId, true)
}

Expand All @@ -218,6 +236,7 @@ class V2ImportController(
@PathVariable("languageId") languageId: Long,
@PathVariable("translationId") translationId: Long
) {
checkBaseImportPermissions()
resolveTranslation(languageId, translationId, false)
}

Expand All @@ -231,6 +250,7 @@ class V2ImportController(
fun resolveTranslationSetOverride(
@PathVariable("languageId") languageId: Long
) {
checkBaseImportPermissions()
resolveAllOfLanguage(languageId, true)
}

Expand All @@ -244,6 +264,7 @@ class V2ImportController(
fun resolveTranslationSetKeepExisting(
@PathVariable("languageId") languageId: Long,
) {
checkBaseImportPermissions()
resolveAllOfLanguage(languageId, false)
}

Expand All @@ -259,6 +280,7 @@ class V2ImportController(
@RequestBody req: SetFileNamespaceRequest,
request: HttpServletRequest
) {
checkBaseImportPermissions()
val file = checkFileFromProject(fileId)
this.importService.selectNamespace(file, req.namespace)
}
Expand All @@ -275,6 +297,7 @@ class V2ImportController(
@PathVariable("importLanguageId") importLanguageId: Long,
@PathVariable("existingLanguageId") existingLanguageId: Long,
) {
checkBaseImportPermissions()
val existingLanguage = checkLanguageFromProject(existingLanguageId)
val importLanguage = checkImportLanguageInProject(importLanguageId)
this.importService.selectExistingLanguage(importLanguage, existingLanguage)
Expand All @@ -290,6 +313,7 @@ class V2ImportController(
fun resetExistingLanguage(
@PathVariable("importLanguageId") importLanguageId: Long,
) {
checkBaseImportPermissions()
val importLanguage = checkImportLanguageInProject(importLanguageId)
this.importService.selectExistingLanguage(importLanguage, null)
}
Expand Down Expand Up @@ -318,6 +342,7 @@ class V2ImportController(
summary = "Get namespaces"
)
fun getAllNamespaces(): CollectionModel<ImportNamespaceModel> {
checkBaseImportPermissions()
val import = importService.get(
projectId = projectHolder.project.id,
authorId = authenticationFacade.userAccount.id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class WebSocketConfig(
if (projectId != null) {
try {
val user = (accessor.user as? UsernamePasswordAuthenticationToken)?.principal as UserAccountDto
securityService.checkProjectPermission(projectId = projectId, Scope.TRANSLATIONS_VIEW, user)
securityService.checkProjectPermissionNoApiKey(projectId = projectId, Scope.TRANSLATIONS_VIEW, user)
} catch (e: Exception) {
throw MessagingException("Forbidden")
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
package io.tolgee.api.v2.controllers.v2ImportController

import io.tolgee.ProjectAuthControllerTest
import io.tolgee.constants.MtServiceType
import io.tolgee.development.testDataBuilder.data.dataImport.ImportTestData
import io.tolgee.fixtures.andAssertThatJson
import io.tolgee.fixtures.andIsForbidden
import io.tolgee.fixtures.andIsOk
import io.tolgee.fixtures.isValidId
import io.tolgee.fixtures.node
import io.tolgee.testing.AuthorizedControllerTest
import io.tolgee.model.enums.Scope
import io.tolgee.testing.annotations.ProjectApiKeyAuthTestMethod
import io.tolgee.testing.assert
import io.tolgee.testing.assertions.Assertions.assertThat
import org.junit.jupiter.api.Test

class V2ImportControllerApplicationTest : AuthorizedControllerTest() {
class V2ImportControllerApplicationTest : ProjectAuthControllerTest("/v2/projects/") {
@Test
fun `it applies the import`() {
val testData = ImportTestData()
Expand Down Expand Up @@ -83,7 +85,7 @@ class V2ImportControllerApplicationTest : AuthorizedControllerTest() {
val path = "/v2/projects/$projectId/import/apply?forceMode=OVERRIDE"
performAuthPut(path, null).andIsForbidden.andAssertThatJson {
node("params") {
node("[0]").isEqualTo("""["keys.edit"]""")
node("[0]").isEqualTo(""""keys.create"""")
}
}
}
Expand Down Expand Up @@ -128,6 +130,22 @@ class V2ImportControllerApplicationTest : AuthorizedControllerTest() {
performAuthPut(path, null).andIsForbidden
}

@Test
@ProjectApiKeyAuthTestMethod(scopes = [Scope.TRANSLATIONS_VIEW])
fun `it checks permissions with API key (view only)`() {
val testData = ImportTestData()
testData.importBuilder.data.importFiles[0].data.importKeys.removeIf { it.self == testData.newLongKey }
val resolveFrench = testData.addFrenchTranslations()
resolveFrench()

testDataService.saveTestData(testData.root)
projectSupplier = { testData.project }
userAccount = testData.userAccount

val path = "import/apply?forceMode=OVERRIDE"
performProjectAuthPut(path, null).andIsForbidden
}

@Test
fun `it sets outdated on update`() {
val testData = ImportTestData()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import io.tolgee.dtos.request.key.KeyScreenshotDto
import io.tolgee.exceptions.FileStoreException
import io.tolgee.fixtures.andAssertThatJson
import io.tolgee.fixtures.andIsCreated
import io.tolgee.fixtures.andIsForbidden
import io.tolgee.fixtures.andPrettyPrint
import io.tolgee.fixtures.isValidId
import io.tolgee.fixtures.node
Expand Down Expand Up @@ -63,7 +64,7 @@ class KeyControllerCreationTest : ProjectAuthControllerTest("/v2/projects/") {
}
}

@ProjectApiKeyAuthTestMethod(scopes = [Scope.KEYS_CREATE])
@ProjectApiKeyAuthTestMethod(scopes = [Scope.KEYS_CREATE, Scope.TRANSLATIONS_EDIT])
@Test
fun `creates key with keys create scope`() {
performProjectAuthPost("keys", CreateKeyDto(name = "super_key", translations = mapOf("en" to "", "de" to "")))
Expand All @@ -73,6 +74,13 @@ class KeyControllerCreationTest : ProjectAuthControllerTest("/v2/projects/") {
}
}

@ProjectApiKeyAuthTestMethod(scopes = [Scope.KEYS_CREATE])
@Test
fun `create key with translations require translate permissions`() {
performProjectAuthPost("keys", CreateKeyDto(name = "super_key", translations = mapOf("en" to "", "de" to "")))
.andIsForbidden
}

@ProjectJWTAuthTestMethod
@Test
fun `creates key with translations and tags and screenshots`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import org.springframework.web.client.RestTemplate
@SpringBootTest(
properties = [
"tolgee.telemetry.report-period-ms=200",
"tolgee.telemetry.enabled=true"
"tolgee.telemetry.enabled=false"
]
)
class TelemetryServiceTest : AbstractSpringTest() {
Expand All @@ -39,7 +39,7 @@ class TelemetryServiceTest : AbstractSpringTest() {

@AfterEach
fun clean() {
telemetryProperties.enabled = true
telemetryProperties.enabled = false
}

@Test
Expand All @@ -61,6 +61,7 @@ class TelemetryServiceTest : AbstractSpringTest() {

@Test
fun `reports when enabled`() {
telemetryProperties.enabled = true
val testData = BaseTestData().apply {
this.root.addProject { name = "bbbb" }.build {
val en = addEnglish()
Expand Down
1 change: 1 addition & 0 deletions backend/app/src/test/resources/application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ tolgee:
enabled: false
telemetry:
enabled: false
server: http://localhost:8080
logging:
level:
io.tolgee.billing.api.v2.OrganizationInvoicesController: DEBUG
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,15 @@ interface ImportLanguageRepository : JpaRepository<ImportLanguage, Long> {
"""
)
fun findViewById(languageId: Long): Optional<ImportLanguageView>

@Query(
"""
select distinct il.existingLanguage.id
from ImportLanguage il
join il.file if
where if.import.id = :importId
and il.existingLanguage.id is not null
"""
)
fun findAssignedExistingLanguageIds(importId: Long): List<Long>
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class StoredDataImporter(
private fun checkKeyPermissions() {
val isCreatingKey = keysToSave.values.any { it.id == 0L }
if (isCreatingKey) {
securityService.checkProjectPermission(import.project.id, Scope.KEYS_EDIT)
securityService.checkProjectPermission(import.project.id, Scope.KEYS_CREATE)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,17 @@ class SecurityService @Autowired constructor(
throw PermissionException()
}

fun checkProjectPermission(projectId: Long, requiredScopes: Scope, userAccountDto: UserAccountDto) {
checkProjectPermissionOr(projectId, listOf(requiredScopes), userAccountDto)
}

fun checkProjectPermission(projectId: Long, requiredScopes: Scope, apiKey: ApiKey) {
checkProjectPermissionOr(listOf(requiredScopes), apiKey)
checkProjectPermission(listOf(requiredScopes), apiKey)
}

private fun checkProjectPermissionOr(requiredScopes: List<Scope>, apiKey: ApiKey) {
this.checkApiKeyScopesOr(requiredScopes, apiKey)
private fun checkProjectPermission(requiredScopes: List<Scope>, apiKey: ApiKey) {
this.checkApiKeyScopes(requiredScopes, apiKey)
}

fun checkProjectPermissionOr(
fun checkProjectPermissionNoApiKey(
projectId: Long,
requiredScopes: Collection<Scope>,
requiredScope: Scope,
userAccountDto: UserAccountDto
) {
if (isUserAdmin(userAccountDto)) {
Expand All @@ -64,32 +60,27 @@ class SecurityService @Autowired constructor(
val allowedScopes = getProjectPermissionScopes(projectId, userAccountDto.id)
?: throw PermissionException(Message.USER_HAS_NO_PROJECT_ACCESS)

checkProjectPermissionOr(projectId, requiredScopes, allowedScopes)
checkPermission(requiredScope, allowedScopes)
}

fun checkProjectPermissionOr(
projectId: Long,
requiredScopes: Collection<Scope>,
private fun checkPermission(
requiredScope: Scope,
allowedScopes: Array<Scope>
) {
if (!allowedScopes.any { requiredScopes.contains(it) }) {
if (!allowedScopes.contains(requiredScope)) {
@Suppress("UNCHECKED_CAST")
throw PermissionException(
Message.OPERATION_NOT_PERMITTED,
listOf(requiredScopes.map { it.value }) as List<Serializable>
listOf(requiredScope.value) as List<Serializable>
)
}
}

fun checkProjectPermission(projectId: Long, requiredPermission: Scope) {
val apiKey = activeApiKey ?: return checkProjectPermission(projectId, requiredPermission, activeUser)
val apiKey = activeApiKey ?: return checkProjectPermissionNoApiKey(projectId, requiredPermission, activeUser)
return checkProjectPermission(projectId, requiredPermission, apiKey)
}

fun checkProjectPermissionOr(projectId: Long, requiredPermissions: Collection<Scope>) {
checkProjectPermissionOr(projectId, requiredPermissions, activeUser)
}

fun checkLanguageViewPermissionByTag(projectId: Long, languageTags: Collection<String>) {
checkProjectPermission(projectId, Scope.TRANSLATIONS_VIEW)
checkLanguagePermissionByTag(
Expand Down Expand Up @@ -226,9 +217,10 @@ class SecurityService @Autowired constructor(
}
}

fun checkApiKeyScopesOr(scopes: Collection<Scope>, apiKey: ApiKey) {
fun checkApiKeyScopes(scopes: Collection<Scope>, apiKey: ApiKey) {
checkApiKeyScopes(apiKey) { expandedScopes ->
if (!expandedScopes.any { it in apiKey.scopesEnum }) {
val hasRequiredPermission = scopes.all { expandedScopes.contains(it) }
if (!hasRequiredPermission) {
throw PermissionException()
}
}
Expand Down
Loading

0 comments on commit 4776cba

Please sign in to comment.