mirror/opencloud
1
0
Fork 0
mirror of https://github.com/opencloud-eu/opencloud.git synced 2026-06-14 02:54:44 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix(graph): translate sharing roles consitently

Browse Source 
GetRoleDefinition/s does now handle l10n correctly. Previsouly it just
returned the non-localized string. What made things worse was that
ListPermissions() mutated global list of available roles and replaced
some strings with translated values depending on the `accept-language`
header. Which resulted in GetRoleDefinition returning results in mixed
localization depending on who/what called ListPermissions before.

Fixes: #2800
This commit is contained in:
Ralf Haferkamp
2026-06-11 14:31:44 +02:00
parent 3c0d70987d
commit d163c8ed29
5 changed files with 307 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
services/graph/pkg/service/v0/api_driveitem_permissions.go
View File

@@ -22,7 +22,6 @@ import (
"github.com/go-chi/chi/v5"
"github.com/go-chi/render"
libregraph "github.com/opencloud-eu/libre-graph-api-go"
revactx "github.com/opencloud-eu/reva/v2/pkg/ctx"
"github.com/opencloud-eu/reva/v2/pkg/publicshare"
"github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool"
@@ -30,16 +29,14 @@ import (
"github.com/opencloud-eu/reva/v2/pkg/storagespace"
"github.com/opencloud-eu/reva/v2/pkg/utils"
"github.com/opencloud-eu/opencloud/pkg/l10n"
l10n_pkg "github.com/opencloud-eu/opencloud/services/graph/pkg/l10n"
"github.com/opencloud-eu/opencloud/services/graph/pkg/odata"
"github.com/opencloud-eu/opencloud/pkg/conversions"
"github.com/opencloud-eu/opencloud/pkg/l10n"
"github.com/opencloud-eu/opencloud/pkg/log"
"github.com/opencloud-eu/opencloud/services/graph/pkg/config"
"github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode"
"github.com/opencloud-eu/opencloud/services/graph/pkg/identity"
"github.com/opencloud-eu/opencloud/services/graph/pkg/identity/cache"
"github.com/opencloud-eu/opencloud/services/graph/pkg/odata"
"github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole"
"github.com/opencloud-eu/opencloud/services/graph/pkg/validate"
)
@@ -755,16 +752,8 @@ func (api DriveItemPermissionsApi) ListPermissions(w http.ResponseWriter, r *htt
loc := r.Header.Get(l10n.HeaderAcceptLanguage)
w.Header().Add("Content-Language", loc)
if loc != "" && loc != "en" {
err := l10n_pkg.TranslateEntity(loc, "en", permissions,
l10n.TranslateEach("LibreGraphPermissionsRolesAllowedValues",
l10n.TranslateField("Description"),
l10n.TranslateField("DisplayName"),
),
)
if err != nil {
api.logger.Error().Err(err).Msg("tranlation error")
}
for i := range permissions.LibreGraphPermissionsRolesAllowedValues {
permissions.LibreGraphPermissionsRolesAllowedValues[i] = unifiedrole.LocalizeRole(&permissions.LibreGraphPermissionsRolesAllowedValues[i], loc)
}
render.Status(r, http.StatusOK)
@@ -806,16 +795,8 @@ func (api DriveItemPermissionsApi) ListSpaceRootPermissions(w http.ResponseWrite
loc := r.Header.Get(l10n.HeaderAcceptLanguage)
w.Header().Add("Content-Language", loc)
if loc != "" && loc != "en" {
err := l10n_pkg.TranslateEntity(loc, "en", permissions,
l10n.TranslateEach("LibreGraphPermissionsRolesAllowedValues",
l10n.TranslateField("Description"),
l10n.TranslateField("DisplayName"),
),
)
if err != nil {
api.logger.Error().Err(err).Msg("tranlation error")
}
for i := range permissions.LibreGraphPermissionsRolesAllowedValues {
permissions.LibreGraphPermissionsRolesAllowedValues[i] = unifiedrole.LocalizeRole(&permissions.LibreGraphPermissionsRolesAllowedValues[i], loc)
}
render.Status(r, http.StatusOK)

9
services/graph/pkg/service/v0/rolemanagement.go
View File

@@ -7,14 +7,17 @@ import (
"github.com/go-chi/chi/v5"
"github.com/go-chi/render"
"github.com/opencloud-eu/opencloud/pkg/l10n"
"github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode"
"github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole"
)
// GetRoleDefinitions a list of permission roles than can be used when sharing with users or groups
func (g Graph) GetRoleDefinitions(w http.ResponseWriter, r *http.Request) {
loc := r.Header.Get(l10n.HeaderAcceptLanguage)
w.Header().Add("Content-Language", loc)
render.Status(r, http.StatusOK)
render.JSON(w, r, g.availableRoles)
render.JSON(w, r, unifiedrole.LocalizeRoles(g.availableRoles, loc))
}
// GetRoleDefinition a permission role than can be used when sharing with users or groups
@@ -32,6 +35,8 @@ func (g Graph) GetRoleDefinition(w http.ResponseWriter, r *http.Request) {
errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, err.Error())
return
}
loc := r.Header.Get(l10n.HeaderAcceptLanguage)
w.Header().Add("Content-Language", loc)
render.Status(r, http.StatusOK)
render.JSON(w, r, role)
render.JSON(w, r, unifiedrole.LocalizeRole(role, loc))
}

170
services/graph/pkg/service/v0/rolemanagement_test.go Normal file
View File

@@ -0,0 +1,170 @@
package svc_test
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
libregraph "github.com/opencloud-eu/libre-graph-api-go"
"github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool"
cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks"
"google.golang.org/grpc"
"github.com/opencloud-eu/opencloud/pkg/shared"
"github.com/opencloud-eu/opencloud/services/graph/mocks"
"github.com/opencloud-eu/opencloud/services/graph/pkg/config"
"github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults"
service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0"
"github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole"
)
var _ = Describe("RoleManagement", func() {
var (
svc service.Service
gatewayClient *cs3mocks.GatewayAPIClient
gatewaySelector pool.Selectable[gateway.GatewayAPIClient]
eventsPublisher mocks.Publisher
permSvc mocks.Permissions
cfg *config.Config
rr *httptest.ResponseRecorder
ctx context.Context
)
BeforeEach(func() {
rr = httptest.NewRecorder()
ctx = context.Background()
cfg = defaults.FullDefaultConfig()
cfg.Identity.LDAP.CACert = ""
cfg.TokenManager.JWTSecret = "loremipsum"
cfg.Commons = &shared.Commons{}
cfg.GRPCClientTLS = &shared.GRPCClientTLS{}
pool.RemoveSelector("GatewaySelector" + "eu.opencloud.api.gateway")
gatewayClient = &cs3mocks.GatewayAPIClient{}
gatewaySelector = pool.GetSelector[gateway.GatewayAPIClient](
"GatewaySelector",
"eu.opencloud.api.gateway",
func(cc grpc.ClientConnInterface) gateway.GatewayAPIClient {
return gatewayClient
},
)
eventsPublisher = mocks.Publisher{}
permSvc = mocks.Permissions{}
var err error
svc, err = service.NewService(
service.Config(cfg),
service.WithGatewaySelector(gatewaySelector),
service.EventsPublisher(&eventsPublisher),
service.PermissionService(&permSvc),
)
Expect(err).ToNot(HaveOccurred())
})
Describe("GetRoleDefinitions", func() {
It("returns all available roles in English when no Accept-Language is set", func() {
r := httptest.NewRequest(http.MethodGet, "/graph/v1beta1/roleManagement/permissions/roleDefinitions", nil)
r = r.WithContext(ctx)
svc.ServeHTTP(rr, r)
Expect(rr.Code).To(Equal(http.StatusOK))
var roles []libregraph.UnifiedRoleDefinition
Expect(json.Unmarshal(rr.Body.Bytes(), &roles)).To(Succeed())
Expect(roles).NotTo(BeEmpty())
viewer := findRoleByID(roles, unifiedrole.UnifiedRoleViewerID)
Expect(viewer).NotTo(BeNil())
Expect(viewer.GetDisplayName()).To(Equal("Can view"))
})
It("returns translated roles when Accept-Language is German", func() {
r := httptest.NewRequest(http.MethodGet, "/graph/v1beta1/roleManagement/permissions/roleDefinitions", nil)
r.Header.Set("Accept-Language", "de")
r = r.WithContext(ctx)
svc.ServeHTTP(rr, r)
Expect(rr.Code).To(Equal(http.StatusOK))
Expect(rr.Header().Get("Content-Language")).To(Equal("de"))
var roles []libregraph.UnifiedRoleDefinition
Expect(json.Unmarshal(rr.Body.Bytes(), &roles)).To(Succeed())
viewer := findRoleByID(roles, unifiedrole.UnifiedRoleViewerID)
Expect(viewer).NotTo(BeNil())
Expect(viewer.GetDisplayName()).To(Equal("Kann anzeigen"))
Expect(viewer.GetDescription()).To(Equal("Ansehen und herunterladen."))
})
It("does not mutate the global buildInRoles after a German request", func() {
r := httptest.NewRequest(http.MethodGet, "/graph/v1beta1/roleManagement/permissions/roleDefinitions", nil)
r.Header.Set("Accept-Language", "de")
r = r.WithContext(ctx)
svc.ServeHTTP(rr, r)
Expect(rr.Code).To(Equal(http.StatusOK))
// A second request without a locale must still return English
rr2 := httptest.NewRecorder()
r2 := httptest.NewRequest(http.MethodGet, "/graph/v1beta1/roleManagement/permissions/roleDefinitions", nil)
r2 = r2.WithContext(ctx)
svc.ServeHTTP(rr2, r2)
Expect(rr2.Code).To(Equal(http.StatusOK))
var roles []libregraph.UnifiedRoleDefinition
Expect(json.Unmarshal(rr2.Body.Bytes(), &roles)).To(Succeed())
viewer := findRoleByID(roles, unifiedrole.UnifiedRoleViewerID)
Expect(viewer).NotTo(BeNil())
Expect(viewer.GetDisplayName()).To(Equal("Can view"))
})
})
Describe("GetRoleDefinition", func() {
It("returns a single role in English when no Accept-Language is set", func() {
r := httptest.NewRequest(http.MethodGet, "/graph/v1beta1/roleManagement/permissions/roleDefinitions/"+unifiedrole.UnifiedRoleViewerID, nil)
r = r.WithContext(ctx)
svc.ServeHTTP(rr, r)
Expect(rr.Code).To(Equal(http.StatusOK))
var role libregraph.UnifiedRoleDefinition
Expect(json.Unmarshal(rr.Body.Bytes(), &role)).To(Succeed())
Expect(role.GetDisplayName()).To(Equal("Can view"))
})
It("returns a single role translated when Accept-Language is German", func() {
r := httptest.NewRequest(http.MethodGet, "/graph/v1beta1/roleManagement/permissions/roleDefinitions/"+unifiedrole.UnifiedRoleViewerID, nil)
r.Header.Set("Accept-Language", "de")
r = r.WithContext(ctx)
svc.ServeHTTP(rr, r)
Expect(rr.Code).To(Equal(http.StatusOK))
Expect(rr.Header().Get("Content-Language")).To(Equal("de"))
var role libregraph.UnifiedRoleDefinition
Expect(json.Unmarshal(rr.Body.Bytes(), &role)).To(Succeed())
Expect(role.GetDisplayName()).To(Equal("Kann anzeigen"))
Expect(role.GetDescription()).To(Equal("Ansehen und herunterladen."))
})
It("returns 404 for an unknown roleID", func() {
r := httptest.NewRequest(http.MethodGet, "/graph/v1beta1/roleManagement/permissions/roleDefinitions/unknown-role-id", nil)
r = r.WithContext(ctx)
svc.ServeHTTP(rr, r)
Expect(rr.Code).To(Equal(http.StatusNotFound))
})
})
})
// findRoleByID returns the first role with the given ID from the slice, or nil.
func findRoleByID(roles []libregraph.UnifiedRoleDefinition, id string) *libregraph.UnifiedRoleDefinition {
for i := range roles {
if roles[i].GetId() == id {
return &roles[i]
}
}
return nil
}

44
services/graph/pkg/unifiedrole/roles.go
View File

@@ -6,11 +6,11 @@ import (
"strings"
libregraph "github.com/opencloud-eu/libre-graph-api-go"
"github.com/opencloud-eu/reva/v2/pkg/conversions"
"google.golang.org/protobuf/proto"
"github.com/opencloud-eu/reva/v2/pkg/conversions"
"github.com/opencloud-eu/opencloud/pkg/l10n"
graphl10n "github.com/opencloud-eu/opencloud/services/graph/pkg/l10n"
)
const (
@@ -546,6 +546,46 @@ func weightRoles(roleSet []*libregraph.UnifiedRoleDefinition, constraints string
return roleSet
}
// cloneRole returns a shallow struct copy of r with independent allocations for
// the Description and DisplayName pointer fields — the only fields mutated by
// TranslateEntity. All other fields (Id, LibreGraphWeight, RolePermissions) are
// either read-only or stripped before rendering, so sharing their values is safe.
func cloneRole(r *libregraph.UnifiedRoleDefinition) libregraph.UnifiedRoleDefinition {
c := *r
if r.Description != nil {
s := *r.Description
c.Description = &s
}
if r.DisplayName != nil {
s := *r.DisplayName
c.DisplayName = &s
}
return c
}
// LocalizeRole returns a translated, independent copy of a single role definition.
// The global buildInRoles singleton is never modified.
func LocalizeRole(r *libregraph.UnifiedRoleDefinition, locale string) libregraph.UnifiedRoleDefinition {
c := cloneRole(r)
if locale != "" && locale != "en" {
_ = graphl10n.TranslateEntity(locale, "en", &c,
l10n.TranslateField("Description"),
l10n.TranslateField("DisplayName"),
)
}
return c
}
// LocalizeRoles returns a translated, independent copy of each role definition.
// The global buildInRoles singleton is never modified.
func LocalizeRoles(roles []*libregraph.UnifiedRoleDefinition, locale string) []libregraph.UnifiedRoleDefinition {
out := make([]libregraph.UnifiedRoleDefinition, len(roles))
for i, r := range roles {
out[i] = LocalizeRole(r, locale)
}
return out
}
// GetAllowedResourceActions returns the allowed resource actions for the provided role by condition
func GetAllowedResourceActions(role *libregraph.UnifiedRoleDefinition, condition string) []string {
if role == nil {

82
services/graph/pkg/unifiedrole/roles_test.go
View File

@@ -2,6 +2,7 @@ package unifiedrole_test
import (
"slices"
"sync"
"testing"
. "github.com/onsi/gomega"
@@ -267,3 +268,84 @@ func TestGetAllowedResourceActions(t *testing.T) {
})
}
}
func TestLocalizeRole_English(t *testing.T) {
g := NewWithT(t)
original := unifiedrole.RoleViewer
result := unifiedrole.LocalizeRole(original, "en")
// Strings are unchanged for English
g.Expect(result.GetDisplayName()).To(Equal(original.GetDisplayName()))
g.Expect(result.GetDescription()).To(Equal(original.GetDescription()))
// Result is an independent copy — mutating it must not touch the global
translated := "mutated"
result.DisplayName = &translated
g.Expect(original.GetDisplayName()).NotTo(Equal("mutated"))
}
func TestLocalizeRole_German(t *testing.T) {
g := NewWithT(t)
original := unifiedrole.RoleViewer
result := unifiedrole.LocalizeRole(original, "de")
g.Expect(result.GetDisplayName()).To(Equal("Kann anzeigen"))
g.Expect(result.GetDescription()).To(Equal("Ansehen und herunterladen."))
// Global singleton must be untouched
g.Expect(original.GetDisplayName()).NotTo(Equal("Kann anzeigen"))
g.Expect(original.GetDescription()).NotTo(Equal("Ansehen und herunterladen."))
}
func TestLocalizeRole_EmptyLocale(t *testing.T) {
g := NewWithT(t)
original := unifiedrole.RoleViewer
result := unifiedrole.LocalizeRole(original, "")
// Empty locale falls back to source strings
g.Expect(result.GetDisplayName()).To(Equal(original.GetDisplayName()))
g.Expect(result.GetDescription()).To(Equal(original.GetDescription()))
}
func TestLocalizeRoles_German(t *testing.T) {
g := NewWithT(t)
roles := unifiedrole.BuildInRoles
results := unifiedrole.LocalizeRoles(roles, "de")
g.Expect(results).To(HaveLen(len(roles)))
// Every result is a value (not a pointer)
for i, r := range results {
// Id is preserved
g.Expect(r.GetId()).To(Equal(roles[i].GetId()))
// Global singleton is not mutated
g.Expect(roles[i].GetDisplayName()).NotTo(Equal(r.GetDisplayName()),
"global displayName for role %s was mutated", r.GetId())
}
}
func TestLocalizeRole_ConcurrentCallsDoNotRace(t *testing.T) {
// Run with -race to detect data races on the global buildInRoles strings.
const goroutines = 20
var wg sync.WaitGroup
wg.Add(goroutines)
for i := 0; i < goroutines; i++ {
locale := "de"
if i%2 == 0 {
locale = "fr"
}
go func(loc string) {
defer wg.Done()
_ = unifiedrole.LocalizeRoles(unifiedrole.BuildInRoles, loc)
}(locale)
}
wg.Wait()
// After all concurrent translations the globals must still hold English strings
g := NewWithT(t)
g.Expect(unifiedrole.RoleViewer.GetDisplayName()).To(Equal("Can view"))
}