-
-
Notifications
You must be signed in to change notification settings - Fork 305
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
WalkthroughIntroduces a SUPPORTER server role and read-only access mode across backend and webapp. Adds read-only/write operation annotations and interceptors, expands JWT to carry actor/read-only flags, updates permission/role checks, extends OpenAPI tooling to mark access mode, adjusts APIs (rename removeDirectProjectPermissions), adds HATEOAS AuthInfoModel, and updates UI/schema accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as Server
participant Jwt as JwtService
participant Auth as AuthenticationFacade
Note over C,S: Token issuance with impersonation & read-only
C->>S: POST /admin/impersonate { userId }
S->>Auth: get authenticated user (actor)
alt already impersonating
S-->>C: 400 ALREADY_IMPERSONATING_USER
else actor not allowed
S-->>C: 403 IMPERSONATION_OF_ADMIN_BY_SUPPORTER_NOT_ALLOWED
else allowed
S->>Jwt: emitImpersonationToken(userId)
Jwt-->>S: JWT (claims: act.sub, ro?, d.id, super?)
S-->>C: 200 { token }
end
Note over C,S: Token validation on request
C->>S: Request with Authorization: Bearer <JWT>
S->>Jwt: validateToken(jwt)
Jwt->>Jwt: parse claims (sub, act.sub, ro, d.id, super)
Jwt->>Auth: build TolgeeAuthentication (isReadOnly, actor, isSuperToken)
Jwt-->>S: Authentication
S-->>C: proceeds to endpoint handling
sequenceDiagram
autonumber
participant C as Client
participant S as Server
participant RO as ReadOnlyModeInterceptor
participant AA as AdminAccessInterceptor
participant Ctl as Controller
Note over C,S: Request access evaluation
C->>S: HTTP method + path
S->>RO: preHandle(handler, method)
alt unauthenticated or not read-only
RO-->>S: allow
else handler.isReadOnly(method) == true
RO-->>S: allow
else
RO-->>S: throw OPERATION_NOT_PERMITTED_IN_READ_ONLY_MODE
end
S->>AA: preHandle(handler, method)
alt user is ADMIN
AA-->>S: allow
else user is SUPPORTER and handler.isReadOnly(method)
AA-->>S: allow
else
AA-->>S: throw OPERATION_NOT_PERMITTED
end
S->>Ctl: invoke endpoint
Ctl-->>C: response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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)
Comment |
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
.../api/src/main/kotlin/io/tolgee/api/v2/controllers/administration/AdministrationController.kt
Show resolved
Hide resolved
backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt
Show resolved
Hide resolved
@@ -156,7 +155,7 @@ class AdministrationController( | |||
} | |||
|
|||
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.
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.
This is exceptional work! ❤️ Thanks a lot! I just had few final comments, but it looks much much better now.
private val publicInvitationModelAssembler: PublicInvitationModelAssembler, | ||
) { | ||
@GetMapping("/v2/invitations/{code}/accept") | ||
@WriteOperation |
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.
Oh crap! Why is the method GET... 🤦 My bad...
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.
Should I add the POST version of the endpoint and mark the GET version as deprecated? ^^
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2UserController.kt
Outdated
Show resolved
Hide resolved
backend/api/src/main/kotlin/io/tolgee/component/PreferredOrganizationFacade.kt
Outdated
Show resolved
Hide resolved
return authentication.userAccountView | ||
} | ||
|
||
// -- ACTING USER |
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.
Please use documentation comments instead of this weirdo comments and explain what is the purpose of each property.
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'm just following the existing style there (as created by Cynthia). What would you recommend I use to split the file into sections? Or should I remove the sections and add a separate doc string for each property?
backend/api/src/main/kotlin/io/tolgee/controllers/PublicController.kt
Outdated
Show resolved
Hide resolved
*/ | ||
fun emitToken( | ||
userAccountId: Long, | ||
actingAsUserAccountId: Long? = null, |
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.
This is just a nitpick, but I want to mention it so we follow it in the future: I think we should not pass booleans in this case. Instead we can refactor this method so we:
- create a base method that generates the bas builder
- we add the claims in the other methods
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.
Builder for token. I like this. I might just do it here anyway. It will fix the whole mess of "if you set the isSuper
of emitTokenRefreshForCurrentUser
to null
it will keep the super token same as it is rn" since now I can just make a function for it. I'll return to this one.
backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthentication.kt
Outdated
Show resolved
Hide resolved
val baseline = currentDateProvider.date | ||
currentDateProvider.forcedDate = Date(baseline.time - authenticationProperties.jwtSuperExpiration - 10_000) | ||
val expiredToken = jwtService.emitToken(data.franta.id, true) | ||
val expiredToken = jwtService.emitToken(data.franta.id, isSuper = true) |
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 would prefer having method "emitSuperToken" without passing the boolean, but it's just a nitpick.
...ckend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/AiPromptCustomizationController.kt
Show resolved
Hide resolved
...security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.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: 1
🧹 Nitpick comments (5)
backend/api/src/main/kotlin/io/tolgee/controllers/PublicController.kt (1)
15-15
: Remove unused AuthenticationFacade injection
TheAuthenticationFacade
import (line 15) and constructor parameter (line 55) inPublicController.kt
are never used; remove them.backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt (2)
78-92
: Clarify the comment for supporter role.The logic correctly handles the security consideration to hide project existence for users without access. However, the comment on line 90 states "Admin access for read-only operations is allowed" but this applies to supporters as well, not just admins.
Consider updating the comment for clarity:
- // Admin access for read-only operations is allowed, but it's not enough for the current operation. + // Supporter/Admin access for read-only operations is allowed, but it's not enough for the current operation.
187-208
: Consider simplifying canBypassForReadOnly.The
canBypassForReadOnly()
method checkscanUseAdminPermissions
, but this condition is already verified by the only caller (canBypass()
at line 194) before reaching the call at line 203. While this doesn't cause incorrect behavior, it's redundant.Apply this diff to remove the redundant check:
private fun canBypassForReadOnly(): Boolean { - return canUseAdminPermissions && authenticationFacade.authenticatedUser.isSupporterOrAdmin() + return authenticationFacade.authenticatedUser.isSupporterOrAdmin() }backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthentication.kt (1)
80-89
: Fix indentation inconsistency.The indentation after the
if (isReadOnly) {
statement is inconsistent with the rest of the codebase.Apply this diff to fix the indentation:
private val authorityFromIsReadOnly: GrantedAuthority get() { return SimpleGrantedAuthority( if (isReadOnly) { - ROLE_READ_ONLY - } else { - ROLE_READ_WRITE - } + ROLE_READ_ONLY + } else { + ROLE_READ_WRITE + } ) }backend/data/src/main/kotlin/io/tolgee/dtos/ComputedPermissionDto.kt (1)
136-137
: Logic is correct—Scope.readOnlyScopes
is a lazily initializedArray<Scope>
, so.toList()
yields aList<Scope>
andcontainsAll
works as intended. Consider defining this extension property at the file top-level for better discoverability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2UserController.kt
(2 hunks)backend/api/src/main/kotlin/io/tolgee/controllers/PublicController.kt
(3 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/ComputedPermissionDto.kt
(3 hunks)backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthentication.kt
(3 hunks)backend/data/src/main/kotlin/io/tolgee/service/security/PermissionService.kt
(2 hunks)backend/security/src/main/kotlin/io/tolgee/security/authentication/WriteOperation.kt
(1 hunks)backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt
(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/security/PermissionService.kt
🧬 Code graph analysis (1)
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)
🔇 Additional comments (13)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2UserController.kt (2)
114-122
: LGTM! Token refresh implementation aligns with past review decision.The refactoring to use
emitTokenRefreshForCurrentUser()
is correct. By not passingisSuper = true
, this aligns with the previous review decision to not emit a super token here for safety reasons, particularly considering 2FA edge cases mentioned in past comments.
270-287
: LGTM! Super token generation correctly implemented.The refactoring to use
emitTokenRefreshForCurrentUser(isSuper = true)
is correct. The method properly:
- Validates MFA or password before issuing the super token
- Passes
isSuper = true
to generate the elevated permission token- Preserves all existing security checks
backend/api/src/main/kotlin/io/tolgee/controllers/PublicController.kt (1)
72-72
: LGTM! Named parameter improves clarity.Using the named parameter
isSuper = true
is a good practice, especially as the JWT service signature has been expanded to support additional parameters (acting-as users, device IDs, read-only flags). The super token is correctly issued after MFA verification.backend/security/src/main/kotlin/io/tolgee/security/authentication/WriteOperation.kt (1)
1-27
: LGTM! Follows Spring Security best practices.The annotation definition is correct and follows the modern Spring Security approach of composing
@PreAuthorize
into custom annotations for maintainability. The use of role-based authorization (hasRole('READ_WRITE')
) provides a cleaner enforcement mechanism compared to HTTP method inspection mentioned in the PR objectives.Based on learnings.
backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt (1)
21-22
: LGTM!The new imports are used correctly in the refactored bypass logic.
backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthentication.kt (4)
30-48
: LGTM!The new constructor parameters are well-documented and appropriately expand the authentication model to support the supporter role, read-only mode, and impersonation.
61-77
: LGTM!The role hierarchy is correctly implemented with ADMIN including ROLE_SUPPORTER, and all roles now properly append the read-only/read-write authority.
113-116
: LGTM!The new role constants are consistent with the naming convention suggested in the past review comments.
43-43
: ReadOnlyModeInterceptor enforces read-only tokens for write operations
The existingReadOnlyModeInterceptor
checksauthenticationFacade.isReadOnly
and blocks non-read methods, so no additional enforcement is needed.backend/data/src/main/kotlin/io/tolgee/dtos/ComputedPermissionDto.kt (4)
66-66
: LGTM: Modern enum API usage.Using
Scope.entries
is the recommended approach in modern Kotlin, replacing the oldervalues().toList()
pattern.
68-89
: Well-structured supporter permission logic.The function correctly handles both ADMIN and SUPPORTER roles with clear documentation. The structural check at line 77 (
type == NONE && scopes.isEmpty()
) properly addresses the previous review concern about reference equality, ensuring supporters without base permissions receive the dedicatedSERVER_SUPPORTER
permission.The optimization path for empty permissions and the fallback to extended permissions are both logically sound.
128-134
: Clean permission extension implementation.The helper properly delegates to the base permission and lazily deduplicates the merged scopes. This approach avoids unnecessary object creation while maintaining a clean API.
159-167
: Consistent supporter permission definition.
SERVER_SUPPORTER
follows the established pattern ofSERVER_ADMIN
andORGANIZATION_OWNER
, using appropriate scope (ALL_VIEW
), type (VIEW
), and origin for supporter read-only semantics.
backend/security/src/main/kotlin/io/tolgee/security/authentication/WriteOperation.kt
Show resolved
Hide resolved
…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
…rmissions check for `translationsInTask`
19baa8a
to
3e4f45e
Compare
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/cypress/e2e/administration/base.cy.ts (1)
118-122
: Add e2e test for the ‘Supporter’ roleNo calls to
changeUserRole(..., 'Supporter')
exist in the e2e suite—add a test in e2e/cypress/e2e/administration/base.cy.ts (alongside the Admin/User cases around lines 63–71) that selects ‘Supporter’ and asserts “Role changed.”
♻️ Duplicate comments (2)
backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt (2)
83-85
: Actor ID claim type mismatch remains unresolved.The actor ID is stored as a String (line 84), but
validateActor
(line 286) only handles String types. If the JWT library serializes numeric strings asNumber
types (similar to Date serialization), the actor context will be silently dropped whenas? String
returns null.This is a duplicate of the past review comment at lines 285-291. Apply the previously suggested fix to handle both
String
andNumber
types invalidateActor
:private fun validateActor(claims: Claims): UserAccountDto? { - val actorId = claims[JWT_TOKEN_ACTING_USER_ID_CLAIM] as? String ?: return null - val account = userAccountService.findDto(actorId.toLong()) - ?: throw AuthenticationException(Message.USER_NOT_FOUND) - return account + val raw = claims[JWT_TOKEN_ACTING_USER_ID_CLAIM] ?: return null + val id = when (raw) { + is Number -> raw.toLong() + is String -> raw.toLong() + else -> throw AuthenticationException(Message.INVALID_JWT_TOKEN) + } + return userAccountService.findDto(id) ?: throw AuthenticationException(Message.USER_NOT_FOUND) }
285-290
: Actor ID type handling is incomplete.The method only handles
String
types (line 286) but should also handleNumber
types in case the JWT library serializes the numeric ID as aNumber
. See the critical issue flagged at lines 83-85 for the recommended fix.
🧹 Nitpick comments (9)
backend/testing/src/main/kotlin/io/tolgee/testing/AuthorizedControllerTest.kt (1)
69-69
: LGTM! Named parameter improves clarity.The change from a positional boolean to
isSuper = true
makes the token generation intent explicit and aligns with the broader API updates for the supporter role feature.Optional: Consider adding a helper for read-only token generation.
Since the PR introduces read-only/supporter access modes, tests that verify supporter behavior might benefit from a dedicated helper method alongside
generateJwtToken
:protected fun generateSupporterJwtToken(userAccountId: Long, actingAsUserAccountId: Long? = null) = jwtService.emitToken(userAccountId, isSuper = false, isReadOnly = true, actingAsUserAccountId = actingAsUserAccountId)This would make it easier to write tests for the new supporter role without affecting existing tests.
webapp/src/views/projects/ProjectPage.tsx (1)
25-27
: Consider renamingisAdminAccess
for clarity.The variable now includes both
SERVER_ADMIN
andSERVER_SUPPORTER
roles, but the name implies admin-only access. Since supporters have read-only permissions while admins have full access, a more descriptive name likeisServerRoleAccess
orisAdminOrSupporterAccess
would better reflect the actual semantics.Apply this diff to improve clarity:
- const isAdminAccess = + const isAdminOrSupporterAccess = project.computedPermission.origin === 'SERVER_ADMIN' || project.computedPermission.origin === 'SERVER_SUPPORTER'; return ( <DashboardPage - isAdminAccess={isAdminAccess} + isAdminAccess={isAdminOrSupporterAccess} fixedContent={<ProjectMenu />} rightPanelContent={rightPanelContent} >Note: If
DashboardPage
'sisAdminAccess
prop also needs renaming to better represent its purpose, consider updating that component's prop name as well for consistency across the codebase.backend/data/src/main/kotlin/io/tolgee/repository/ProjectRepository.kt (1)
58-58
: Update the comment to include SUPPORTER role.The comment currently states "it makes sense to give all projects to admin only when organizationId is provided," but line 66 now grants the same access to SUPPORTER role. Please update the comment to reflect this change.
Apply this diff:
- // it makes sense to give all projects to admin only when organizationId is provided + // it makes sense to give all projects to admin or supporter only when organizationId is providedbackend/data/src/main/kotlin/io/tolgee/model/enums/Scope.kt (2)
7-7
: Redundant import
import kotlin.arrayOf
is unnecessary;arrayOf
is available via default Kotlin imports. Remove to avoid lint warnings.-import kotlin.arrayOf
55-56
: Optimize read-only check: use Set for O(1) contains
readOnlyScopes
is an Array;contains
is O(n). Convert to Set once.- val readOnlyScopes by lazy { ALL_VIEW.expand() } + val readOnlyScopes: Set<Scope> by lazy { ALL_VIEW.expand().toSet() } @@ - fun isReadOnly(scope: Scope): Boolean { - return readOnlyScopes.contains(scope) - } + fun isReadOnly(scope: Scope): Boolean = scope in readOnlyScopesAlso applies to: 254-260
backend/security/src/main/kotlin/io/tolgee/security/authentication/ReadOnlyOperation.kt (1)
19-30
: Annotation looks good; minimal, clear contractTarget/retention are correct and docs explain the override. Consider adding @MustBeDocumented for API clarity (optional).
webapp/src/globalContext/helpers.tsx (1)
24-33
: New supporter helpers: LGTMBoth hooks are correct and safe with absent userInfo. Optional: centralize role literals in a shared enum/const to avoid string drift across the app.
backend/data/src/main/kotlin/io/tolgee/service/security/PermissionService.kt (1)
238-239
: Clarify admin/supporter helper behavior
getAdminOrSupporterPermissions
already tolerates nulluserRole
(falls back to base permissions) and has no default.- Add tests covering SUPPORTER vs ADMIN for all organization roles (OWNER/MEMBER/DIRECT/NONE) to verify SUPPORTER grants strictly read-only scopes.
backend/security/src/main/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptor.kt (1)
74-89
: Consider consolidating bypass checks for clarity.The code checks
canBypassForReadOnly()
twice: once insidecanBypass()
at line 153 and again at line 82. While this achieves the desired security behavior (throwingPermissionException
instead ofNotFoundException
for read-only supporters without view permission), the double check may confuse future maintainers.Consider refactoring to make the intent clearer:
- if (!organizationRoleService.canUserViewStrict(userId, organization.id)) { - if (!canBypass(request, handler)) { - logger.debug( - "Rejecting access to org#{} for user#{} - No view permissions", - organization.id, - userId, - ) - - if (!canBypassForReadOnly()) { - // Security consideration: if the user cannot see the organization, pretend it does not exist. - throw NotFoundException() - } - - // Admin access for read-only operations is allowed, but it's not enough for the current operation. - throw PermissionException() - } + if (!organizationRoleService.canUserViewStrict(userId, organization.id)) { + val canFullBypass = canBypass(request, handler) + val canReadOnlyBypass = canBypassForReadOnly() + + if (!canFullBypass && !canReadOnlyBypass) { + logger.debug( + "Rejecting access to org#{} for user#{} - No view permissions", + organization.id, + userId, + ) + // Security consideration: if the user cannot see the organization, pretend it does not exist. + throw NotFoundException() + } + + if (!canFullBypass) { + // Admin access for read-only operations is allowed, but it's not enough for the current operation. + throw PermissionException() + }This makes it clear that:
- Users without any bypass throw
NotFoundException
(don't leak org existence)- Read-only supporters without full bypass throw
PermissionException
(they know org exists but can't write)- Admins and read-only supporters on read-only operations bypass completely
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (95)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/ApiKeyController.kt
(3 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/administration/AdministrationController.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/controllers/AuthProviderChangeController.kt
(1 hunks)backend/api/src/main/kotlin/io/tolgee/controllers/PublicController.kt
(3 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/V2UserControllerTest.kt
(2 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/administration/AdministrationControllerTest.kt
(3 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
(4 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
(2 hunks)backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt
(8 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
(6 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
(4 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)
🚧 Files skipped from review as they are similar to previous changes (41)
- webapp/src/component/PermissionsSettings/useScopeTranslations.tsx
- backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/InitialDataController.kt
- backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserMfaController.kt
- backend/app/src/test/kotlin/io/tolgee/security/ProjectApiKeyAuthenticationTest.kt
- backend/data/src/main/kotlin/io/tolgee/model/enums/ProjectPermissionType.kt
- backend/app/src/main/kotlin/io/tolgee/configuration/WebSecurityConfig.kt
- backend/data/src/main/kotlin/io/tolgee/model/UserAccount.kt
- backend/security/src/main/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptor.kt
- backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/SlugController.kt
- backend/security/src/main/kotlin/io/tolgee/security/authorization/AbstractAuthorizationInterceptor.kt
- backend/api/src/main/kotlin/io/tolgee/hateoas/project/ProjectModelAssembler.kt
- backend/data/src/main/kotlin/io/tolgee/dtos/cacheable/UserAccountDto.kt
- backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationService.kt
- backend/data/src/main/kotlin/io/tolgee/service/security/SecurityService.kt
- backend/api/src/main/kotlin/io/tolgee/controllers/AuthProviderChangeController.kt
- backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2UserController.kt
- backend/security/src/main/kotlin/io/tolgee/security/authentication/AdminAccessInterceptor.kt
- ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/glossary/GlossaryController.kt
- webapp/src/component/common/useFeatureMissingExplanation.tsx
- webapp/src/translationTools/useErrorTranslation.ts
- backend/api/src/main/kotlin/io/tolgee/hateoas/auth/AuthInfoModelAssembler.kt
- ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/SuggestionController.kt
- backend/api/src/main/kotlin/io/tolgee/controllers/PublicController.kt
- webapp/src/views/projects/ProjectListView.tsx
- backend/api/src/main/kotlin/io/tolgee/hateoas/auth/AuthInfoModel.kt
- backend/security/src/test/kotlin/io/tolgee/security/authentication/AdminAccessInterceptorTest.kt
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/V2UserControllerTest.kt
- backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthenticationDetails.kt
- backend/data/src/main/kotlin/io/tolgee/constants/Message.kt
- backend/app/src/test/kotlin/io/tolgee/security/ServerAdminFilterTest.kt
- webapp/src/ee/billing/component/UserMenu/BillingMenuItem.tsx
- backend/security/src/test/kotlin/io/tolgee/security/authentication/AuthenticationFilterTest.kt
- backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/V2InvitationController.kt
- backend/data/src/main/kotlin/io/tolgee/service/security/MfaService.kt
- backend/app/src/test/kotlin/io/tolgee/service/dataImport/StoredDataImporterTest.kt
- backend/app/src/test/kotlin/io/tolgee/service/dataImport/ImportServiceTest.kt
- webapp/src/views/userSettings/apiKeys/EditApiKeyDialog.tsx
- backend/data/src/main/kotlin/io/tolgee/service/security/SignUpService.kt
- ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/PromptController.kt
- webapp/src/views/organizations/OrganizationsRouter.tsx
- backend/security/src/test/kotlin/io/tolgee/security/authentication/ReadOnlyModeInterceptorTest.kt
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-08-12T11:01:00.701Z
Learnt from: stepan662
PR: tolgee/tolgee-platform#3134
File: webapp/src/component/activity/configuration.tsx:500-502
Timestamp: 2025-08-12T11:01:00.701Z
Learning: In the Tolgee platform project, do not flag missing translation keys or suggest adding keys to JSON files during code reviews. All translation keys are managed through the Tolgee platform interface, not in code. Translation key availability is validated at build time, so missing key warnings in code reviews are irrelevant and should not be raised. The JSON translation files are generated artifacts.
Applied to files:
webapp/src/component/layout/TopBar/announcements/DebuggingCustomerAccountAnnouncement.tsx
📚 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/api/src/main/kotlin/io/tolgee/api/v2/controllers/organization/OrganizationController.kt
backend/data/src/main/kotlin/io/tolgee/service/security/PermissionService.kt
backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt
📚 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
📚 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/test/kotlin/io/tolgee/security/authentication/JwtServiceTest.kt
backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.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
backend/data/src/main/kotlin/io/tolgee/constants/ComputedPermissionOrigin.kt
🧬 Code graph analysis (12)
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)
webapp/src/component/layout/TopBar/announcements/DebuggingCustomerAccountAnnouncement.tsx (1)
webapp/src/component/layout/TopBar/announcements/TopBarAnnouncementWithIcon.tsx (1)
TopBarAnnouncementWithAlertIcon
(29-35)
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/views/organizations/components/BaseOrganizationSettingsView.tsx (1)
webapp/src/globalContext/helpers.tsx (1)
useIsAdminOrSupporter
(29-33)
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/security/UserMenu/UserPresentAvatarMenu.tsx (1)
webapp/src/globalContext/helpers.tsx (1)
useIsAdminOrSupporter
(29-33)
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
(173-180)performWriteRequests
(182-190)
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/app/src/test/kotlin/io/tolgee/api/v2/controllers/administration/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/app/src/test/kotlin/io/tolgee/AuthTest.kt (1)
backend/testing/src/main/kotlin/io/tolgee/testing/AuthorizedControllerTest.kt (1)
setForcedDate
(158-161)
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
(284-293)performWriteRequests
(295-309)
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). (1)
- GitHub Check: Build backend 🏗️
🔇 Additional comments (78)
backend/data/src/main/kotlin/io/tolgee/development/testDataBuilder/data/AdministrationTestData.kt (1)
9-9
: LGTM! Test data addition follows existing patterns.The supporter user account is properly integrated into the test data builder, mirroring the structure of existing admin and user accounts. The field declaration and addUserAccount block are consistent with established conventions.
Also applies to: 23-28
backend/app/src/test/kotlin/io/tolgee/service/LanguageServiceTest.kt (1)
153-163
: LGTM!The explicit named parameters improve code clarity, and the default values are appropriate for these write-operation tests (
isReadOnly = false
is correct for language deletion).webapp/src/component/layout/TopBar/announcements/DebuggingCustomerAccountAnnouncement.tsx (4)
5-8
: LGTM!The import addition is necessary for reading the read-only mode state from global context. The multi-line import formatting is clean and follows conventions.
21-23
: LGTM!The read-only mode check is well-implemented:
- Optional chaining safely handles potentially undefined
authInfo
- The explicit
=== true
comparison ensuresisReadOnlyMode
is always a boolean type rather thanboolean | undefined
, improving type safety- Correctly defaults to
false
(standard mode) whenauthInfo
orisReadOnly
is undefined
25-29
: LGTM!The conditional message logic is clean and correctly differentiates between read-only and standard debugging modes. The ternary operator is appropriately used for this simple conditional rendering.
33-33
: LGTM!The rendering is clean and straightforward, using the derived message variable within a Box wrapper.
backend/security/src/main/kotlin/io/tolgee/security/authorization/FeatureAuthorizationInterceptor.kt (3)
24-24
: LGTM: Explicit configuration of global route handling.Passing
allowGlobalRoutes = false
ensures that feature authorization checks do not apply to global routes, which is the expected behavior for feature-gated endpoints.
62-70
: LGTM: Improved error reporting for missing features.The refactored logic now collects all missing features before throwing, which provides better error messages to clients and consolidates logging into a single debug statement. The early return when no features are missing is a clean short-circuit.
77-85
: LGTM: Cleaner one-of-features check.Using
any(...)
is more idiomatic and readable than a loop-based search. The error reporting and logging improvements mirror thecheckFeaturesEnabled
refactor.backend/data/src/main/kotlin/io/tolgee/repository/ProjectRepository.kt (1)
66-66
: Verify SUPPORTER role grants organization-wide project visibility (ProjectRepository.kt:66).
The predicate(ua.role = 'ADMIN' OR ua.role = 'SUPPORTER') AND :organizationId IS NOT NULL
gives SUPPORTER the same org-level access as ADMIN. Confirm this matches intended SUPPORTER permissions.backend/api/src/main/kotlin/io/tolgee/hateoas/initialData/InitialDataModel.kt (1)
5-5
: LGTM! AuthInfoModel integration is clean and well-structured.The addition of the
authInfo
field toInitialDataModel
is correctly implemented:
- Import is properly added for the new
AuthInfoModel
type- Field placement is logical (after server config, before user-specific data)
- Nullable type (
AuthInfoModel?
) appropriately handles unauthenticated requests- Default value (
null
) ensures backward compatibility- Mutability (
var
) is consistent with existing fields (userInfo
,ssoInfo
, etc.)Also applies to: 12-12
backend/data/src/main/kotlin/io/tolgee/model/enums/OrganizationRoleType.kt (1)
3-8
: Confirm MEMBER read-only semantics and enforcementThe
MEMBER
role now hasisReadOnly = true
, changing its behavior. Verify:
- MEMBERs should be blocked from write operations—ensure code checks
.isReadOnly
.- Distinction between the new SUPPORTER server role and org MEMBER.
- Migration path for existing MEMBERs losing write access.
Run:
rg -n 'isReadOnly' -C2
backend/api/src/main/kotlin/io/tolgee/hateoas/project/ProjectWithStatsModelAssembler.kt (1)
43-44
: computeProjectPermission safely handles a nullable userRole and admin/supporter logic behaves as intended
Null is treated like USER (no NPE risk), and ADMIN/SUPPORTER branches in getAdminOrSupporterPermissions apply as designed.backend/data/src/main/kotlin/io/tolgee/service/StartupImportService.kt (1)
82-90
: Auth construction update: LGTMMatches new TolgeeAuthentication fields; named args aid readability.
backend/data/src/main/kotlin/io/tolgee/model/enums/Scope.kt (1)
111-117
: Confirm coupling: LANGUAGES_EDIT requires PROMPTS_VIEWThis makes LANGUAGES_EDIT implicitly grant PROMPTS_VIEW. Is this intentional? If not, drop the dependency.
- HierarchyItem(LANGUAGES_EDIT, listOf(HierarchyItem(PROMPTS_VIEW))), + // If unintentional, remove PROMPTS_VIEW requirement: + // HierarchyItem(LANGUAGES_EDIT),backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/project/ProjectsTransferringController.kt (1)
75-76
: Transfer permission check update: LGTMDelegation to OrganizationRoleService.checkUserCanTransferProjectToOrganization() is appropriate.
ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/TaskController.kt (1)
315-318
: Mark calculate-scope as read-only: LGTM@ReadOnlyOperation suits this read-only computation endpoint.
backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt (1)
53-71
: Role checks refactored: usage verified
- New authorization helpers are invoked in controllers for invitation deletion, project transfer, and project creation.
webapp/src/component/PermissionsSettings/usePermissionsStructure.ts (1)
35-37
: ALL_VIEW integration verified
'all.view' is included in the Scope union (apiSchema + fixtures) and the translation key 'permissions_item_all_view' exists in all locales.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/ApiKeyController.kt (1)
80-82
: Approve isAdmin() usage and enum entries
Kotlin version (2.1.20) supports enum.entries; ready to merge.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 appropriately marks this endpoint as read-only, aligning with the PR's goal of establishing read-only access patterns for the supporter role.Also applies to: 297-297
ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/SsoProviderController.kt (1)
7-7
: LGTM: Admin check refactored for consistency.Refactoring to use the
isAdmin()
extension function improves maintainability and ensures consistent admin checks across the codebase. Based on learnings, this correctly excludes the SUPPORTER role from SSO tenant domain changes.Also applies to: 49-49
ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/AiPromptCustomizationController.kt (1)
84-84
: LGTM: Documentation corrected for GET endpoint.The OpenAPI summary now accurately reflects that this is a read operation returning language-level prompt customizations, rather than incorrectly stating it "Sets" them.
backend/data/src/main/kotlin/io/tolgee/constants/ComputedPermissionOrigin.kt (1)
9-9
: LGTM: Enum extended to support new role.The
SERVER_SUPPORTER
enum value enables tracking permissions granted through the new supporter role.webapp/src/component/layout/TopBar/announcements/AdministrationAccessAnnouncement.tsx (1)
4-4
: LGTM: Role-specific messaging implemented.The component now displays appropriate messages based on whether the user is a supporter or admin, improving the user experience with role-specific context.
Also applies to: 7-17
ee/backend/app/src/main/kotlin/io/tolgee/ee/api/v2/controllers/glossary/GlossaryTermHighlightsController.kt (1)
15-15
: LGTM: Read-only annotation correctly applied.The
@ReadOnlyOperation
annotation appropriately marks this endpoint as read-only, consistent with the PR's read-only access pattern for supporter users.Also applies to: 37-37
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/suggestion/TranslationSuggestionController.kt (1)
17-17
: LGTM: Read-only annotation correctly applied.The
@ReadOnlyOperation
annotation appropriately marks this endpoint as read-only, as it retrieves translation memory suggestions without making any modifications.Also applies to: 100-100
webapp/src/component/security/UserMenu/UserPresentAvatarMenu.tsx (1)
7-7
: LGTM: Menu visibility extended to supporters.The server administration menu item is now visible to both admins and supporters, appropriately reflecting that supporters should have access to administration features (in read-only mode).
Also applies to: 55-55, 164-164
backend/app/src/test/kotlin/io/tolgee/AuthTest.kt (1)
260-260
: LGTM! Named parameter improves readability.The transition from positional to named parameter (
isSuper
) clarifies intent and reduces the risk of argument order mistakes. The test logic remains correct.Also applies to: 267-267
backend/app/src/main/kotlin/io/tolgee/configuration/openApi/OpenApiSecurityHelper.kt (1)
20-20
: LGTM! Lambda arity adjusted for API compatibility.The additional unused parameter aligns with the updated
customizeOperations
API signature. The underscore correctly signals that the parameter is intentionally ignored.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/batch/V2ExportController.kt (1)
74-74
: LGTM! Correctly marks non-mutating POST as read-only.The
@ReadOnlyOperation
annotation appropriately designates this POST endpoint as read-only. While POST typically implies mutation, this endpoint uses POST solely to accommodate large payloads via request body, making the read-only classification semantically correct.backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/administration/AdministrationControllerTest.kt (3)
92-101
: LGTM! Test validates write access for admin-impersonated token.The test correctly verifies that an admin-generated impersonation token permits write operations (organization creation).
103-126
: LGTM! Test validates read-only enforcement for supporter-impersonated tokens.The test correctly verifies that:
- Supporter-generated impersonation tokens can read organizations (line 111-116)
- Write operations are forbidden for supporter-impersonated tokens (line 119-125)
This aligns with the PR's read-only access objective for supporters.
137-150
: LGTM! Test validates supporter role restrictions.The tests correctly verify that supporters:
- Can access read-only admin endpoints (lines 137-142)
- Cannot access write admin endpoints (lines 145-150)
This provides good coverage of the supporter role's read-only behavior.
backend/security/src/main/kotlin/io/tolgee/security/authentication/AuthenticationFilter.kt (1)
117-124
: LGTM! Named parameters improve clarity.The TolgeeAuthentication constructor calls now use named parameters across all three authentication paths (disabled auth, PAK, PAT). This improves readability and reduces the risk of argument order errors while maintaining the same runtime behavior.
Also applies to: 168-175, 196-203
webapp/src/views/administration/components/RoleSelector.tsx (1)
53-53
: Approve: SUPPORTER role addition, type safety, and translation
- onChange casting to
Role
ensures type safety- SUPPORTER MenuItem placed correctly between USER and ADMIN
- Translation key
administration_user_role_supporter
present in all locale fileswebapp/src/views/organizations/components/BaseOrganizationSettingsView.tsx (1)
52-53
: Restrict supporter write access on organization endpoints
The updatedcanManageOrganization
now includes SUPPORTER and exposes write actions. Confirm server‐side guards for all write mappings under/v2/organizations/{id}
:
- V2InvitationController PUT
/v2/organizations/{id}/invite
must enforce OWNER or ADMIN only- Any membership removal, privileges, apps, SSO or billing POST/PUT/DELETE endpoints must reject SUPPORTER
Ensure the frontend also disables or hides write controls for SUPPORTER.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/administration/AdministrationController.kt (1)
158-164
: Confirm impersonation guards and implement disabled-user check
- Both UserAccount.isAdmin() and UserAccountDto.isAdmin() return
role == ADMIN
, so no ambiguity.- Current guards block re-impersonation and supporter→admin; supporter→supporter and admin→any are allowed.
- Verify and enforce disabled-user policy: reject impersonation if the target account is disabled.
backend/data/src/main/kotlin/io/tolgee/service/security/PermissionService.kt (1)
474-480
: removeDirectProjectPermissions semantics: OK; verify controller guardsIdempotent delete of direct perms looks good and correctly routes through delete(permission.id) to refresh caches/preferences. Note this bypasses the revoke() org‑member check by design (only removes direct perms). Please verify the calling endpoint enforces proper authorization (e.g., project maintainer/owner).
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/organization/OrganizationController.kt (1)
13-13
: Use of isAdmin() extension: good modernizationSwapping direct role comparisons for isAdmin() improves consistency with the new role model. Import path is correct and usage matches both branches.
Also applies to: 109-110, 114-115
backend/security/src/main/kotlin/io/tolgee/security/authentication/WriteOperation.kt (1)
21-30
: PreAuthorize-based write guard: confirm wiring
- Verify ‘ROLE_READ_WRITE’ is granted only to admins/non-read-only users.
- Confirm global method security is enabled (e.g.,
@EnableMethodSecurity
or@EnableGlobalMethodSecurity
on a config class).
If you already enforce this via a ReadOnlyInterceptor, this remains a useful belt-and-suspenders check.backend/security/src/main/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptor.kt (1)
144-158
: LGTM! Clean bypass logic implementation.The centralized bypass logic cleanly handles the different authorization contexts:
- Admins can always bypass
- Supporters can bypass only for read-only operations
This aligns well with the broader read-only mode framework introduced across the PR.
backend/security/src/main/kotlin/io/tolgee/security/authentication/ReadOnlyModeExtension.kt (1)
1-32
: Well-designed read-only detection logic.The extension function provides a clean, consistent way to determine if an endpoint is read-only. The implementation correctly:
- Validates annotation consistency (lines 16-21)
- Respects explicit annotations over HTTP method conventions
- Falls back to standard HTTP method semantics (GET, HEAD, OPTIONS)
The early validation for conflicting annotations prevents configuration errors at startup, which is a good defensive programming practice.
backend/security/src/main/kotlin/io/tolgee/security/authentication/ReadOnlyModeInterceptor.kt (1)
38-63
: LGTM! Correct read-only enforcement with proper guards.The interceptor correctly implements read-only mode enforcement:
- Skips unauthenticated requests (line 43) - let other interceptors handle
- Allows non-read-only authenticated users (line 48)
- Permits read-only operations (line 53)
- Blocks write operations with clear error message (lines 58-62)
The early return pattern ensures
authenticatedUser
is non-null when accessed at line 60, addressing the concern raised in the previous review.backend/app/src/main/kotlin/io/tolgee/configuration/openApi/OpenApiConfiguration.kt (1)
113-113
: LGTM! Mechanical signature update for consistency.The addition of the unused 4th parameter (
_
) to thecustomizeOperations
lambdas aligns with the updated signature inOpenApiGroupBuilder.kt
(line 180). This enables the builder to pass the HTTP method to consumers that need it (likeaddAccessModeExtension
), while existing consumers can ignore it.This is a clean backward-compatible extension of the API.
Also applies to: 171-171, 193-193
backend/data/src/main/kotlin/io/tolgee/dtos/ComputedPermissionDto.kt (4)
66-66
: LGTM! Modern Kotlin enum API usage.The change from
Scope.values().toList()
toScope.entries
uses the modern Kotlin enum API introduced in Kotlin 1.9, which returns an immutableList
instead of creating a new array each time.
68-89
: Well-implemented supporter permissions with proper optimization.The renamed method and added documentation address the previous review feedback. The supporter logic is correctly implemented:
- Lines 76-82: Optimization for users with no permissions - returns static
SERVER_SUPPORTER
instance- Lines 83-87: For users with existing permissions, extends them with
ALL_VIEW
scope while preserving their base permissionsThe fix at line 77 correctly detects the "no base permission" case by checking
type == ProjectPermissionType.NONE && scopes.isEmpty()
instead of identity comparison, addressing the critical issue raised in the previous review.
128-137
: Clean helper for permission composition.The
getExtendedPermission
helper (lines 128-134) provides a reusable way to overlay additional scopes onto base permissions using delegation, which is more maintainable than duplicating this logic.The
isAllReadOnlyPermitted
extension property (lines 136-137) provides a clear, reusable check for read-only permission completeness.
159-167
: LGTM! Consistent permission constant design.The
SERVER_SUPPORTER
constant follows the same pattern asSERVER_ADMIN
andORGANIZATION_OWNER
, using:
ALL_VIEW
scope for read-only access across all featuresVIEW
permission typeSERVER_SUPPORTER
origin for permission trackingThis consistency makes the codebase easier to understand and maintain.
backend/data/src/test/kotlin/io/tolgee/security/authentication/JwtServiceTest.kt (3)
45-97
: Excellent test setup for actor impersonation scenarios.The test setup correctly adds:
- Actor ID constants (lines 45-46) for admin and supporter scenarios
- Mock actors with appropriate roles (lines 60-97)
- AuthenticationFacade mock for token refresh testing
The mock configuration properly sets up the role hierarchy:
userAccount
: regular user (role set to ADMIN at line 89, later reset)adminActor
: admin role (line 93)supporterActor
: supporter role (line 97)This provides comprehensive test coverage for the impersonation feature.
127-134
: LGTM! Named parameters improve test readability.The change to named parameter syntax (e.g.,
isSuper = true
) makes the test intent much clearer compared to positional arguments. The move fromdetails?.isSuperToken
toauth.isSuperToken
(lines 132-134, 144) reflects the architectural change where super-token state is now a first-class property of the authentication object.Also applies to: 140-145
237-296
: Comprehensive test coverage for security-critical features.The new tests thoroughly cover important security scenarios:
- Lines 237-244: Read-only flag propagation without actor impersonation
- Lines 246-254: Actor information carried in tokens for admin impersonation
- Lines 256-263: Security boundary - rejecting read-only tokens for admin subjects
- Lines 265-270: Security boundary - preventing non-read-only supporter impersonation
- Lines 272-280: Allowing supporter impersonation only with read-only flag
- Lines 282-296: Token refresh preserving read-only, super, and actor state
These tests ensure the impersonation and read-only features work correctly and maintain security boundaries.
Based on learnings: The test at lines 256-263 correctly validates the intended behavior that read-only tokens cannot be used to impersonate admin accounts, as confirmed in the learning note.
backend/app/src/main/kotlin/io/tolgee/configuration/openApi/OpenApiGroupBuilder.kt (2)
115-123
: LGTM! Valuable access mode metadata for API consumers.The new
addAccessModeExtension
function enriches OpenAPI documentation withx-access-mode
metadata (readOnly
orreadWrite
), which:
- Helps API consumers understand endpoint capabilities
- Enables client-side enforcement of read-only constraints
- Provides documentation clarity about operation safety
The implementation correctly uses the
isReadOnly
helper to determine the mode based on annotations or HTTP method.
180-214
: Well-designed API extension with backward compatibility.The signature change to
customizeOperations
(line 180) adds the HTTP method parameter while maintaining backward compatibility:
- Consumers that don't need the method can use
_
to ignore it (lines 127, 218)- Consumers that need it (like
addAccessModeExtension
) can use it (line 77)The change from
readOperations()
toreadOperationsMap()
(line 187) provides access to the HTTP method, which is necessary for the new access mode detection.backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/project/ProjectsController.kt (2)
244-252
: Update API docs and client references for the renamed endpoint
- Update the OpenAPI/Swagger spec to use
removeDirectProjectPermissions
as theoperationId
and in all documentation.- Search for any exact occurrences of the old name and rename them:
rg -nP '\bsetOrganizationBase\b' -g '*.kt' -g '*.yml' -g '*.json'- Regenerate affected client SDKs and integration tests to pick up the new endpoint name.
88-88
: Permission check logic verified
checkUserCanCreateProject
delegates tocheckUserIsOwnerOrMaintainerOrServerAdmin
, maintaining owner/maintainer restrictions, additionally permitting server admins—supporters remain blocked.backend/data/src/main/kotlin/io/tolgee/security/authentication/AuthenticationFacade.kt (3)
82-84
: LGTM!The
actingUser
property correctly exposes the acting user account for impersonation scenarios by delegating toauthentication.actingAsUserAccount
.
87-91
: LGTM!The
deviceId
andisReadOnly
properties correctly delegate to the underlying authentication object, following the established pattern in this facade.
92-93
: LGTM!The update to
isUserSuperAuthenticated
correctly accessesauthentication.isSuperToken
directly, aligning with the refactoredTolgeeAuthentication
structure whereisSuperToken
is now a top-level property instead of being nested indetails
.backend/security/src/test/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptorTest.kt (3)
51-52
: LGTM!The mock setup correctly introduces the
authentication
mock and configuresisReadOnly
behavior, which is essential for testing read-only access scenarios.Also applies to: 76-76, 80-82
148-171
: LGTM!The new test methods comprehensively verify supporter role behavior:
- Supporter can access read-only endpoints bypassing organization permissions
- Supporter cannot bypass checks for write endpoints
- Admin retains full access to all endpoints
The test scenarios align well with the intended access control model.
173-195
: LGTM!The helper methods
performReadOnlyRequests
,performWriteRequests
, andandSatisfies
effectively reduce test duplication and follow the same pattern established inProjectAuthorizationInterceptorTest
.backend/data/src/main/kotlin/io/tolgee/security/authentication/TolgeeAuthentication.kt (3)
30-48
: LGTM!The expanded constructor parameters are well-documented with clear KDoc comments explaining the purpose of each property:
deviceId
for activity loggingactingAsUserAccount
for impersonation trackingisReadOnly
for restricting write operationsisSuperToken
for super authentication
59-89
: LGTM!The updated
getAuthorities()
method correctly implements the role hierarchy:
USER
→ROLE_USER
SUPPORTER
→ROLE_USER
+ROLE_SUPPORTER
ADMIN
→ROLE_USER
+ROLE_SUPPORTER
+ROLE_ADMIN
Each branch appropriately appends the read-only/read-write authority via
authorityFromIsReadOnly
, which returnsROLE_READ_ONLY
whenisReadOnly
is true, otherwiseROLE_READ_WRITE
.
112-117
: LGTM!The new role constants
ROLE_SUPPORTER
,ROLE_READ_ONLY
, andROLE_READ_WRITE
are properly defined and follow the existing naming convention.webapp/src/service/apiSchema.generated.ts (1)
1223-1225
: Ensure client-side gating for read-only flag
Only DebuggingCustomerAccountAnnouncement.tsx (line 22) checksauthInfo?.isReadOnly
; audit all write CTAs (forms, buttons, settings) to disable or hide them whenisReadOnly
is true.backend/security/src/test/kotlin/io/tolgee/security/authorization/ProjectAuthorizationInterceptorTest.kt (5)
35-35
: LGTM!The new imports are necessary for the test endpoints and helper methods that exercise read-only and write operation annotations.
Also applies to: 46-47, 51-51
57-57
: LGTM!The new authentication mock is properly integrated with setup (lines 91, 97-99) and teardown (line 119).
259-282
: LGTM!The three new tests properly validate supporter and admin access patterns:
- Supporters can read but not write (even with empty scopes)
- Admins have full access regardless of scopes
The tests correctly exercise the new read-only operation annotations.
284-314
: LGTM!The helper methods follow the established pattern from
OrganizationAuthorizationInterceptorTest
and correctly distinguish between read-only operations (GET methods or POST with@ReadOnlyOperation
) and write operations (POST methods or GET with@WriteOperation
).
333-370
: LGTM!The new test endpoints properly exercise various combinations of HTTP methods with
@ReadOnlyOperation
and@WriteOperation
annotations, enabling comprehensive testing of the new access control logic.backend/data/src/main/kotlin/io/tolgee/security/authentication/JwtService.kt (7)
49-49
: LGTM!The
authenticationFacade
dependency is necessary for the new token refresh and impersonation methods. This is a constructor signature change that affects all instantiation sites.
68-69
: LGTM!The new parameters expand the token to support impersonation and read-only mode while maintaining backward compatibility through default values.
87-88
: Verify deviceId generation strategy.Every token emission generates a new random deviceId, including token refreshes. This means each refresh creates a new device session identifier. Confirm this is the intended behavior and won't break device-based session tracking or analytics.
110-119
: LGTM!The method properly propagates impersonation and read-only state. The documentation (lines 102-109) clearly explains the security rationale for defaulting
isSuper
tofalse
, addressing the past review concern about super-token propagation.
121-149
: LGTM!The impersonation methods correctly implement the access model:
- Admin impersonation: full read-write access with super powers
- Supporter impersonation: read-only access without super powers
- Dispatcher logic correctly returns early for admins (line 142) before checking supporter-or-admin
213-226
: LGTM!The validation logic correctly enforces the access model:
- Read-only tokens cannot impersonate admin accounts (lines 215-218) - prevents privilege escalation
- Actor role is re-validated at token use time (lines 220-226) - handles demotion scenarios
The comments clearly explain the security rationale. Based on learnings, the check at line 215 correctly validates the target account (not the actor) to block read-only access to admin accounts.
314-316
: LGTM!The new JWT claim constants follow standard naming conventions with appropriate namespace prefixes.
val expiredToken = jwtService.emitToken(data.franta.id, isSuper = true) | ||
currentDateProvider.forcedDate = null | ||
|
||
return mapOf( | ||
"frantasProjectId" to data.frantasProject.id, | ||
"pepasProjectId" to data.pepasProject.id, | ||
"frantaExpiredSuperJwt" to expiredToken, | ||
"pepaExpiredSuperJwt" to jwtService.emitToken(data.pepa.id, false), | ||
"pepaExpiredSuperJwt" to jwtService.emitToken(data.pepa.id, isSuper = false), | ||
) |
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.
Test-data mismatch: ‘pepaExpiredSuperJwt’ is neither super nor expired
You reset forcedDate to null and emitToken with isSuper = false. The name suggests an expired super JWT.
Fix by generating an expired super token for Pepa too:
val baseline = currentDateProvider.date
currentDateProvider.forcedDate = Date(baseline.time - authenticationProperties.jwtSuperExpiration - 10_000)
- val expiredToken = jwtService.emitToken(data.franta.id, isSuper = true)
+ val expiredToken = jwtService.emitToken(data.franta.id, isSuper = true)
currentDateProvider.forcedDate = null
return mapOf(
"frantasProjectId" to data.frantasProject.id,
"pepasProjectId" to data.pepasProject.id,
"frantaExpiredSuperJwt" to expiredToken,
- "pepaExpiredSuperJwt" to jwtService.emitToken(data.pepa.id, isSuper = false),
+ // Generate Pepa's expired super JWT as well
+ // (re-use baseline to ensure expiration)
+ "pepaExpiredSuperJwt" to run {
+ currentDateProvider.forcedDate =
+ Date(baseline.time - authenticationProperties.jwtSuperExpiration - 10_000)
+ val token = jwtService.emitToken(data.pepa.id, isSuper = true)
+ currentDateProvider.forcedDate = null
+ token
+ },
)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
val expiredToken = jwtService.emitToken(data.franta.id, isSuper = true) | |
currentDateProvider.forcedDate = null | |
return mapOf( | |
"frantasProjectId" to data.frantasProject.id, | |
"pepasProjectId" to data.pepasProject.id, | |
"frantaExpiredSuperJwt" to expiredToken, | |
"pepaExpiredSuperJwt" to jwtService.emitToken(data.pepa.id, false), | |
"pepaExpiredSuperJwt" to jwtService.emitToken(data.pepa.id, isSuper = false), | |
) | |
val expiredToken = jwtService.emitToken(data.franta.id, isSuper = true) | |
currentDateProvider.forcedDate = null | |
return mapOf( | |
"frantasProjectId" to data.frantasProject.id, | |
"pepasProjectId" to data.pepasProject.id, | |
"frantaExpiredSuperJwt" to expiredToken, | |
// Generate Pepa's expired super JWT as well | |
// (re-use baseline to ensure expiration) | |
"pepaExpiredSuperJwt" to run { | |
currentDateProvider.forcedDate = | |
Date(baseline.time - authenticationProperties.jwtSuperExpiration - 10_000) | |
val token = jwtService.emitToken(data.pepa.id, isSuper = true) | |
currentDateProvider.forcedDate = null | |
token | |
}, | |
) |
@GetMapping("/v2/organizations/{id}/requires-admin") | ||
@GetMapping("/v2/organizations/{id}/requires-owner") | ||
@RequiresOrganizationRole(OrganizationRoleType.OWNER) | ||
fun requiresAdmin( |
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.
🛠️ Refactor suggestion | 🟠 Major
Rename test controller methods for consistency.
The method names still reference "Admin" (e.g., requiresAdmin
, requiresAdminWriteMethod
) but should be renamed to "Owner" to match the endpoint paths (requires-owner
) and the @RequiresOrganizationRole(OrganizationRoleType.OWNER)
annotations.
Apply this diff to align method names with endpoint paths:
- fun requiresAdmin(
+ fun requiresOwner(
@PathVariable id: Long,
) = "hello from org #$id!"
- fun requiresAdminWriteMethod(
+ fun requiresOwnerWriteMethod(
@PathVariable id: Long,
) = "hello from org #$id!"
- fun requiresAdminWriteAnnotation(
+ fun requiresOwnerWriteAnnotation(
@PathVariable id: Long,
) = "hello from org #$id!"
- fun requiresAdminReadAnnotation(
+ fun requiresOwnerReadAnnotation(
@PathVariable id: Long,
) = "hello from org #$id!"
Also applies to: 235-235, 242-242, 249-249
🤖 Prompt for AI Agents
In
backend/security/src/test/kotlin/io/tolgee/security/authorization/OrganizationAuthorizationInterceptorTest.kt
around lines 229, 235, 242 and 249, rename the test controller methods that
currently reference "Admin" (e.g., requiresAdmin, requiresAdminWriteMethod,
etc.) to use "Owner" so they match the endpoint paths (`requires-owner`) and the
@RequiresOrganizationRole(OrganizationRoleType.OWNER) annotations; update each
method name and all references/usages in the file to the new names to keep
consistency and ensure tests compile.
put: operations["removeDirectProjectPermissions"]; | ||
}; |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Operation rename: set-by-organization → removeDirectProjectPermissions
If any client code referenced the old opId/type, update it. Path stayed the same, but op name/types may be imported elsewhere.
Also applies to: 18781-18816
🏁 Script executed:
#!/bin/bash
# Check for stale references to previous op name(s)
rg -n -C2 --type=ts --type=tsx 'set-by-organization|setByOrganization|setOrganizationBase|removeDirectProjectPermissions'
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check for stale references to previous op name(s) in TS/TSX
rg -n -C2 'set-by-organization|setByOrganization|setOrganizationBase' -g '*.ts' -g '*.tsx'
Length of output: 2831
Rename client references to the renamed operation
- In webapp/src/views/projects/members/component/useUpdatePermissions.ts (≈47–56): rename the
setByOrganization
hook and returned method toremoveDirectProjectPermissions
. - In webapp/src/views/projects/members/component/MemberItem.tsx (≈76–90): update the destructured
setByOrganization
and its calls toremoveDirectProjectPermissions()
.
| "all.view" | ||
)[]; |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Scope matrix expanded with "all.view" (and hierarchy updates)
Update permission helpers, scope-to-UI mapping, and checkboxes to recognize "all.view" as read-only/global view. Ensure it doesn’t break existing “*.view” checks.
Also applies to: 1599-1600, 2694-2696, 4004-4005, 4078-4079, 19497-19498
🏁 Script executed:
#!/bin/bash
# Locate permission utilities and surface references to scopes
rg -n --type=ts --type=tsx -C2 'translations\.view|keys\.view|all\.view|hasScope|canView|scope.*view'
Length of output: 27
🏁 Script executed:
#!/bin/bash
rg -n -g '*.ts' -g '*.tsx' -C2 'translations\.view|keys\.view|all\.view|hasScope|canView|scope.*view'
Length of output: 42188
#!/bin/bash
rg -n -C3 "satisfiesPermission" -g '*.ts' -g '*.tsx'
rg -n -C3 "satisfiesLanguageAccess" -g '*.ts' -g '*.tsx'
🏁 Script executed:
#!/bin/bash
rg -n "satisfiesPermission" -g '*.ts' -g '*.tsx'
rg -n "satisfiesLanguageAccess" -g '*.ts' -g '*.tsx'
Length of output: 20340
#!/bin/bash
sed -n '1,200p' webapp/src/fixtures/permissions.ts
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' webapp/src/fixtures/permissions.ts
Length of output: 1391
Add “all.view” support in permission logic and UI
- Extend fixtures/permissions.ts so satisfiesLanguageAccess and/or SCOPE_TO_LANG_PROPERTY_MAP recognize “all.view” as a global read-only scope.
- Update your scope translation hook (useScopeTranslations.tsx) and permission-structure components (usePermissionsStructure.ts) to include “all.view” in labels and trees.
- Ensure checkboxes (GenerateApiKeyDialog, PermissionsSettings) render “all.view” and that existing “*.view” checks continue to work unchanged.
🤖 Prompt for AI Agents
In webapp/src/service/apiSchema.generated.ts around lines 1178-1179, add support
for the "all.view" global read-only scope across permission fixtures and UI:
update fixtures/permissions.ts so satisfiesLanguageAccess and
SCOPE_TO_LANG_PROPERTY_MAP recognize "all.view" as a global read-only scope;
update useScopeTranslations.tsx to include a translation/label for "all.view";
update usePermissionsStructure.ts to include "all.view" in the permission tree
and structures; ensure GenerateApiKeyDialog and PermissionsSettings render a
checkbox for "all.view" while preserving existing "*.view" wildcard checks and
behavior unchanged.
| "SERVER_SUPPORTER"; | ||
permissionModel?: components["schemas"]["PermissionModel"]; |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
New permission origins: SERVER_ADMIN, SERVER_SUPPORTER
UI displaying “permission origin” must handle these labels/icons/tooltips.
🏁 Script executed:
#!/bin/bash
# Find code mapping ComputedPermissionModel.origin to labels/icons
rg -n --type=ts --type=tsx -C2 '\bComputedPermissionModel\b|\borigin\b|permission origin'
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for usage of ComputedPermissionModel in TS and TSX files
rg -n 'ComputedPermissionModel' -g '*.ts' -g '*.tsx'
Length of output: 838
🏁 Script executed:
#!/bin/bash
echo "=== ComputedPermissionModel type snippet ==="
sed -n '1530,1580p' webapp/src/service/apiSchema.generated.ts
echo "=== computedPermission usage in GenerateApiKeyDialog.tsx ==="
rg -n 'computedPermission' -C5 -g 'webapp/src/views/userSettings/apiKeys/GenerateApiKeyDialog.tsx'
echo "=== origin usage in GenerateApiKeyDialog.tsx ==="
rg -n 'origin' -C5 -g 'webapp/src/views/userSettings/apiKeys/GenerateApiKeyDialog.tsx'
Length of output: 4681
🏁 Script executed:
#!/bin/bash
# Search for direct usage of computedPermission.origin in UI components
rg -n 'computedPermission\.origin' -g 'webapp/src/**/*.ts' -g 'webapp/src/**/*.tsx' --ignore 'webapp/src/service/apiSchema.generated.ts'
# Search for any UI usage of 'origin' keyword excluding generated schema
rg -n 'origin' -g 'webapp/src/**/*.ts' -g 'webapp/src/**/*.tsx' --ignore 'webapp/src/service/apiSchema.generated.ts'
Length of output: 231
Implement UI support for new permission origins SERVER_ADMIN and SERVER_SUPPORTER
Extend the permission-origin mapping (labels/icons/tooltips) in the component(s) that render ComputedPermissionModel.origin
to cover these two new values.
🤖 Prompt for AI Agents
In webapp/src/service/apiSchema.generated.ts around lines 1551-1552, the enum
for permission origins now includes SERVER_ADMIN and SERVER_SUPPORTER but the UI
mapping used by components that render ComputedPermissionModel.origin wasn't
updated; add entries for "SERVER_ADMIN" and "SERVER_SUPPORTER" to the
origin->metadata mapping used by those components (provide a human-friendly
label, appropriate icon identifier, and a tooltip/description), update any
TypeScript types if necessary, and ensure the rendering components use the
mapping fallback for unknown origins so these two new values display correctly
(also add/adjust unit/story tests for the rendering if present).
| "impersonation_of_admin_by_supporter_not_allowed" | ||
| "already_impersonating_user" | ||
| "operation_not_permitted_in_read_only_mode"; | ||
params?: unknown[]; |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
New error codes for read-only/impersonation
Add localized/user-friendly messages and handling for:
- operation_not_permitted_in_read_only_mode
- impersonation_of_admin_by_supporter_not_allowed
- already_impersonating_user
- unsupported_media_type (if not already mapped)
Also applies to: 5531-5534
🏁 Script executed:
#!/bin/bash
# Find error-to-message mapping and ensure new codes are covered
rg -n --type=ts --type=tsx -C2 'operation_not_permitted_in_read_only_mode|impersonation_of_admin_by_supporter_not_allowed|already_impersonating_user|unsupported_media_type|error.*code|translateError|mapError|toast.*error'
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Locate usages of the new error codes
rg -n -C2 'operation_not_permitted_in_read_only_mode|impersonation_of_admin_by_supporter_not_allowed|already_impersonating_user|unsupported_media_type' -g '*.ts' -g '*.tsx'
# Locate error-to-message mapping functions
rg -n -C2 'translateError|mapError|toast.*error' -g '*.ts' -g '*.tsx'
Length of output: 4115
Add missing error-code mappings in useErrorTranslation
The new codes impersonation_of_admin_by_supporter_not_allowed
, already_impersonating_user
and unsupported_media_type
aren’t handled—only operation_not_permitted_in_read_only_mode
is. Add case
entries returning t('…')
for each.
file: webapp/src/translationTools/useErrorTranslation.ts
🤖 Prompt for AI Agents
In webapp/src/translationTools/useErrorTranslation.ts (in the switch/handler
that maps API error codes to t('...')), add missing case entries for
"impersonation_of_admin_by_supporter_not_allowed", "already_impersonating_user",
and "unsupported_media_type" that each return the appropriate t('...')
translation key (e.g. t('impersonation_of_admin_by_supporter_not_allowed'),
t('already_impersonating_user'), t('unsupported_media_type')); keep the existing
pattern and formatting, ensure fallthroughs are avoided (use return), and run a
quick build/test to confirm no unused-import or lint errors.
/** Format: int64 */ | ||
id: number; |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
New global server role SUPPORTER
Add SUPPORTER to role selectors, badges, and guards. Ensure SUPPORTER implies read-only UX except allowed flows.
Also applies to: 5963-5964, 6752-6753
🏁 Script executed:
#!/bin/bash
# Find globalServerRole usage and role enums in UI
rg -n --type=ts --type=tsx -C2 'globalServerRole|SERVER_?ROLE|ADMIN|USER|SUPPORTER|setRole'
Length of output: 27
🏁 Script executed:
#!/bin/bash
rg -n -C2 'globalServerRole|SERVER_ROLE|ADMIN|USER|SUPPORTER|setRole' -g '*.ts' -g '*.tsx'
Length of output: 115507
Correct SUPPORTER handling in UI guards
- In ProjectPage.tsx, remove
project.computedPermission.origin === 'SERVER_SUPPORTER'
from theisAdminAccess
check so SUPPORTERs aren’t treated as admins. - In BillingMenuItem.tsx, drop
user.globalServerRole === 'SUPPORTER'
from the billing‐menu condition so SUPPORTERs only get read-only billing views (or hide billing entirely).
Implements #3218
Known limitations:
Summary by CodeRabbit
New Features
Improvements
API Changes