-
-
Notifications
You must be signed in to change notification settings - Fork 302
feat: supporter user role #3248
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a SERVER_SUPPORTER role, read‑only mode and annotations, JWT actor/read‑only claims, safer impersonation and token refresh APIs, interceptors enforcing read/write rules, permission check updates across organization/project services, OpenAPI/access-mode metadata, HATEOAS auth info, client UI updates, and related tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant AdminCtrl as AdministrationController
participant AuthF as AuthenticationFacade
participant UserSvc as UserAccountService
participant Jwt as JwtService
Client->>AdminCtrl: POST /administration/users/{id}/token
AdminCtrl->>AuthF: authentication (user, actingUser, isReadOnly)
alt already impersonating
AdminCtrl-->>Client: 409 ALREADY_IMPERSONATING_USER
else
AdminCtrl->>UserSvc: find target user
alt supporter tries impersonate admin target
AdminCtrl-->>Client: 403 IMPERSONATION_OF_ADMIN_BY_SUPPORTER_NOT_ALLOWED
else allowed
AdminCtrl->>Jwt: emitImpersonationToken(targetId)
Jwt->>AuthF: read current authentication
Jwt-->>AdminCtrl: token(with act.sub, d.id, ro)
AdminCtrl-->>Client: 200 { token }
end
end
sequenceDiagram
autonumber
actor Client
participant Filter as AuthenticationFilter
participant JwtSrv as JwtService
participant ROInt as ReadOnlyModeInterceptor
participant AdminInt as AdminAccessInterceptor
participant Controller as ControllerHandler
Client->>Filter: request with Authorization: Bearer <token>
Filter->>JwtSrv: validateJwt(token)
JwtSrv-->>Filter: TolgeeAuthentication(isReadOnly, role, actor)
Filter->>ROInt: preHandle(request, handler)
alt isReadOnly && handler not read-only
ROInt-->>Client: 403 OPERATION_NOT_PERMITTED_IN_READ_ONLY_MODE
else
ROInt-->>Filter: ok
Filter->>AdminInt: preHandle(request, handler)
alt isAdmin
AdminInt-->>Filter: ok
else isSupporter && handler read-only
AdminInt-->>Filter: ok
else
AdminInt-->>Client: 403 OPERATION_NOT_PERMITTED
end
end
Note over Controller: controller executes if passed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
🔇 Additional comments (2)
Comment |
…oject and organization endpoints with read only interceptor since other interceptors already handle that
…rejected requests logging
…re permissions requierd by endpoints - more idiomatic behavior and easier to understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
e2e/cypress/e2e/administration/base.cy.ts (1)
118-122
: Add E2E test coverage for the Supporter role.The function signature now accepts 'Supporter', but no existing test exercises this new role. Given the PR's known limitations around read-only mode enforcement in the frontend and the complexity of the supporter permission model, E2E test coverage is essential to verify:
- Supporter role can be assigned via the admin UI
- Supporter users have appropriate read-only access
- Write operations correctly fail for supporter users
Consider adding a test case that assigns the Supporter role and verifies the expected behavior.
Would you like me to generate a test case for the Supporter role, or open a new issue to track this task?
backend/data/src/main/kotlin/io/tolgee/service/security/MfaService.kt (3)
64-64
: Inconsistent MFA check pattern.Line 64 still uses the old pattern
user.totpKey?.isNotEmpty() != true
while line 41 was updated to useuser.isMfaEnabled
. For consistency and maintainability, update this line to use the same property.Apply this diff to align with the new pattern:
- if (user.totpKey?.isNotEmpty() != true) { + if (!user.isMfaEnabled) { throw BadRequestException(Message.MFA_NOT_ENABLED) }
96-98
: Simplify by delegating toisMfaEnabled
.The
hasMfaEnabled
method calls the companionmfaEnabled(user.totpKey)
, which duplicates the logic thatuser.isMfaEnabled
presumably encapsulates. For consistency, delegate directly to the property.Apply this diff to simplify:
fun hasMfaEnabled(user: UserAccount): Boolean { - return mfaEnabled(user.totpKey) + return user.isMfaEnabled }
125-125
: Inconsistent MFA check pattern.Line 125 still uses the old pattern
user.totpKey?.isNotEmpty() != true
while the rest of the file is being refactored to useuser.isMfaEnabled
. For consistency, update this line as well.Apply this diff to align with the new pattern:
- if (user.totpKey?.isNotEmpty() != true) return true + if (!user.isMfaEnabled) return true
🧹 Nitpick comments (17)
backend/data/src/main/kotlin/io/tolgee/service/security/MfaService.kt (1)
153-155
: Remove redundant mfaEnabled(ByteArray?) in MfaService
Only referenced internally at line 97 and duplicating user.isMfaEnabled; delete this helper and call user.isMfaEnabled directly.webapp/src/views/userSettings/apiKeys/EditApiKeyDialog.tsx (2)
88-100
: Add defensive check for scopes property.The type assertion
as Scopes
at line 89 assumesapiKeyLoadable.data.scopes
is always defined. While the form only renders whenapiKeyLoadable.data
exists (line 118), thescopes
property itself could theoretically beundefined
ornull
depending on the API schema.Consider adding a defensive check or fallback:
const getInitialValues = () => { - const scopes = apiKeyLoadable.data?.scopes as Scopes; + const scopes = (apiKeyLoadable.data?.scopes ?? []) as Scopes; const currentScopes = scopes?.filter((currentScope) => availableScopes.has(currentScope) );This ensures the code handles edge cases gracefully without runtime errors.
137-143
: Handle empty scopes gracefully. WhenavailableScopes
is empty, the multi-select renders no options or feedback, leaving just the “Scopes” label. For read-only/supporter users, either hide or disable this field and display a helper text (e.g. “No scopes available”).backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2InvitationController.kt (1)
66-91
: Consider adding @WriteOperation annotation.The
deleteInvitation
endpoint performs a write operation (deletion) but lacks the@WriteOperation
annotation, unlikeacceptInvitation
at line 57. While the explicitisReadOnlyAccess = false
check provides protection, adding@WriteOperation
would make the security constraints more declarative and consistent.Apply this diff to add the annotation:
@DeleteMapping("/v2/invitations/{invitationId}") +@WriteOperation @Operation(summary = "Deletes invitation by ID") fun deleteInvitation(
backend/api/src/main/kotlin/io/tolgee/controllers/PublicController.kt (1)
55-55
: Drop the unusedAuthenticationFacade
dependency.The controller never references
authenticationFacade
, so the extra constructor parameter/import just expands the bean graph without benefit. Please remove it (or wire it in if a follow-up change will use it) to keep the controller lean.backend/security/src/main/kotlin/io/tolgee/security/authentication/ReadOnlyModeExtension.kt (1)
3-6
: Consider merged/class-level annotations and locale-safe HTTP method handlingCurrent checks only method-level annotations and use default-locale uppercasing. Improve robustness:
- Use AnnotatedElementUtils to detect merged and class-level annotations.
- Normalize method with Locale.ROOT and use a Set for O(1) membership.
Apply this diff:
-import org.springframework.core.annotation.AnnotationUtils +import org.springframework.core.annotation.AnnotatedElementUtils import org.springframework.web.method.HandlerMethod +import java.util.Locale -val READ_ONLY_METHODS = arrayOf("GET", "HEAD", "OPTIONS") +private val READ_ONLY_METHODS: Set<String> = setOf("GET", "HEAD", "OPTIONS") fun HandlerMethod.isReadOnly(httpMethod: String): Boolean { - val forceReadOnly = AnnotationUtils.getAnnotation(method, ReadOnlyOperation::class.java) != null - val forceWrite = AnnotationUtils.getAnnotation(method, WriteOperation::class.java) != null + val forceReadOnly = + AnnotatedElementUtils.hasAnnotation(method, ReadOnlyOperation::class.java) || + AnnotatedElementUtils.hasAnnotation(beanType, ReadOnlyOperation::class.java) + val forceWrite = + AnnotatedElementUtils.hasAnnotation(method, WriteOperation::class.java) || + AnnotatedElementUtils.hasAnnotation(beanType, WriteOperation::class.java) if (forceReadOnly && forceWrite) { - // This doesn't make sense - throw RuntimeException( + throw IllegalStateException( "Both `@ReadOnlyOperation` and `@WriteOperation` have been set for this endpoint!", ) } if (forceWrite) { return false } if (forceReadOnly) { return true } - return httpMethod.uppercase() in READ_ONLY_METHODS + return httpMethod.uppercase(Locale.ROOT) in READ_ONLY_METHODS }Also applies to: 12-32
backend/security/src/test/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptorTest.kt (3)
134-146
: Good coverage of role gating; minor naming nitThe endpoint methods are named requiresAdmin* but assert OWNER role. Consider renaming to requiresOwner* for clarity (optional).
148-189
: Supporter/admin read-only vs write cases well coveredSolid assertions for supporter bypass on read-only and denial on write; and admin access. Consider adding a case: admin with read-only token attempting write → Forbidden, to pin down hasAdminAccess(isReadonlyAccess=false) behavior.
I can add this test if desired.
219-279
: Controller scaffolding is fine; optional minor cleanupReturning string bodies is fine for tests. Optionally instantiate the controller (standaloneSetup(TestController())) to avoid relying on class-based setup semantics.
backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt (2)
77-95
: 404 vs 403 distinction is well-appliedGreat: no-view yields 404 unless admin can at least read-only bypass; then 403 for write operations. This avoids resource enumeration.
Cache handler.isReadOnly(request.method) in a local to avoid recomputation if needed in multiple checks.
60-60
: Remove unused localuser
val user = authenticationFacade.authenticatedUser
is never used.Apply this diff:
- val user = authenticationFacade.authenticatedUser val userId = authenticationFacade.authenticatedUser.id
backend/security/src/main/kotlin/io/tolgee/security/authentication/AdminAccessInterceptor.kt (1)
29-32
: Clarify the class KDoc.The comment still describes read-only enforcement, but this interceptor now governs admin vs supporter access. Updating the description would avoid confusion for future readers.
backend/data/src/main/kotlin/io/tolgee/model/enums/Scope.kt (2)
55-56
: Optimize read-only scope membership lookupUse a Set to avoid linear contains checks and clarify intent.
- private val readOnlyScopes by lazy { ALL_VIEW.expand() } + private val readOnlyScopes by lazy { ALL_VIEW.expand().toSet() }
7-7
: Unnecessary importImport of kotlin.arrayOf is redundant in Kotlin.
-import kotlin.arrayOf
backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthentication.kt (1)
80-89
: Avoid creating a new GrantedAuthority on every getAuthorities() callCache the RO/RW authority once; it’s immutable per token.
- private val authorityFromIsReadOnly: GrantedAuthority - get() { - return SimpleGrantedAuthority( - if (isReadOnly) { - ROLE_RO - } else { - ROLE_RW - } - ) - } + private val authorityFromIsReadOnly: GrantedAuthority = + SimpleGrantedAuthority(if (isReadOnly) ROLE_RO else ROLE_RW)backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt (1)
81-91
: Optional: write acting-user id as String for consistencyAlternatively, normalize the claim at write-time to a String to keep claim typing uniform.
- if (actingAsUserAccountId != null) { - builder.claim(JWT_TOKEN_ACTING_USER_ID_CLAIM, actingAsUserAccountId) - } + if (actingAsUserAccountId != null) { + builder.claim(JWT_TOKEN_ACTING_USER_ID_CLAIM, actingAsUserAccountId.toString()) + }backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt (1)
52-71
: Supporter view bypass is intentional—verify coverageSwitch to user.isSupporterOrAdmin() for view access looks correct for the new role. Please ensure endpoints relying on “can view” are covered by tests for supporter and non-member users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (97)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/AdministrationController.kt
(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/ApiKeyController.kt
(3 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/BusinessEventController.kt
(1 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/InitialDataController.kt
(3 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/SlugController.kt
(3 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserMfaController.kt
(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2InvitationController.kt
(3 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2UserController.kt
(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/batch/V2ExportController.kt
(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/keys/KeyController.kt
(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/organization/OrganizationController.kt
(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/project/ProjectsController.kt
(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/project/ProjectsTransferringController.kt
(1 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/suggestion/TranslationSuggestionController.kt
(2 hunks)backend/api/src/main/kotlin/io/tolgee/component/PreferredOrganizationFacade.kt
(3 hunks)backend/api/src/main/kotlin/io/tolgee/controllers/AuthProviderChangeController.kt
(1 hunks)backend/api/src/main/kotlin/io/tolgee/controllers/PublicController.kt
(4 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/auth/AuthInfoModel.kt
(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/auth/AuthInfoModelAssembler.kt
(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/initialData/InitialDataModel.kt
(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/project/ProjectModelAssembler.kt
(1 hunks)backend/api/src/main/kotlin/io/tolgee/hateoas/project/ProjectWithStatsModelAssembler.kt
(1 hunks)backend/app/src/main/kotlin/io/tolgee/configuration/WebSecurityConfig.kt
(6 hunks)backend/app/src/main/kotlin/io/tolgee/configuration/openApi/OpenApiConfiguration.kt
(3 hunks)backend/app/src/main/kotlin/io/tolgee/configuration/openApi/OpenApiGroupBuilder.kt
(5 hunks)backend/app/src/main/kotlin/io/tolgee/configuration/openApi/OpenApiSecurityHelper.kt
(1 hunks)backend/app/src/test/kotlin/io/tolgee/AuthTest.kt
(1 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/AdministrationControllerTest.kt
(3 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.kt
(2 hunks)backend/app/src/test/kotlin/io/tolgee/security/ProjectApiKeyAuthenticationTest.kt
(1 hunks)backend/app/src/test/kotlin/io/tolgee/security/ServerAdminFilterTest.kt
(1 hunks)backend/app/src/test/kotlin/io/tolgee/service/LanguageServiceTest.kt
(1 hunks)backend/app/src/test/kotlin/io/tolgee/service/dataImport/ImportServiceTest.kt
(1 hunks)backend/app/src/test/kotlin/io/tolgee/service/dataImport/StoredDataImporterTest.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/constants/ComputedPermissionOrigin.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/constants/Message.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/AdministrationTestData.kt
(2 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/ComputedPermissionDto.kt
(3 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/cacheable/UserAccountDto.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/UserAccount.kt
(2 hunks)backend/data/src/main/kotlin/io/tolgee/model/enums/OrganizationRoleType.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/enums/ProjectPermissionType.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/enums/Scope.kt
(6 hunks)backend/data/src/main/kotlin/io/tolgee/repository/ProjectRepository.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/security/authentication/AuthenticationFacade.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt
(8 hunks)backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthentication.kt
(3 hunks)backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthenticationDetails.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/StartupImportService.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt
(6 hunks)backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationService.kt
(2 hunks)backend/data/src/main/kotlin/io/tolgee/service/security/MfaService.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/security/PermissionService.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt
(10 hunks)backend/data/src/main/kotlin/io/tolgee/service/security/SignUpService.kt
(1 hunks)backend/data/src/test/kotlin/io/tolgee/security/authentication/JwtServiceTest.kt
(3 hunks)backend/development/src/main/kotlin/io/tolgee/controllers/internal/e2eData/SensitiveOperationProtectionE2eDataController.kt
(1 hunks)backend/security/src/main/kotlin/io/tolgee/security/authentication/AdminAccessInterceptor.kt
(1 hunks)backend/security/src/main/kotlin/io/tolgee/security/authentication/AuthenticationFilter.kt
(3 hunks)backend/security/src/main/kotlin/io/tolgee/security/authentication/ReadOnlyModeExtension.kt
(1 hunks)backend/security/src/main/kotlin/io/tolgee/security/authentication/ReadOnlyModeInterceptor.kt
(1 hunks)backend/security/src/main/kotlin/io/tolgee/security/authentication/ReadOnlyOperation.kt
(1 hunks)backend/security/src/main/kotlin/io/tolgee/security/authentication/WriteOperation.kt
(1 hunks)backend/security/src/main/kotlin/io/tolgee/security/authorization/AbstractAuthorizationInterceptor.kt
(3 hunks)backend/security/src/main/kotlin/io/tolgee/security/authorization/FeatureAuthorizationInterceptor.kt
(2 hunks)backend/security/src/main/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptor.kt
(5 hunks)backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt
(5 hunks)backend/security/src/test/kotlin/io/tolgee/security/authentication/AdminAccessInterceptorTest.kt
(1 hunks)backend/security/src/test/kotlin/io/tolgee/security/authentication/AuthenticationFilterTest.kt
(1 hunks)backend/security/src/test/kotlin/io/tolgee/security/authentication/ReadOnlyModeInterceptorTest.kt
(1 hunks)backend/security/src/test/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptorTest.kt
(7 hunks)backend/security/src/test/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptorTest.kt
(6 hunks)backend/testing/src/main/kotlin/io/tolgee/testing/AuthorizedControllerTest.kt
(1 hunks)e2e/cypress/e2e/administration/base.cy.ts
(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/AiPromptCustomizationController.kt
(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/PromptController.kt
(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/SsoProviderController.kt
(2 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/SuggestionController.kt
(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/TaskController.kt
(2 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/glossary/GlossaryController.kt
(1 hunks)ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/glossary/GlossaryTermHighlightsController.kt
(2 hunks)webapp/src/component/PermissionsSettings/usePermissionsStructure.ts
(1 hunks)webapp/src/component/PermissionsSettings/useScopeTranslations.tsx
(1 hunks)webapp/src/component/common/useFeatureMissingExplanation.tsx
(1 hunks)webapp/src/component/layout/TopBar/announcements/AdministrationAccessAnnouncement.tsx
(1 hunks)webapp/src/component/layout/TopBar/announcements/DebuggingCustomerAccountAnnouncement.tsx
(2 hunks)webapp/src/component/security/UserMenu/UserPresentAvatarMenu.tsx
(3 hunks)webapp/src/ee/billing/component/UserMenu/BillingMenuItem.tsx
(1 hunks)webapp/src/globalContext/helpers.tsx
(1 hunks)webapp/src/service/apiSchema.generated.ts
(16 hunks)webapp/src/translationTools/useErrorTranslation.ts
(1 hunks)webapp/src/views/administration/components/RoleSelector.tsx
(1 hunks)webapp/src/views/organizations/OrganizationsRouter.tsx
(2 hunks)webapp/src/views/organizations/components/BaseOrganizationSettingsView.tsx
(3 hunks)webapp/src/views/projects/ProjectListView.tsx
(2 hunks)webapp/src/views/projects/ProjectPage.tsx
(1 hunks)webapp/src/views/userSettings/apiKeys/EditApiKeyDialog.tsx
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-28T15:26:38.207Z
Learnt from: Anty0
PR: tolgee/tolgee-platform#3043
File: backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt:155-164
Timestamp: 2025-04-28T15:26:38.207Z
Learning: In the OrganizationRoleService class, the function `isUserMember` checks if a user has any role in an organization (owner, maintainer, or member) by verifying if the role type is not null. It replaced the previous `isUserMemberOrOwner` function to simplify naming while maintaining the same behavior.
Applied to files:
backend/data/src/main/kotlin/io/tolgee/model/enums/OrganizationRoleType.kt
backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt
🧬 Code graph analysis (19)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/AdministrationTestData.kt (1)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/builders/TestDataBuilder.kt (1)
addUserAccount
(36-51)
webapp/src/component/common/useFeatureMissingExplanation.tsx (1)
webapp/src/globalContext/helpers.tsx (1)
useIsAdmin
(21-22)
backend/app/src/test/kotlin/io/tolgee/AuthTest.kt (1)
backend/testing/src/main/kotlin/io/tolgee/testing/AuthorizedControllerTest.kt (1)
setForcedDate
(158-161)
webapp/src/component/layout/TopBar/announcements/AdministrationAccessAnnouncement.tsx (2)
webapp/src/globalContext/helpers.tsx (1)
useIsSupporter
(24-27)webapp/src/component/layout/TopBar/announcements/TopBarAnnouncementWithIcon.tsx (1)
TopBarAnnouncementWithAlertIcon
(29-35)
backend/security/src/main/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptor.kt (1)
backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt (1)
canBypass
(190-197)
webapp/src/views/projects/ProjectListView.tsx (1)
webapp/src/globalContext/helpers.tsx (1)
useIsAdminOrSupporter
(29-33)
webapp/src/views/userSettings/apiKeys/EditApiKeyDialog.tsx (2)
webapp/src/component/common/form/StandardForm.tsx (1)
StandardForm
(37-121)webapp/src/constants/GlobalValidationSchema.tsx (1)
Validation
(44-524)
webapp/src/views/organizations/OrganizationsRouter.tsx (1)
webapp/src/globalContext/helpers.tsx (1)
useIsAdminOrSupporter
(29-33)
backend/data/src/main/kotlin/io/tolgee/model/enums/Scope.kt (1)
backend/data/src/main/kotlin/io/tolgee/model/enums/OrganizationRoleType.kt (1)
isReadOnly
(3-9)
backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt (1)
backend/data/src/main/kotlin/io/tolgee/service/security/PermissionService.kt (2)
getProjectPermissionScopesNoApiKey
(86-89)getProjectPermissionScopesNoApiKey
(91-96)
backend/app/src/test/kotlin/io/tolgee/security/ServerAdminFilterTest.kt (1)
backend/testing/src/main/kotlin/io/tolgee/testing/AuthorizedControllerTest.kt (1)
performAuthGet
(130-133)
backend/app/src/main/kotlin/io/tolgee/configuration/openApi/OpenApiConfiguration.kt (1)
backend/app/src/main/kotlin/io/tolgee/configuration/openApi/OpenApiGroupBuilder.kt (1)
customizeOperations
(180-215)
webapp/src/component/security/UserMenu/UserPresentAvatarMenu.tsx (1)
webapp/src/globalContext/helpers.tsx (1)
useIsAdminOrSupporter
(29-33)
webapp/src/component/layout/TopBar/announcements/DebuggingCustomerAccountAnnouncement.tsx (1)
webapp/src/component/layout/TopBar/announcements/TopBarAnnouncementWithIcon.tsx (1)
TopBarAnnouncementWithAlertIcon
(29-35)
backend/security/src/test/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptorTest.kt (1)
backend/security/src/test/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptorTest.kt (2)
performReadOnlyRequests
(191-201)performWriteRequests
(203-210)
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/AdministrationControllerTest.kt (1)
backend/testing/src/main/kotlin/io/tolgee/testing/AuthorizedControllerTest.kt (4)
performPost
(98-104)performAuthGet
(130-133)performGet
(91-96)performAuthPut
(114-120)
backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt (1)
backend/security/src/main/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptor.kt (1)
canBypass
(144-150)
backend/security/src/test/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptorTest.kt (1)
backend/security/src/test/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptorTest.kt (2)
performReadOnlyRequests
(301-313)performWriteRequests
(315-328)
webapp/src/views/organizations/components/BaseOrganizationSettingsView.tsx (1)
webapp/src/globalContext/helpers.tsx (1)
useIsAdminOrSupporter
(29-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Frontend static check 🪲
- GitHub Check: Build backend 🏗️
🔇 Additional comments (96)
backend/app/src/test/kotlin/io/tolgee/security/ProjectApiKeyAuthenticationTest.kt (1)
146-146
: LGTM! Named parameter improves clarity.The change from positional to named parameter (
isSuper = true
) makes the intent of generating a super-user token explicit, improving code readability without altering behavior.ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/AiPromptCustomizationController.kt (1)
84-84
: Documentation correction approved.The summary now correctly describes this GET endpoint as returning language-level prompt customizations, replacing the misleading "Sets project level" text.
backend/development/src/main/kotlin/io/tolgee/controllers/internal/e2eData/SensitiveOperationProtectionE2eDataController.kt (2)
36-36
: LGTM! Named parameter improves clarity.The refactoring from a positional boolean to the named parameter
isSuper = true
makes the intent explicit and improves code readability, which is especially valuable in test data generation contexts.
43-43
: LGTM! Consistent named parameter usage.The use of
isSuper = false
maintains consistency with line 36 and ensures uniform adoption of named parameters across allemitToken
call sites in this file.ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/SsoProviderController.kt (2)
7-7
: LGTM!Import is necessary for the
isAdmin()
extension method used on line 49.
49-49
: LGTM!The refactoring from direct role comparison to the
isAdmin()
extension method is a clean improvement. It centralizes the admin check logic, making the code more maintainable and aligning with the broader supporter role implementation pattern introduced in this PR.The security semantics are preserved: only admin users will be allowed to change the SSO domain, while supporter users (read-only) will correctly be restricted.
webapp/src/component/layout/TopBar/announcements/DebuggingCustomerAccountAnnouncement.tsx (2)
5-8
: LGTM!The addition of
useGlobalContext
import is necessary for the read-only mode check and is used correctly in the component.
21-23
: LGTM!The read-only mode detection correctly uses optional chaining and explicit boolean comparison to safely access the authentication info from global context.
backend/data/src/main/kotlin/io/tolgee/service/security/MfaService.kt (1)
41-41
: LGTM! Improved consistency.Using
user.isMfaEnabled
is clearer and more maintainable than directly checkinguser.totpKey?.isNotEmpty() == true
.webapp/src/views/userSettings/apiKeys/EditApiKeyDialog.tsx (2)
115-117
: LGTM: Simplified loading state check.The removal of the
availableScopesLoadable
check streamlines the loading logic, as scopes are now derived directly fromprojectLoadable.data
. This change is consistent with the broader refactoring.
84-86
: Verify fallback for computedPermission.scopes. The newavailableScopes
Set will be empty ifcomputedPermission
or itsscopes
isnull
/undefined
, causing all current scopes to be filtered out. Confirm thatcomputedPermission.scopes
is always defined whenprojectLoadable.data
exists or add an explicit default before constructingavailableScopes
.ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/SuggestionController.kt (1)
50-50
: LGTM!The suppression annotation resolves IDE false positives for Spring Data's
PagedResourcesAssembler
autowiring with generics—a common pattern in Spring projects.webapp/src/ee/billing/component/UserMenu/BillingMenuItem.tsx (1)
27-32
: LGTM!The condition correctly extends billing menu access to users with
SUPPORTER
role, aligning with the PR's objective to implement supporter functionality.webapp/src/component/PermissionsSettings/useScopeTranslations.tsx (1)
53-53
: LGTM!The new
all.view
scope translation correctly supports the expanded permission taxonomy introduced in this PR.ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/PromptController.kt (1)
45-45
: LGTM!The suppression annotation resolves IDE false positives for
PagedResourcesAssembler
autowiring—consistent with the pattern used across EE controllers.webapp/src/views/projects/ProjectPage.tsx (1)
25-27
: LGTM!The logic correctly grants admin-level access to both
SERVER_ADMIN
andSERVER_SUPPORTER
origins, consistent with the supporter role implementation.webapp/src/component/layout/TopBar/announcements/AdministrationAccessAnnouncement.tsx (1)
4-17
: LGTM!The component correctly uses
useIsSupporter
to conditionally render the appropriate announcement message, providing role-specific messaging for supporters.backend/api/src/main/kotlin/io/tolgee/component/PreferredOrganizationFacade.kt (2)
35-45
: LGTM! Read-only mode handling implemented correctly.The helper method correctly prevents modification operations (findOrCreate) in read-only mode by using
find
instead, aligning with the PR's read-only semantics.
22-22
: LGTM! Null-safe handling.The safe navigation operator correctly handles cases where
getCurrentUserPreferences()
returns null in read-only mode when preferences don't exist.backend/api/src/main/kotlin/io/tolgee/hateoas/auth/AuthInfoModel.kt (1)
1-7
: LGTM! Clean HATEOAS model for read-only state.The
AuthInfoModel
class is well-structured as a simple data carrier for conveying read-only authentication state in HATEOAS responses. The singleisReadOnly
Boolean property and extension ofRepresentationModel
are appropriate for this use case.backend/data/src/main/kotlin/io/tolgee/model/enums/OrganizationRoleType.kt (1)
3-9
: Verify that MEMBER role should always be read-only.The implementation hardcodes
MEMBER(true)
as read-only whileOWNER(false)
andMAINTAINER(false)
have write access. This aligns with typical RBAC patterns where members have view-only access.However, please confirm this matches the intended business logic, especially considering:
- Whether all organization members should be restricted to read-only access
- If there are scenarios where members need write permissions at the organization level
- How this interacts with project-level permissions (which may grant write access independently)
Based on learnings, the
OrganizationRoleService.isUserMember
function checks for any role (owner, maintainer, or member). Ensure this read-only flag doesn't inadvertently restrict members who should have write access through other permission paths.backend/data/src/main/kotlin/io/tolgee/constants/Message.kt (1)
311-313
: LGTM! New error messages support the supporter role feature.The three new enum constants appropriately support the supporter role and read-only mode implementation:
IMPERSONATION_OF_ADMIN_BY_SUPPORTER_NOT_ALLOWED
: Enforces security boundary preventing supporters from impersonating adminsALREADY_IMPERSONATING_USER
: Validates impersonation stateOPERATION_NOT_PERMITTED_IN_READ_ONLY_MODE
: Enforces read-only mode restrictionsbackend/data/src/main/kotlin/io/tolgee/model/UserAccount.kt (2)
164-176
: LGTM! Supporter role logic correctly implements read-only access.The
hasAdminAccess
method properly implements the supporter role semantics:
ADMIN
: Always has admin access (both read-only and write)SUPPORTER
: Only has admin access in read-only mode- Other roles: No admin access
This design allows supporters to view admin-level data without the ability to make changes, which aligns with the PR objectives.
188-202
: LGTM! Clean extension functions for role checks.The extension functions provide a clean, type-safe API for checking user roles:
- Clear naming (e.g.,
isAdmin()
,isSupporter()
)- Proper null safety with
?: false
fallback- Delegation to the enum's
hasAdminAccess
method maintains single source of truthbackend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt (4)
52-59
: LGTM! Read-only access check properly integrated.The
checkAnyProjectPermission
method now correctly accepts anisReadonlyAccess
parameter and useshasCurrentUserServerAdminAccess(isReadonlyAccess)
to determine if the user has appropriate admin-level access. This allows supporters to access project information in read-only mode.
219-303
: LGTM! Read-only flags correctly mapped to operation types.The read-only access flags are correctly assigned based on operation semantics:
TRANSLATIONS_VIEW
:isReadonlyAccess = true
(Lines 232, 245)TRANSLATIONS_SUGGEST
:isReadonlyAccess = false
(Line 221)TRANSLATIONS_EDIT
:isReadonlyAccess = false
(Line 270)TRANSLATIONS_STATE_EDIT
:isReadonlyAccess = false
(Line 302)This ensures supporters can view translations but cannot modify them.
339-353
: LGTM! Generalized admin access check supports read-only mode.The refactoring from
isCurrentUserServerAdmin()
tohasCurrentUserServerAdminAccess(isReadonlyAccess)
properly generalizes the admin check to support both full admin and read-only supporter access. The private helper correctly delegates to the user'shasAdminAccess
method.Also applies to: 554-556
158-160
: No action required: Scope.isReadOnly() is implemented correctly.
Scope.isReadOnly()
is defined inScope.kt
(lines 51, 254) and returns true for all view/read scopes and false for edit/write scopes.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/BusinessEventController.kt (1)
37-37
: LGTM! Correct use of read-only access for event reporting.The addition of
isReadonlyAccess = true
to thecheckAnyProjectPermission
call is appropriate because reporting business events is a read operation. This allows supporters to view business events without requiring write permissions.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/suggestion/TranslationSuggestionController.kt (2)
100-100
: LGTM! Appropriate use of @ReadOnlyOperation.The
@ReadOnlyOperation
annotation correctly marks thesuggestTranslationMemory
method as read-only, since it retrieves suggestions without modifying any data.
54-91
: Consider adding @ReadOnlyOperation to machine translation methods.The
suggestMachineTranslations
(lines 61-66) andsuggestMachineTranslationsStreaming
(lines 79-91) methods appear to be read-only operations as well, since they generate suggestions without modifying stored translations.Should these methods also be annotated with
@ReadOnlyOperation
for consistency?If these methods trigger any side effects (e.g., logging, caching, external API calls that count against quotas), they might not be purely read-only. Please verify whether they should be marked as read-only operations.
backend/data/src/main/kotlin/io/tolgee/service/security/PermissionService.kt (1)
476-482
: Controller handles authorization The only caller of removeDirectProjectPermissions is in ProjectsController, which is annotated with @RequiresProjectPermissions([Scope.MEMBERS_EDIT]) and @RequiresSuperAuthentication and loads the project via projectHolder, covering the removed membership and validation checks.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/batch/V2ExportController.kt (1)
13-13
: LGTM! ReadOnlyOperation annotation is appropriate.The
@ReadOnlyOperation
annotation correctly marks the export endpoint as read-only, which is semantically accurate for data export operations.Also applies to: 74-74
backend/testing/src/main/kotlin/io/tolgee/testing/AuthorizedControllerTest.kt (1)
69-69
: LGTM! Named parameter improves clarity.The change to use the named parameter
isSuper = true
enhances code readability by making the intent explicit.backend/data/src/main/kotlin/io/tolgee/service/security/SignUpService.kt (1)
42-42
: LGTM! Named parameter improves clarity.The change to use the named parameter
isSuper = true
enhances code readability by making the intent explicit.backend/app/src/test/kotlin/io/tolgee/AuthTest.kt (1)
260-260
: LGTM! Named parameters improve test clarity.The changes to use named parameters
isSuper = false
andisSuper = true
make the test's intent explicit and improve readability.Also applies to: 267-267
ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/glossary/GlossaryTermHighlightsController.kt (1)
15-15
: LGTM! ReadOnlyOperation annotation is appropriate.The
@ReadOnlyOperation
annotation correctly marks the glossary highlights endpoint as read-only, which is semantically accurate for this read operation.Also applies to: 37-37
backend/data/src/main/kotlin/io/tolgee/repository/ProjectRepository.kt (1)
66-66
: Approve – SUPPORTER read-only access correctly scoped
Including SUPPORTER alongside ADMIN in the organization-scoped project query aligns with existing authorization rules (supporters bypass read-only checks but are forbidden for write). Verified usage inProjectAuthorizationInterceptorTest
,OrganizationAuthorizationInterceptorTest
, andhasAdminAccess
logic.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2UserController.kt (1)
120-120
: Impersonation context not implemented
There’s no acting-user or impersonation logic in the codebase, so no validation of context clearing is required.Likely an incorrect or invalid review comment.
backend/api/src/main/kotlin/io/tolgee/hateoas/project/ProjectModelAssembler.kt (1)
73-74
: Confirm null userRole is handled safely
computeProjectPermission
declaresuserRole: UserAccount.Role?
and, when null, skips admin‐permission grants and returns the base computed permission. PassingauthenticatedUserOrNull?.role
is safe.webapp/src/translationTools/useErrorTranslation.ts (1)
210-211
: LGTM!The new error translation case is correctly placed and follows the established pattern for mapping error codes to translation keys.
backend/data/src/main/kotlin/io/tolgee/service/StartupImportService.kt (1)
83-90
: LGTM!The updated
TolgeeAuthentication
construction correctly supplies the new fields with appropriate values for the startup import context (no device, no acting-as-user, not read-only, not super token).ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/TaskController.kt (1)
317-317
: LGTM!The
@ReadOnlyOperation
annotation is correctly applied to thecalculateScope
method, which performs a calculation without modifying data. This aligns with the PR's read-only mode implementation.ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/glossary/GlossaryController.kt (1)
56-59
: LGTM!The
@Suppress
annotations appropriately silence IDE autowiring warnings for Spring-managedPagedResourcesAssembler
beans. This is a common pattern when working with Spring framework types.backend/data/src/main/kotlin/io/tolgee/constants/ComputedPermissionOrigin.kt (1)
9-9
: LGTM!The new
SERVER_SUPPORTER
enum constant is correctly added to support the supporter role. The placement afterSERVER_ADMIN
is logical and maintains enum consistency.backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.kt (2)
249-249
: LGTM!The test correctly updates to access
isSuperToken
directly on the authentication object, reflecting the refactored authentication structure where this property moved fromTolgeeAuthenticationDetails
toTolgeeAuthentication
.
267-267
: LGTM!Consistent with the previous change, this test correctly accesses
isSuperToken
directly on the authentication object.backend/api/src/main/kotlin/io/tolgee/controllers/AuthProviderChangeController.kt (1)
82-82
: LGTM!The token emission correctly updates to use
emitTokenRefreshForCurrentUser(isSuper = true)
, which leverages the current authentication context and aligns with the PR's refactored token emission pattern. The named parameter improves code clarity.backend/app/src/main/kotlin/io/tolgee/configuration/openApi/OpenApiSecurityHelper.kt (1)
20-20
: LGTM!The lambda signature correctly updates to accept the additional parameter from the expanded
customizeOperations
API. The underscore convention appropriately indicates the parameter is unused in this context.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserMfaController.kt (2)
40-40
: LGTM! Token emission method updated.The replacement of
jwtService.emitToken(authenticationFacade.authenticatedUser.id, true)
withjwtService.emitTokenRefreshForCurrentUser(isSuper = true)
aligns with the broader token emission API evolution. The new method derives the user ID from the current authentication context, maintaining the same behavior while improving encapsulation.
56-56
: LGTM! Consistent token emission update.Same token emission method update as in
enableMfa
, maintaining consistency across MFA operations.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/keys/KeyController.kt (1)
35-35
: LGTM! Read-only annotation correctly applied.The
@ReadOnlyOperation
annotation on thegetInfo
endpoint is semantically correct, as the POST method is used only for request body convenience (retrieving key information without mutation).Also applies to: 297-297
webapp/src/views/projects/ProjectListView.tsx (1)
14-17
: LGTM! Supporter role integrated correctly.The replacement of
useIsAdmin
withuseIsAdminOrSupporter
correctly extends admin-like access to supporters. TheisAdminAccess
logic properly gates access when the user has no organization role but is an admin or supporter.Also applies to: 59-62
webapp/src/component/security/UserMenu/UserPresentAvatarMenu.tsx (1)
7-7
: LGTM! Admin menu item access extended to supporters.The change correctly extends server administration menu access to supporters by using
useIsAdminOrSupporter
instead of a direct role check. This aligns with the PR's goal of providing supporters with admin-like access.Also applies to: 55-55, 164-164
backend/data/src/main/kotlin/io/tolgee/model/enums/ProjectPermissionType.kt (1)
87-87
: LGTM! Modernized enum iteration.The change from
values()
toentries
is a recommended modernization in Kotlin 1.9+.EnumEntries
provides better type safety and performance compared to the deprecatedvalues()
array. No functional change, only improved idiomatic Kotlin.Also applies to: 92-92
backend/data/src/main/kotlin/io/tolgee/dtos/cacheable/UserAccountDto.kt (1)
47-61
: LGTM—Role.hasAdminAccess verified. The enumRole
defineshasAdminAccess(isReadonlyAccess: Boolean)
, so the delegation is correct. Extension functions provide clean role-based permission helpers with proper null-safety.webapp/src/component/PermissionsSettings/usePermissionsStructure.ts (1)
35-37
: LGTM — translation key verified
Found'permissions_item_all_view'
mapped in useScopeTranslations.tsx.backend/api/src/main/kotlin/io/tolgee/hateoas/project/ProjectWithStatsModelAssembler.kt (1)
43-44
: LGTM. This change pulls in the actualauthenticatedUserOrNull?.role
, andcomputeProjectPermission
already accepts a nullableUserAccount.Role?
and handlesnull
by falling back to base permissions.webapp/src/views/organizations/components/BaseOrganizationSettingsView.tsx (1)
15-15
: LGTM! Role check correctly broadened to include supporters.The refactoring correctly replaces
useIsAdmin
withuseIsAdminOrSupporter
, allowing both admin and supporter roles to manage organizations. The implementation is consistent throughout the file.Also applies to: 39-39, 53-53
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/ApiKeyController.kt (2)
10-10
: LGTM! Cleaner admin check using extension function.The refactoring from direct role comparison to the
isAdmin()
extension function improves maintainability and aligns with the broader permission model changes in this PR.Also applies to: 80-82
315-316
: LGTM! Modern Kotlin enum iteration.The change from
values()
toentries
follows Kotlin's modern best practices and is functionally equivalent.webapp/src/views/organizations/OrganizationsRouter.tsx (1)
8-8
: LGTM! Admin access correctly includes supporters.The refactoring properly extends admin access checks to include the supporter role, maintaining the existing logic structure while broadening the permission scope.
Also applies to: 20-24
backend/app/src/test/kotlin/io/tolgee/service/dataImport/ImportServiceTest.kt (1)
123-130
: LGTM! Test updated to match new authentication constructor.The change from positional to named parameters improves test readability and aligns with the enhanced authentication model. All parameters are correctly set for the test scenario.
backend/app/src/test/kotlin/io/tolgee/service/dataImport/StoredDataImporterTest.kt (1)
45-52
: LGTM! Test authentication setup aligned with new model.The named parameter constructor call matches the pattern used across other updated tests and correctly initializes all authentication fields for the test context.
webapp/src/globalContext/helpers.tsx (1)
24-33
: LGTM! Well-designed role check hooks.The new
useIsSupporter
anduseIsAdminOrSupporter
hooks follow the established pattern and provide clean abstractions for role-based UI logic. The implementation correctly checks theglobalServerRole
field.webapp/src/component/common/useFeatureMissingExplanation.tsx (1)
4-4
: LGTM! Refactored to use centralized admin check.The change improves maintainability by using the
useIsAdmin
hook instead of an inline role check, keeping the logic consistent across the application.Also applies to: 8-8
backend/security/src/main/kotlin/io/tolgee/security/authentication/AuthenticationFilter.kt (1)
117-124
: LGTM! Authentication construction updated to named parameters.All three
TolgeeAuthentication
constructor calls have been correctly converted to named parameters, improving readability and maintainability. The new fields (deviceId
,actingAsUserAccount
,isReadOnly
,isSuperToken
) are appropriately initialized for each authentication scenario:
- Disabled auth:
isSuperToken=true
- PAK auth: credentials set to PAK,
isSuperToken=false
- PAT auth: credentials set to PAT,
isSuperToken=false
Also applies to: 168-175, 196-203
backend/app/src/test/kotlin/io/tolgee/security/ServerAdminFilterTest.kt (1)
18-30
: LGTM! Well-structured test for supporter role access.The test correctly verifies that users with the SUPPORTER role can access administration endpoints. The implementation mirrors the existing admin test pattern and appropriately validates the new role's read access permissions.
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/AdministrationTestData.kt (1)
9-9
: LGTM! Consistent test data expansion.The supporter user addition follows the established pattern for admin and user test data, providing necessary test fixtures for supporter role scenarios.
Also applies to: 23-28
backend/security/src/main/kotlin/io/tolgee/security/authentication/ReadOnlyOperation.kt (1)
19-27
: LGTM! Clear annotation for read-only operation override.The annotation appropriately marks operations that use non-safe HTTP methods but are semantically read-only (e.g., POST for complex queries). Unlike
@WriteOperation
, this annotation lacks@PreAuthorize
, which is correct—it serves as a marker for interceptor logic (likelyReadOnlyModeInterceptor
) rather than declarative security enforcement.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/SlugController.kt (1)
50-50
: LGTM! Appropriate use of @ReadOnlyOperation.Both slug generation endpoints correctly use
@ReadOnlyOperation
since they employ POST for request body convenience but perform no state modifications—they simply compute and return slugs.Also applies to: 60-60
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/AdministrationControllerTest.kt (3)
92-101
: LGTM! Validates write access with admin-generated token.The test extension appropriately verifies that tokens generated by admins for users grant full write access, confirming the token generation endpoint works correctly for standard admin impersonation.
137-150
: LGTM! Comprehensive supporter permission validation.Both tests correctly validate the supporter role's read-only access pattern:
- Read endpoints (organizations, users) are accessible
- Write endpoints (set-role, enable, disable) are forbidden
This properly enforces the supporter role's limited privileges.
103-126
: Verify supporter-generated JWT sets read-only flag.Ensure the
/v2/administration/users/{id}/generate-token
endpoint sets theisReadOnly
claim in the JWT when invoked by a supporter.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2InvitationController.kt (2)
57-57
: LGTM! Appropriate @WriteOperation annotation.Accepting an invitation is a state-changing operation that correctly requires write access.
83-86
: LGTM! Proper read-only access check for write operation.The
isReadOnlyAccess = false
parameter correctly ensures that users with read-only access cannot delete organization invitations, maintaining consistency with the write-operation security model.backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthenticationDetails.kt (1)
19-21
: Refactoring approved: isSuperToken relocation verified. No remainingdetails.isSuperToken
references;isSuperToken
is correctly defined inTolgeeAuthentication
.backend/security/src/main/kotlin/io/tolgee/security/authentication/WriteOperation.kt (1)
19-27
: Approve WriteOperation annotation
It correctly enforcesROLE_RW
via@PreAuthorize("hasRole('RW')")
, andTolgeeAuthentication
grantsROLE_RW
when not read-only.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/project/ProjectsTransferringController.kt (1)
75-76
: Explicit non-read-only ownership check looks goodHard-coding
isReadOnlyAccess = false
ensures we never accept a read-only supporter token for this write path. 👍backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/organization/OrganizationController.kt (1)
107-113
: Switch toisAdmin()
is correctReplacing raw role checks with the shared
isAdmin()
helper keeps the admin semantics centralized without altering behavior. ✔️backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/InitialDataController.kt (2)
37-39
: LGTM — assembler DI is correct and cohesiveConstructor injection of AuthInfoModelAssembler fits existing pattern and keeps the controller lean.
54-54
: Populate authInfo only when authenticated — goodSetting data.authInfo via assembler under the user-present guard is appropriate. Ensure InitialDataModel.authInfo is nullable to avoid serialization issues when unauthenticated.
backend/api/src/main/kotlin/io/tolgee/hateoas/auth/AuthInfoModelAssembler.kt (1)
8-18
: Straightforward assembler — minimal and correctMaps isReadOnly cleanly; no unnecessary coupling or links. Good bean registration.
backend/security/src/test/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptorTest.kt (2)
74-83
: Mock setup looks correct; minor check on thenCallRealMethodStubbing authenticationFacade.authentication and then calling real method on isReadOnly is fine if the method isn’t final. Please confirm isReadOnly isn’t final/static to avoid Mockito issues.
191-215
: Helper composition is cleanandSatisfies improves readability; grouping requests reduces duplication.
backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt (4)
72-75
: Clearer logging of requirements — niceUsing “read-only” when no scopes required makes logs actionable.
97-114
: Missing scopes path correctly returns PermissionExceptionGood differentiation from the no-view case; bypass honored consistently.
141-146
: Context propagation maintainedProject/organization holders and activity revision are set even when bypassing. Good.
187-197
: Bypass helper is clear; API key guard is rightDisallowing admin bypass for project API key auth is correct. Helper encapsulation is good.
backend/security/src/test/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptorTest.kt (3)
259-299
: Good coverage for supporter/read-only/admin flowsScenarios clearly validate read-only bypass and write restrictions. Looks solid.
Consider adding a HEAD-case for write-annotated endpoints for parity with GET coverage.
301-333
: Helpful test helpersperformReadOnlyRequests/performWriteRequests reduce duplication; andSatisfies is neat.
91-100
: Real method on isReadOnlyStubbing isReadOnly to call the real method on a mock is fine if the method is not final. If AuthenticationFacade.isReadOnly is final, Mockito-inline or a spy may be needed.
backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthentication.kt (1)
59-78
: Confirm RO/RW authorities aren’t used as standalone gatesAppending ROLE_RO/ROLE_RW to all tokens (even when userAccount.role is null) is fine if endpoints always also require ROLE_USER/ADMIN/SUPPORTER. Please verify no endpoint authorizes solely on RO/RW.
backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt (3)
116-130
: Admin override honors read-only—confirm hasAdminAccess semanticscheckUserIsOwner() now relies on hasAdminAccess(isReadOnlyAccess). Verify hasAdminAccess returns false for supporters and for admins with read-only tokens.
132-146
: Same as above for maintainer pathEnsure hasAdminAccess(isReadOnlyAccess) cannot elevate a supporter to write paths and blocks admin when read-only.
148-154
: Ensure read-only flag is correctly propagated
In V2InvitationController.kt:83, ProjectsTransferringController.kt:75 and ProjectsController.kt:88 you’re passingisReadOnlyAccess = false
. Confirm this is intentional or replace with the actual read-only state (e.g.authenticationFacade.isReadOnly
).
backend/app/src/main/kotlin/io/tolgee/configuration/WebSecurityConfig.kt
Show resolved
Hide resolved
backend/data/src/main/kotlin/io/tolgee/dtos/ComputedPermissionDto.kt
Outdated
Show resolved
Hide resolved
backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt
Show resolved
Hide resolved
backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt
Show resolved
Hide resolved
backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt
Show resolved
Hide resolved
backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationService.kt
Show resolved
Hide resolved
...nd/security/src/test/kotlin/io/tolgee/security/authentication/ReadOnlyModeInterceptorTest.kt
Show resolved
Hide resolved
webapp/src/component/layout/TopBar/announcements/DebuggingCustomerAccountAnnouncement.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/security/src/main/kotlin/io/tolgee/security/authentication/AdminAccessInterceptor.kt (1)
48-52
: Clarify the comment.The comment on line 50 says "If not in read-only mode, allow" but the condition checks
isAdmin()
, which grants write access to administrators. The comment should more accurately describe the logic.Apply this diff to clarify:
val hasWriteAccess = authenticationFacade.authenticatedUser.isAdmin() if (hasWriteAccess) { - // If not in read-only mode, allow + // Admin users have full access, allow return true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/data/src/main/kotlin/io/tolgee/dtos/ComputedPermissionDto.kt
(3 hunks)backend/data/src/main/kotlin/io/tolgee/model/enums/Scope.kt
(6 hunks)backend/security/src/main/kotlin/io/tolgee/security/authentication/AdminAccessInterceptor.kt
(1 hunks)backend/security/src/main/kotlin/io/tolgee/security/authentication/ReadOnlyModeInterceptor.kt
(1 hunks)webapp/src/service/apiSchema.generated.ts
(16 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/data/src/main/kotlin/io/tolgee/model/enums/Scope.kt (1)
backend/data/src/main/kotlin/io/tolgee/model/enums/OrganizationRoleType.kt (1)
isReadOnly
(3-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Frontend static check 🪲
- GitHub Check: Build backend 🏗️
🔇 Additional comments (4)
backend/security/src/main/kotlin/io/tolgee/security/authentication/AdminAccessInterceptor.kt (4)
29-37
: LGTM!The class documentation clearly explains the interceptor's purpose, and the dependency injection is correctly configured.
43-46
: LGTM!Correctly defers to other interceptors when the user is not authenticated.
54-58
: LGTM with a note on known limitation.The logic correctly allows supporters read-only access. Note that this relies on HTTP method inspection via
handler.isReadOnly(request.method)
, which aligns with the known limitation mentioned in the PR description: "the server-side check currently inspects the HTTP method rather than actual permission state."
60-60
: LGTM!Correctly denies access by throwing
PermissionException
when the user lacks the necessary privileges.
backend/security/src/main/kotlin/io/tolgee/security/authentication/ReadOnlyModeInterceptor.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am very confused by the name readOnly
. I did not have finished the review I believe that we have to discuss this first before I continue.
backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt
Outdated
Show resolved
Hide resolved
backend/data/src/main/kotlin/io/tolgee/dtos/cacheable/UserAccountDto.kt
Outdated
Show resolved
Hide resolved
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/AdministrationController.kt
Outdated
Show resolved
Hide resolved
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/AdministrationController.kt
Outdated
Show resolved
Hide resolved
backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt
Outdated
Show resolved
Hide resolved
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/AdministrationController.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt (2)
187-196
: Inconsistent language permission bypass for supporters/admins.The
checkLanguageViewPermissionByTag
method does not wrap its language permission check withrunIfUserNotServerSupporterOrAdmin
, unlikecheckLanguageViewPermission
(lines 226-230). This creates inconsistent behavior:
- When checking by language IDs: supporters and admins bypass language restrictions
- When checking by language tags: supporters and admins do NOT bypass language restrictions
This inconsistency could lead to unexpected permission denials when supporters use tag-based operations.
Apply this diff to add the missing wrapper:
fun checkLanguageViewPermissionByTag( projectId: Long, languageTags: Collection<String>, ) { checkProjectPermission(projectId, Scope.TRANSLATIONS_VIEW) - checkLanguagePermissionByTag( - projectId, - languageTags, - ) { data, languageIds -> data.checkViewPermitted(*languageIds.toLongArray()) } + runIfUserNotServerSupporterOrAdmin { + checkLanguagePermissionByTag( + projectId, + languageTags, + ) { data, languageIds -> data.checkViewPermitted(*languageIds.toLongArray()) } + } }
198-207
: Inconsistent language permission bypass for admins.The
checkLanguageTranslatePermissionByTag
method does not wrap its language permission check withrunIfUserNotServerAdmin
, unlikecheckLanguageTranslatePermission
(lines 266-270). This creates inconsistent behavior where admins bypass language restrictions for ID-based operations but not for tag-based operations.Apply this diff to add the missing wrapper:
fun checkLanguageTranslatePermissionByTag( projectId: Long, languageTags: Collection<String>, ) { checkProjectPermission(projectId, Scope.TRANSLATIONS_EDIT) - checkLanguagePermissionByTag( - projectId, - languageTags, - ) { data, languageIds -> data.checkTranslatePermitted(*languageIds.toLongArray()) } + runIfUserNotServerAdmin { + checkLanguagePermissionByTag( + projectId, + languageTags, + ) { data, languageIds -> data.checkTranslatePermitted(*languageIds.toLongArray()) } + } }
🧹 Nitpick comments (2)
backend/security/src/main/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptor.kt (1)
144-158
: Align bypass guard with Project interceptor (block bypass on API-key auth)ProjectAuthorizationInterceptor uses canUseAdminRights = !isProjectApiKeyAuth to avoid bypass with API keys. Here, bypass is possible without that guard. For parity and safety, add the same guard.
class OrganizationAuthorizationInterceptor( @@ ) : AbstractAuthorizationInterceptor() { private val logger = LoggerFactory.getLogger(this::class.java) + private val canUseAdminRights + get() = !authenticationFacade.isProjectApiKeyAuth + @@ private fun canBypass( request: HttpServletRequest, handler: HandlerMethod, ): Boolean { - if (authenticationFacade.authenticatedUser.isAdmin()) { + if (!canUseAdminRights) { + return false + } + if (authenticationFacade.authenticatedUser.isAdmin()) { return true } val forReadOnly = handler.isReadOnly(request.method) - return forReadOnly && canBypassForReadOnly() + return forReadOnly && canBypassForReadOnly() } private fun canBypassForReadOnly(): Boolean { - return authenticationFacade.authenticatedUser.isSupporterOrAdmin() + return canUseAdminRights && authenticationFacade.authenticatedUser.isSupporterOrAdmin() }backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt (1)
187-208
: Deduplicate bypass logic across interceptorsOrganization and Project interceptors now carry near-identical bypass helpers. Consider extracting to a shared utility/base to avoid divergence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2InvitationController.kt
(3 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/project/ProjectsController.kt
(2 hunks)backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/project/ProjectsTransferringController.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/cacheable/UserAccountDto.kt
(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/UserAccount.kt
(2 hunks)backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt
(6 hunks)backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt
(8 hunks)backend/security/src/main/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptor.kt
(5 hunks)backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/project/ProjectsTransferringController.kt
- backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2InvitationController.kt
- backend/data/src/main/kotlin/io/tolgee/dtos/cacheable/UserAccountDto.kt
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-07T14:36:39.331Z
Learnt from: Anty0
PR: tolgee/tolgee-platform#3263
File: backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt:279-290
Timestamp: 2025-10-07T14:36:39.331Z
Learning: In OrganizationRoleService in backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt, the `removeOrDeactivateUser` method intentionally has two distinct paths: if a user can be removed, it calls `removeUserForReal` (which removes organization roles, project permissions, and evicts caches); if a user is managed by the organization and cannot be removed, it calls `userAccountService.disable(userId)` without any cleanup. This design is intentional—disabling should be reversible so the user stays unmodified (keeping their organization roles and project permissions intact), allowing them to be re-enabled if they return to the organization.
Applied to files:
backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/project/ProjectsController.kt
📚 Learning: 2025-10-03T08:38:27.934Z
Learnt from: Anty0
PR: tolgee/tolgee-platform#3248
File: backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationService.kt:157-157
Timestamp: 2025-10-03T08:38:27.934Z
Learning: The SUPPORTER role is read-only and should not be able to create organizations in the name of other users. When checking if a user can create organizations, use `isAdmin()` rather than `isSupporterOrAdmin()` to exclude SUPPORTER from this capability.
Applied to files:
backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt
🧬 Code graph analysis (2)
backend/security/src/main/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptor.kt (1)
backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt (2)
canBypass
(190-204)canBypassForReadOnly
(206-208)
backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt (1)
backend/security/src/main/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptor.kt (2)
canBypass
(144-154)canBypassForReadOnly
(156-158)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Frontend static check 🪲
- GitHub Check: Build backend 🏗️
🔇 Additional comments (19)
backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt (6)
7-8
: LGTM!The new imports support the supporter role implementation and are used consistently throughout the file.
53-60
: LGTM!Correctly allows both admins and supporters to bypass the project access check, which is appropriate since supporters need read-only access to all projects.
214-218
: LGTM!The wrapper usage correctly implements the permission model:
- Read-only operations (VIEW) allow both admins and supporters to bypass language restrictions via
runIfUserNotServerSupporterOrAdmin
- Write operations (SUGGEST, EDIT, STATE_EDIT) only allow admins to bypass language restrictions via
runIfUserNotServerAdmin
This aligns with the supporter role's read-only access design.
Also applies to: 226-230, 240-244, 266-270, 299-303
550-560
: LGTM!Clean helper methods that improve code readability by encapsulating role-based conditional execution.
159-166
: Consider addressing previous review feedback.A previous review comment suggested creating a separate method
hasCurrentUserServerSupporterAccess
and adding documentation comments. The current implementation uses inline checks instead. While the inline approach works, consider:
- Adding documentation comments to explain the supporter role's read-only bypass logic
- Evaluating whether a dedicated helper method would improve clarity
Was the decision to use inline checks instead of a separate method intentional? If so, please add documentation to explain the permission model for future maintainers.
159-166
: Approve read-only bypass logic
Scope.isReadOnly()
is defined and the early returns for admins and supporter read-only access are correct. Optionally, replaceuserAccountDto.isSupporterOrAdmin()
withuserAccountDto.isSupporter()
for clarity:- if (isReadonlyAccess && userAccountDto.isSupporterOrAdmin()) { + if (isReadonlyAccess && userAccountDto.isSupporter()) {backend/security/src/main/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptor.kt (1)
74-92
: 404 vs 403 for supporters: confirm intentionIf the user can’t view the organization and the endpoint isn’t read-only, supporters get 403 (PermissionException) instead of 404 (NotFound). This leaks existence to SUPPORTERs; is that intended?
backend/data/src/main/kotlin/io/tolgee/model/UserAccount.kt (2)
180-190
: Role helpers look correctExtensions return false for null role and correctly capture ADMIN/SUPPORTER semantics.
164-168
: No DB/migration changes needed for SUPPORTER roleThe
role
column is defined asVARCHAR(255)
without any check constraints or seed inserts, and JPA’s@Enumerated(EnumType.STRING)
mapping already accepts the new enum value.backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt (2)
75-92
: No-view handling + 404/403 split is consistentBehavior mirrors org interceptor; bypass is blocked for API keys via canUseAdminRights. Looks good.
97-114
: Missing scopes computation is correct and clearerSet difference logic is straightforward and avoids unnecessary conversions.
backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt (6)
59-71
: Supporter read-only view allowedShifting view checks to user.isSupporterOrAdmin() is aligned with read-only semantics.
84-88
: canUserView overload is correctShort-circuits for supporter/admin; otherwise defers to repository check.
116-127
: Owner or server admin guard uses isAdmin() (excludes supporter)Correctly restricts privileged operations to owners or server admins, excluding supporters.
Based on learnings
141-152
: Owner/Maintainer or server admin guard — correct splitSUPPORTER is excluded from project-creating/maintaining privileged paths.
Based on learnings
154-156
: New checkUserCanCreateProject API is appropriateEncapsulates owner/maintainer/server-admin logic; controller usage matches.
129-139
: No action needed — methods are in use
checkUserCanDeleteInvitation
is invoked in V2InvitationController.kt, andcheckUserCanTransferProjectToOrganization
in ProjectsTransferringController.kt.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/project/ProjectsController.kt (2)
88-89
: Project creation guard updated — goodUses OrganizationRoleService.checkUserCanCreateProject; excludes supporters from creating projects.
244-251
: No action requiredremoveDirectProjectPermissions
exists inPermissionService
(PermissionService.kt:476-479
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/AdministrationController.kt
(2 hunks)backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt
(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-03T08:37:31.704Z
Learnt from: Anty0
PR: tolgee/tolgee-platform#3248
File: backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt:173-179
Timestamp: 2025-10-03T08:37:31.704Z
Learning: In JwtService.validateToken, the check `if (roClaim && account.isAdmin())` correctly prevents read-only tokens from being used to impersonate admin accounts. The `account` variable represents the target user being impersonated, not the actor. This is the intended behavior to block read-only access to admin accounts.
Applied to files:
backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Frontend static check 🪲
- GitHub Check: Build backend 🏗️
🔇 Additional comments (6)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/AdministrationController.kt (3)
152-165
: LGTM! Impersonation safety checks are well-implemented.The method now correctly:
- Prevents nested impersonation (Inception reference noted!)
- Blocks supporters from impersonating admins
- Delegates to
emitImpersonationToken
for proper token emissionThe past review concerns have been addressed: the method now uses dedicated token emission methods instead of boolean flags, and the condition is written clearly.
164-164
:emitImpersonationToken
already enforces read-only for supporters. TheemitSupporterImpersonationToken
call setsisReadOnly = true
.
152-165
: Validate self-impersonation and admin-to-admin policies
- No tests or docs mention self-impersonation; if unwanted, add
if (userId == actingUser.id) throw BadRequestException(Message.CANNOT_IMPERSONATE_SELF)
.- Admins currently may impersonate other admins—confirm this matches your security model.
- Optional: extract the admin-check into
and use it for clarity.private fun isImpersonationForbidden(actor: UserAccount, target: UserAccount) = target.isAdmin() && !actor.isAdmin()backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt (3)
110-119
: LGTM! Token refresh propagation is secure.The method correctly propagates impersonation context (
actingAsUserAccountId
), read-only mode, and super-token state. DefaultingisSuper
tofalse
is intentional for security, as discussed in past reviews, preventing accidental super-token propagation unless explicitly requested.
121-149
: LGTM! Impersonation token emission aligns with role-based access.The methods correctly implement the security model:
- Admins receive read-write super tokens
- Supporters receive read-only non-super tokens
emitImpersonationToken
dispatches appropriately based on the acting user's roleThe logic at line 144 checking
isSupporterOrAdmin()
after the admin check correctly captures supporters.
204-226
: LGTM! Validation logic correctly enforces impersonation constraints.The validation properly:
- Extracts and validates the actor (impersonator) and device ID
- Blocks read-only impersonation of admin accounts (line 215-218) as confirmed in past reviews
- Invalidates tokens when the actor is demoted after impersonation (lines 220-226)
The permission check at line 221 correctly allows admins to impersonate in read-write mode and supporters to impersonate only in read-only mode.
Based on learnings
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/AdministrationController.kt
Show resolved
Hide resolved
backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt
Show resolved
Hide resolved
…rmissions check for `translationsInTask`
} | ||
|
||
val actingUser = authenticationFacade.authenticatedUser | ||
val user = userAccountService.get(userId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JanCizmar Is there a reason to use the full UserAccount
entity instead of just UserAccountDto
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt (3)
56-56
: Behavior change: supporters can pass “any project permission”This now treats server supporters/admins as having implicit read access even without project membership. Good, but ensure callers don’t rely on this method to distinguish “member vs not” (to avoid unintended disclosure flows).
Consider adding a short KDoc to clarify the semantics: “returns true for supporters/admins even with no project membership”.
159-166
: Supporter read-only bypass in project permission checkEarly-return for admin and for supporter when requiredScope.isReadOnly() looks correct and matches the role intent.
Minor: the admin case is covered by isSupporterOrAdmin() too; you can fold into a single condition or leave as-is for clarity.
549-559
: New helper wrappers: add KDoc and align namingHelpers read well. Add brief KDoc documenting “server-level” semantics and when to prefer each wrapper. Optionally rename to clarify the negative predicate (e.g., doIfNotServerAdmin vs runIfUserNotServerAdmin).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt
(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build backend 🏗️
- GitHub Check: Frontend static check 🪲
🔇 Additional comments (8)
backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt (8)
7-9
: LGTM: importing role helpersUsing isAdmin/isSupporterOrAdmin centralizes role checks and improves readability.
192-197
: View-by-tag: skip per-language checks for supporters/adminsGood optimization; supporters/admins won’t hit language permission data paths.
205-211
: Translate-by-tag: only admins skip per-language checksCorrect: supporters should not bypass write-related checks.
218-223
: Suggest permission: only admins skip per-language checksConsistent with write semantics.
230-235
: View permission: supporters/admins skip per-language checksMatches read-only intent.
243-243
: translationsInTask now enforces view permission firstReasonable guard. It will still no-op for supporters/admins due to the wrapper. Ensure LanguageNotPermittedException flows are still handled by callers (they are, via passIfAnyPermissionCheckSucceeds).
298-303
: State change permission: admin skip retained; supporter checkedConsistent with write semantics; same interceptor caveat as above.
265-269
: Guard write endpoints with @WriteOperation Unannotated POST/PUT/PATCH/DELETE mappings in production controllers (e.g. SuggestionController.kt, ContentStorageController.kt) bypass the global read-only interceptor. Exclude test sources and verify every write mapping is annotated with @WriteOperation.
Implements #3218
Known limitations:
Summary by CodeRabbit
New Features
Security
API Changes