Skip to content

Commit 7ff8091

Browse files
JanCizmarJanCizmar
committed
test(apps): cover the install's own principal
Covers what the principal has to be: an account that lets the install write a comment while the person who registered it is disabled and while they are deleted, that is named after the app, that takes no seat, appears in neither the organization's member list nor the server's user list, cannot be signed in as, and is retired when the install goes. Acting as a disabled user still fails, and an app acting as itself is still recorded against the install. The author-role-leak case now also proves the admin role does not bypass a scope check inside the project the app is enabled for, and the backfill test proves every pre-existing install gets a principal of its own.
1 parent 626a591 commit 7ff8091

3 files changed

Lines changed: 353 additions & 6 deletions

File tree

‎backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/apps/AppInstallPrincipalTest.kt‎

Lines changed: 289 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,289 @@
1+
package io.tolgee.api.v2.controllers.apps
2+
3+
import io.tolgee.constants.Message
4+
import io.tolgee.development.testDataBuilder.data.AppsTestData
5+
import io.tolgee.dtos.request.organization.SetOrganizationRoleDto
6+
import io.tolgee.fixtures.andAssertThatJson
7+
import io.tolgee.fixtures.andHasErrorMessage
8+
import io.tolgee.fixtures.andIsCreated
9+
import io.tolgee.fixtures.andIsForbidden
10+
import io.tolgee.fixtures.andIsOk
11+
import io.tolgee.fixtures.node
12+
import io.tolgee.model.enums.OrganizationRoleType
13+
import io.tolgee.service.apps.AppInstallService
14+
import io.tolgee.service.apps.AppManifestHttpClient
15+
import io.tolgee.service.apps.AppsTestFixtures
16+
import io.tolgee.service.apps.lifecycle.AppLifecycleHttpClient
17+
import io.tolgee.testing.AuthorizedControllerTest
18+
import io.tolgee.testing.assert
19+
import org.junit.jupiter.api.AfterEach
20+
import org.junit.jupiter.api.BeforeEach
21+
import org.junit.jupiter.api.Test
22+
import org.springframework.beans.factory.annotation.Autowired
23+
import org.springframework.data.domain.Pageable
24+
import org.springframework.http.HttpHeaders
25+
import org.springframework.http.MediaType
26+
import org.springframework.test.context.bean.override.mockito.MockitoBean
27+
import org.springframework.test.web.servlet.ResultActions
28+
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder
29+
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
30+
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post
31+
32+
/**
33+
* An install acts as an account of its own, so writing a row that references a user works whatever
34+
* became of the person who registered the app — and that account is not a person: it takes no seat,
35+
* appears in no listing, cannot be signed in as, and goes when the install goes.
36+
*/
37+
class AppInstallPrincipalTest : AuthorizedControllerTest() {
38+
@Autowired
39+
lateinit var appInstallService: AppInstallService
40+
41+
@MockitoBean
42+
@Autowired
43+
lateinit var appManifestHttpClient: AppManifestHttpClient
44+
45+
@MockitoBean
46+
@Autowired
47+
lateinit var appLifecycleHttpClient: AppLifecycleHttpClient
48+
49+
lateinit var testData: AppsTestData
50+
var installId: Long = 0
51+
var principalId: Long = 0
52+
var keyId: Long = 0
53+
lateinit var installToken: String
54+
55+
@BeforeEach
56+
fun setup() {
57+
testData = AppsTestData()
58+
testDataService.saveTestData(testData.root)
59+
userAccount = testData.user
60+
AppsTestFixtures.mockManifest(appManifestHttpClient, MANIFEST)
61+
62+
val json =
63+
objectMapper.readTree(
64+
performAuthPost(
65+
"/v2/organizations/${testData.organization.id}/apps/register",
66+
mapOf("manifestUrl" to AppsTestFixtures.MANIFEST_URL),
67+
).andIsOk.andReturn().response.contentAsString,
68+
)
69+
installId = json.get("id").asLong()
70+
principalId = appInstallService.resolveForAppAuth(installId)!!.principal.id
71+
performAuthPut("/v2/projects/${testData.project.id}/apps/$installId", null).andIsOk
72+
installToken = requestInstallToken(json.get("clientId").asText(), json.get("clientSecret").asText())
73+
74+
keyId =
75+
objectMapper
76+
.readTree(
77+
performAuthPost(
78+
"/v2/projects/${testData.project.id}/translations",
79+
mapOf("key" to "commented-key", "translations" to mapOf("en" to "Hello")),
80+
).andIsOk.andReturn().response.contentAsString,
81+
).get("keyId")
82+
.asLong()
83+
}
84+
85+
@AfterEach
86+
fun cleanup() {
87+
testDataService.cleanTestData(testData.root)
88+
}
89+
90+
@Test
91+
fun `writes a comment as itself rather than as the person who registered it`() {
92+
principalId.assert.isNotEqualTo(testData.user.id)
93+
94+
commentAsApp().andIsCreated.andAssertThatJson {
95+
node("comment.author.id").isEqualTo(principalId)
96+
}
97+
}
98+
99+
@Test
100+
fun `writes a comment while its author is disabled`() {
101+
userAccountService.disable(testData.user.id)
102+
103+
commentAsApp().andIsCreated
104+
}
105+
106+
@Test
107+
fun `writes a comment while its author is deleted`() {
108+
// Somebody else has to own the organization first, or deleting its only owner takes it with them.
109+
organizationRoleService.setMemberRole(
110+
testData.organization.id,
111+
testData.member.id,
112+
SetOrganizationRoleDto(OrganizationRoleType.OWNER),
113+
)
114+
userAccountService.delete(testData.user.id)
115+
116+
commentAsApp().andIsCreated
117+
}
118+
119+
/** The principal is what the install acts as, so it must be obvious in any UI that shows it. */
120+
@Test
121+
fun `names the principal after the app`() {
122+
userAccountService
123+
.findActive(principalId)!!
124+
.name.assert
125+
.isEqualTo("Test App [app]")
126+
}
127+
128+
@Test
129+
fun `takes no seat`() {
130+
val seats = userAccountService.countAllEnabled()
131+
132+
AppsTestFixtures.mockManifest(appManifestHttpClient, SECOND_MANIFEST)
133+
performAuthPost(
134+
"/v2/organizations/${testData.organization.id}/apps/register",
135+
mapOf("manifestUrl" to AppsTestFixtures.MANIFEST_URL),
136+
).andIsOk
137+
138+
userAccountService.countAllEnabled().assert.isEqualTo(seats)
139+
}
140+
141+
@Test
142+
fun `is listed neither among the organization's members nor among the server's users`() {
143+
val members: List<Long> =
144+
objectMapper
145+
.readTree(
146+
performAuthGet("/v2/organizations/${testData.organization.id}/users?size=1000")
147+
.andIsOk
148+
.andReturn()
149+
.response.contentAsString,
150+
).path("_embedded")
151+
.path("usersInOrganization")
152+
.values()
153+
.map { it.path("id").asLong() }
154+
155+
members.assert.contains(testData.user.id)
156+
members.assert.doesNotContain(principalId)
157+
158+
userAccountService
159+
.findAllWithDisabledPaged(Pageable.ofSize(1000), null)
160+
.content
161+
.map { it.id }
162+
.assert
163+
.doesNotContain(principalId)
164+
}
165+
166+
@Test
167+
fun `cannot be signed in as`() {
168+
val username = userAccountService.findActive(principalId)!!.username
169+
170+
userAccountService.findActive(username).assert.isNull()
171+
userAccountService.findActiveOrDisabled(username).assert.isNull()
172+
}
173+
174+
@Test
175+
fun `is retired when the install is removed`() {
176+
performAuthDelete("/v2/organizations/${testData.organization.id}/apps/$installId").andIsOk
177+
178+
userAccountService.findActive(principalId).assert.isNull()
179+
}
180+
181+
/**
182+
* Acting as a person is the one path where a person's state still gates the request — the
183+
* install's own principal must not paper over it.
184+
*/
185+
@Test
186+
fun `refuses to act as a disabled user`() {
187+
userAccountService.disable(testData.member.id)
188+
189+
logout()
190+
perform(
191+
commentRequest()
192+
.header(HttpHeaders.AUTHORIZATION, "Bearer $installToken")
193+
.header(ACT_AS_USER_HEADER, testData.member.id.toString()),
194+
).andIsForbidden.andHasErrorMessage(Message.APP_ACTING_AS_USER_NOT_PROJECT_MEMBER)
195+
}
196+
197+
@Test
198+
fun `keeps attributing its own change to the install and to nobody`() {
199+
userAccountService.disable(testData.user.id)
200+
commentAsApp().andIsCreated
201+
202+
// [0] is the key the owner created in setup; the app's comment follows it.
203+
asApp(get("/v2/projects/${testData.project.id}/activity")).andIsOk.andAssertThatJson {
204+
node("_embedded.activities[1].type").isEqualTo("TRANSLATION_COMMENT_ADD")
205+
node("_embedded.activities[1].app.installId").isEqualTo(installId)
206+
node("_embedded.activities[1].author").isNull()
207+
}
208+
}
209+
210+
private fun commentAsApp(): ResultActions = asApp(commentRequest())
211+
212+
private fun commentRequest(): MockHttpServletRequestBuilder {
213+
return post("/v2/projects/${testData.project.id}/translations/create-comment")
214+
.contentType(MediaType.APPLICATION_JSON)
215+
.content(
216+
objectMapper.writeValueAsString(
217+
mapOf(
218+
"keyId" to keyId,
219+
"languageId" to testData.englishLanguage.id,
220+
"text" to "written by the app",
221+
),
222+
),
223+
)
224+
}
225+
226+
private fun asApp(builder: MockHttpServletRequestBuilder): ResultActions {
227+
logout()
228+
return perform(builder.header(HttpHeaders.AUTHORIZATION, "Bearer $installToken"))
229+
}
230+
231+
private fun requestInstallToken(
232+
clientId: String,
233+
clientSecret: String,
234+
): String {
235+
logout()
236+
val response =
237+
perform(
238+
post("/v2/public/apps/token")
239+
.contentType(MediaType.APPLICATION_JSON)
240+
.content(
241+
objectMapper.writeValueAsString(
242+
mapOf(
243+
"grant_type" to "client_credentials",
244+
"client_id" to clientId,
245+
"client_secret" to clientSecret,
246+
),
247+
),
248+
),
249+
).andIsOk.andReturn().response.contentAsString
250+
userAccount = testData.user
251+
return objectMapper.readTree(response).get("access_token").asText()
252+
}
253+
254+
companion object {
255+
private const val ACT_AS_USER_HEADER = "X-Tolgee-Act-As-User-Id"
256+
257+
private val MANIFEST: String =
258+
"""
259+
{
260+
"id": "test-app",
261+
"name": "Test App",
262+
"version": "0.1.0",
263+
"baseUrl": "https://app.example.com",
264+
"scopes": ["translations.edit", "translation-comments.add", "activity.view"],
265+
"modules": {
266+
"project-dashboard-page": [
267+
{"key": "home", "title": "Home", "icon": "🏠", "entry": "/"}
268+
]
269+
}
270+
}
271+
""".trimIndent()
272+
273+
private val SECOND_MANIFEST: String =
274+
"""
275+
{
276+
"id": "second-app",
277+
"name": "Second App",
278+
"version": "0.1.0",
279+
"baseUrl": "https://second.example.com",
280+
"scopes": ["translations.edit"],
281+
"modules": {
282+
"project-dashboard-page": [
283+
{"key": "home", "title": "Home", "icon": "🏠", "entry": "/"}
284+
]
285+
}
286+
}
287+
""".trimIndent()
288+
}
289+
}

