Skip to content

Commit

Permalink
fix: Translation endpoints permission check (#2254)
Browse files Browse the repository at this point in the history
  • Loading branch information
JanCizmar authored Apr 17, 2024
1 parent 3ea21b8 commit f712139
Show file tree
Hide file tree
Showing 16 changed files with 156 additions and 141 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -165,19 +165,14 @@ class ApiKeyController(
)

val computed = permissionData.computedPermissions
val scopes =
when {
apiKeyAuthentication -> authenticationFacade.projectApiKey.scopes.toTypedArray()
else -> computed.scopes
}

return ApiKeyPermissionsModel(
projectIdNotNull,
type = if (apiKeyAuthentication) null else computed.type,
translateLanguageIds = computed.translateLanguageIds.toNormalizedPermittedLanguageSet(),
viewLanguageIds = computed.viewLanguageIds.toNormalizedPermittedLanguageSet(),
stateChangeLanguageIds = computed.stateChangeLanguageIds.toNormalizedPermittedLanguageSet(),
scopes = scopes,
scopes = securityService.getCurrentPermittedScopes(projectIdNotNull).toTypedArray(),
project = simpleProjectModelAssembler.toModel(projectService.get(projectIdNotNull)),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import io.tolgee.hateoas.translations.TranslationHistoryModel
import io.tolgee.hateoas.translations.TranslationHistoryModelAssembler
import io.tolgee.hateoas.translations.TranslationModel
import io.tolgee.hateoas.translations.TranslationModelAssembler
import io.tolgee.model.Screenshot
import io.tolgee.model.enums.AssignableTranslationState
import io.tolgee.model.enums.Scope
import io.tolgee.model.translation.Translation
Expand All @@ -47,6 +46,7 @@ import io.tolgee.service.translation.TranslationService
import jakarta.validation.Valid
import org.springdoc.core.annotations.ParameterObject
import org.springframework.beans.propertyeditors.CustomCollectionEditor
import org.springframework.data.domain.Page
import org.springframework.data.domain.PageRequest
import org.springframework.data.domain.Pageable
import org.springframework.data.domain.Sort
Expand Down Expand Up @@ -229,7 +229,7 @@ When null, resulting file will be a flat key-value object.

@GetMapping(value = [""])
@Operation(summary = "Get translations in project")
@UseDefaultPermissions // Security: check done internally
@RequiresProjectPermissions(scopes = [Scope.KEYS_VIEW]) // Security: check done internally
@AllowApiAccess
@Transactional
@OpenApiOrderExtension(5)
Expand All @@ -252,16 +252,24 @@ When null, resulting file will be a flat key-value object.
translationService
.getViewData(projectHolder.project.id, pageableWithSort, params, languages)

val keysWithScreenshots = getScreenshots(data.map { it.keyId }.toList())

if (keysWithScreenshots != null) {
data.content.forEach { it.screenshots = keysWithScreenshots[it.keyId] ?: listOf() }
}
addScreenshotsToResponse(data)

val cursor = if (data.content.isNotEmpty()) CursorUtil.getCursor(data.content.last(), data.sort) else null
return pagedAssembler.toTranslationModel(data, languages, cursor)
}

private fun addScreenshotsToResponse(data: Page<KeyWithTranslationsView>) {
val canViewScreenshots = securityService.currentPermittedScopesContain(Scope.SCREENSHOTS_VIEW)

if (!canViewScreenshots) {
return
}

val keysWithScreenshots = screenshotService.getScreenshotsForKeys(data.map { it.keyId }.content)

data.content.forEach { it.screenshots = keysWithScreenshots[it.keyId] ?: listOf() }
}

@PutMapping(value = ["/{translationId:[0-9]+}/dismiss-auto-translated-state"])
@Operation(summary = "Dismiss auto-translated", description = """Removes "auto translated" indication""")
@RequestActivity(ActivityType.DISMISS_AUTO_TRANSLATED_STATE)
Expand Down Expand Up @@ -317,23 +325,6 @@ When null, resulting file will be a flat key-value object.
return historyPagedAssembler.toModel(translations, historyModelAssembler)
}