‎backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/apps/AppTokenAuthorizationTest.kt‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ package io.tolgee.api.v2.controllers.apps
22

33
import io.tolgee.constants.Message
44
import io.tolgee.development.testDataBuilder.data.AppsTestData
5+
import io.tolgee.dtos.request.organization.SetOrganizationRoleDto
56
import io.tolgee.fixtures.andHasErrorMessage
67
import io.tolgee.fixtures.andIsForbidden
78
import io.tolgee.fixtures.andIsOk
89
import io.tolgee.fixtures.andIsUnauthorized
910
import io.tolgee.model.UserAccount
11+
import io.tolgee.model.enums.OrganizationRoleType
1012
import io.tolgee.security.authentication.AppTokenService
1113
import io.tolgee.service.apps.AppManifestHttpClient
1214
import io.tolgee.service.apps.AppsTestFixtures
@@ -167,7 +169,23 @@ class AppTokenAuthorizationTest : AuthorizedControllerTest() {
167169
asApp(get("/v2/projects/${testData.project.id}/translations")).andIsOk
168170
}
169171

170-
/** The author's server role must not reach the install, whatever it is. */
172+
@Test
173+
fun `keeps working once its author has been deleted`() {
174+
// Somebody else has to own the organization first, or deleting its only owner takes it with them.
175+
organizationRoleService.setMemberRole(
176+
testData.organization.id,
177+
testData.member.id,
178+
SetOrganizationRoleDto(OrganizationRoleType.OWNER),
179+
)
180+
userAccountService.delete(testData.user.id)
181+
182+
asApp(get("/v2/projects/${testData.project.id}/translations")).andIsOk
183+
}
184+
185+
/**
186+
* The author's server role must not reach the install, whatever it is — neither to reach a project
187+
* the app was never enabled for, nor to bypass a scope check inside one it was.
188+
*/
171189
@Test
172190
fun `does not gain the author's server-admin privileges`() {
173191
val author = userAccountService.get(testData.user.id)
@@ -177,6 +195,12 @@ class AppTokenAuthorizationTest : AuthorizedControllerTest() {
177195
asApp(get("/v2/projects/${testData.siblingProject.id}/translations"))
178196
.andIsForbidden
179197
.andHasErrorMessage(Message.APP_NOT_ENABLED_FOR_PROJECT)
198+
199+
asApp(
200+
post("/v2/projects/${testData.project.id}/translations")
201+
.contentType(MediaType.APPLICATION_JSON)
202+
.content("""{"key":"brand-new-key","translations":{"en":"Hello"}}"""),
203+
).andIsForbidden.andHasErrorMessage(Message.OPERATION_NOT_PERMITTED)
180204
}
181205

182206
@Test

0 commit comments

Comments
 (0)