private fun getScreenshots(keyIds: Collection<Long>): Map<Long, List<Screenshot>>? {
if (
!authenticationFacade.isProjectApiKeyAuth ||
authenticationFacade.projectApiKey.scopes.contains(Scope.SCREENSHOTS_VIEW)
) {
return screenshotService.getScreenshotsForKeys(keyIds)
}
return null
}

private fun checkKeyEditScope() {
securityService.checkProjectPermission(
projectHolder.project.id,
Scope.KEYS_EDIT,
)
}

private fun getSafeSortPageable(pageable: Pageable): Pageable {
var sort = pageable.sort
if (sort.getOrderFor(KeyWithTranslationsView::keyId.name) == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ class V2InvitationControllerTest : AuthorizedControllerTest() {
expectedType: ProjectPermissionType,
) {
assertThat(invitationService.getForProject(project)).hasSize(0)
assertThat(permissionService.getProjectPermissionScopes(project.id, newUser)).isNotNull
val type = permissionService.getProjectPermissionScopes(project.id, newUser)!!
assertThat(permissionService.getProjectPermissionScopesNoApiKey(project.id, newUser)).isNotNull
val type = permissionService.getProjectPermissionScopesNoApiKey(project.id, newUser)!!
type.assert.equalsPermissionType(expectedType)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import io.tolgee.ProjectAuthControllerTest
import io.tolgee.development.testDataBuilder.data.NamespacesTestData
import io.tolgee.development.testDataBuilder.data.TranslationsTestData
import io.tolgee.fixtures.andAssertThatJson
import io.tolgee.fixtures.andIsForbidden
import io.tolgee.fixtures.andIsNotFound
import io.tolgee.fixtures.andIsOk
import io.tolgee.fixtures.andPrettyPrint
Expand Down Expand Up @@ -113,6 +114,17 @@ class TranslationsControllerViewTest : ProjectAuthControllerTest("/v2/projects/"
}
}

@Test
@ProjectApiKeyAuthTestMethod(scopes = [Scope.KEYS_VIEW])
fun `returns empty translations when api key is missing the translations-view scope`() {
testData.addKeysViewOnlyUser()
testDataService.saveTestData(testData.root)
userAccount = testData.user
performProjectAuthGet("/translations?sort=id").andIsOk.andAssertThatJson {
node("_embedded.keys[2].translations.de").isAbsent()
}
}

@Test
@ProjectJWTAuthTestMethod
fun `returns correct screenshot data`() {
Expand Down Expand Up @@ -239,6 +251,26 @@ class TranslationsControllerViewTest : ProjectAuthControllerTest("/v2/projects/"
}
}

@ProjectApiKeyAuthTestMethod(scopes = [Scope.BATCH_JOBS_VIEW])
@Test
fun `checks keys view scope (missing scope)`() {
testDataService.saveTestData(testData.root)
userAccount = testData.user
performProjectAuthGet("/translations").andIsForbidden
}

@ProjectApiKeyAuthTestMethod(scopes = [Scope.TRANSLATIONS_VIEW])
@Test
fun `returns no screenshots when screenshot view scope is missing`() {
testData.addKeysWithScreenshots()
testDataService.saveTestData(testData.root)
userAccount = testData.user
performProjectAuthGet("/translations").andPrettyPrint.andAssertThatJson {
node("_embedded.keys[3].screenshots").isNull()
node("_embedded.keys[3].screenshotCount").isEqualTo(2)
}
}

@ProjectApiKeyAuthTestMethod(apiKeyPresentType = ApiKeyPresentMode.QUERY_PARAM)
@Test
fun `works with API key in query`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class ProjectsControllerPermissionsTest : ProjectAuthControllerTest("/v2/project
permissionTestUtil.withPermissionsTestData { project, user ->
performAuthPut("/v2/projects/${project.id}/users/${user.id}/set-permissions/EDIT", null).andIsOk

permissionService.getProjectPermissionScopes(project.id, user)
permissionService.getProjectPermissionScopesNoApiKey(project.id, user)
.let { Assertions.assertThat(it).equalsPermissionType(ProjectPermissionType.EDIT) }
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import org.springframework.boot.test.context.SpringBootTest

@SpringBootTest
@AutoConfigureMockMvc
open class ProjectsControllerTest : ProjectAuthControllerTest("/v2/projects/") {
class ProjectsControllerTest : ProjectAuthControllerTest("/v2/projects/") {
@Test
fun getAll() {
executeInNewTransaction {
Expand Down Expand Up @@ -242,7 +242,7 @@ open class ProjectsControllerTest : ProjectAuthControllerTest("/v2/projects/") {

performAuthPut("/v2/projects/${repo.id}/users/${user.id}/revoke-access", null).andIsOk

permissionService.getProjectPermissionScopes(repo.id, user)
permissionService.getProjectPermissionScopesNoApiKey(repo.id, user)
.let { assertThat(it).isEmpty() }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ enum class Message {
CURRENT_USER_DOES_NOT_OWN_IMAGE,
USER_CANNOT_VIEW_THIS_ORGANIZATION,
USER_IS_NOT_OWNER_OF_ORGANIZATION,
PAK_CREATED_FOR_DIFFERENT_PROJECT,
;

val code: String
Expand Down
5 changes: 3 additions & 2 deletions backend/data/src/main/kotlin/io/tolgee/model/enums/Scope.kt
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ enum class Scope(

fun expand(scope: Scope): Array<Scope> {
val hierarchyItems = getScopeHierarchyItems(scope)
return hierarchyItems.flatMap { expand(it) }.toTypedArray()
return hierarchyItems.flatMap { expand(it) }.toSet().toTypedArray()
}

/**
Expand All @@ -152,7 +152,8 @@ enum class Scope(
* ADMIN scope, (TRANSLATION_VIEW, KEYS_EDIT, etc.)
*
*/
fun expand(permittedScopes: Array<Scope>): Array<Scope> {
fun expand(permittedScopes: Array<Scope>?): Array<Scope> {
permittedScopes ?: return arrayOf()
return permittedScopes.flatMap { expand(it).toList() }.toSet().toTypedArray()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,7 @@ class LanguageService(
projectId: Long,
userId: Long,
): Set<LanguageDto> {
val canViewTranslations =
permissionService.getProjectPermissionScopes(projectId, userId)?.contains(Scope.TRANSLATIONS_VIEW) == true
val canViewTranslations = securityService.currentPermittedScopesContain(Scope.TRANSLATIONS_VIEW)

if (!canViewTranslations) {
return emptySet()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class ApiKeyService(
project: Project,
): Array<Scope> {
val permittedScopes =
permissionService.getProjectPermissionScopes(project.id, userAccountId)
permissionService.getProjectPermissionScopesNoApiKey(project.id, userAccountId)
?: throw NotFoundException()
return Scope.expand(permittedScopes)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,12 @@ class PermissionService(
return cachedPermissionService.find(id)
}

fun getProjectPermissionScopes(
fun getProjectPermissionScopesNoApiKey(
projectId: Long,
userAccount: UserAccount,
) = getProjectPermissionScopes(projectId, userAccount.id)
) = getProjectPermissionScopesNoApiKey(projectId, userAccount.id)

fun getProjectPermissionScopes(
fun getProjectPermissionScopesNoApiKey(
projectId: Long,
userAccountId: Long,
): Array<Scope>? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import io.tolgee.dtos.cacheable.UserAccountDto
import io.tolgee.exceptions.LanguageNotPermittedException
import io.tolgee.exceptions.NotFoundException
import io.tolgee.exceptions.PermissionException
import io.tolgee.model.ApiKey
import io.tolgee.model.Project
import io.tolgee.model.UserAccount
import io.tolgee.model.enums.Scope
import io.tolgee.model.translation.Translation
import io.tolgee.repository.KeyRepository
import io.tolgee.security.ProjectHolder
import io.tolgee.security.authentication.AuthenticationFacade
import io.tolgee.service.LanguageService
import org.springframework.beans.factory.annotation.Autowired
Expand All @@ -23,6 +23,7 @@ class SecurityService(
private val authenticationFacade: AuthenticationFacade,
private val languageService: LanguageService,
private val keyRepository: KeyRepository,
private val projectHolder: ProjectHolder,
) {
@set:Autowired
lateinit var apiKeyService: ApiKeyService
Expand All @@ -35,13 +36,47 @@ class SecurityService(

fun checkAnyProjectPermission(projectId: Long) {
if (
getProjectPermissionScopes(projectId).isNullOrEmpty() &&
getProjectPermissionScopesNoApiKey(projectId).isNullOrEmpty() &&
!isCurrentUserServerAdmin()
) {
throw PermissionException(Message.USER_HAS_NO_PROJECT_ACCESS)
}
}

fun currentPermittedScopesContain(scope: Scope): Boolean {
return currentPermittedScopesContain(listOf(scope))
}

fun currentPermittedScopesContain(scopes: Collection<Scope>?): Boolean {
scopes ?: return true
return getCurrentPermittedScopes(projectHolder.project.id).containsAll(scopes)
}

/**
* Returns current permitted scopes, expanded
*/
fun getCurrentPermittedScopes(projectId: Long): Set<Scope> {
val projectScopes =
Scope.expand(
getProjectPermissionScopesNoApiKey(projectId, authenticationFacade.authenticatedUser.id),
).toSet()
val apiKey = activeApiKey ?: return projectScopes

return Scope.expand(apiKey.scopes).toSet().intersect(projectScopes.toSet())
}

fun checkProjectPermission(
projectId: Long,
requiredPermission: Scope,
) {
// Always check for the current user even if we're using an API key for security reasons.
// This prevents improper preservation of permissions.
checkProjectPermissionNoApiKey(projectId, requiredPermission, activeUser)

val apiKey = activeApiKey ?: return
checkProjectPermission(projectId, requiredPermission, apiKey)
}

fun checkProjectPermission(
projectId: Long,
requiredScopes: Scope,
Expand All @@ -67,7 +102,7 @@ class SecurityService(
}

val allowedScopes =
getProjectPermissionScopes(projectId, userAccountDto.id)
getProjectPermissionScopesNoApiKey(projectId, userAccountDto.id)
?: throw PermissionException(Message.USER_HAS_NO_PROJECT_ACCESS)

checkPermission(requiredScope, allowedScopes)
Expand All @@ -85,18 +120,6 @@ class SecurityService(
}
}

fun checkProjectPermission(
projectId: Long,
requiredPermission: Scope,
) {
// Always check for the current user even if we're using an API key for security reasons.
// This prevents improper preservation of permissions.
checkProjectPermissionNoApiKey(projectId, requiredPermission, activeUser)

val apiKey = activeApiKey ?: return
checkProjectPermission(projectId, requiredPermission, apiKey)
}

fun checkLanguageViewPermissionByTag(
projectId: Long,
languageTags: Collection<String>,
Expand Down Expand Up @@ -269,19 +292,6 @@ class SecurityService(
checkProjectPermission(projectId, Scope.TRANSLATIONS_EDIT)
}

fun fixInvalidApiKeyWhenRequired(apiKey: ApiKey) {
val oldSize = apiKey.scopesEnum.size
apiKey.scopesEnum.removeIf {
getProjectPermissionScopes(
apiKey.project.id,
apiKey.userAccount.id,
)?.contains(it) != true
}
if (oldSize != apiKey.scopesEnum.size) {
apiKeyService.save(apiKey)
}
}

fun checkApiKeyScopes(
scopes: Set<Scope>,
apiKey: ApiKeyDto,
Expand Down Expand Up @@ -322,11 +332,11 @@ class SecurityService(
checkProjectPermission(projectId, Scope.SCREENSHOTS_UPLOAD)
}

fun getProjectPermissionScopes(
fun getProjectPermissionScopesNoApiKey(
projectId: Long,
userId: Long = activeUser.id,
): Array<Scope>? {
return permissionService.getProjectPermissionScopes(projectId, userId)
return permissionService.getProjectPermissionScopesNoApiKey(projectId, userId)
}

fun checkKeyIdsExistAndIsFromProject(
Expand Down
Loading

0 comments on commit f712139

Please sign in to comment.