Skip to content

Commit 3424e02

Browse files
authored
feat: Shared-drive share-by-link improvements(permissions and other) (#4691)
What do we want to achieve: - A shared-drive file/folder shouldn't have multiple links - A write-capable drive recipient can create and update that link - A read-only drive recipient can see read-only links, but must not see writable links - The drive app must be able to retrieve and display existing links for files shown in the drive
2 parents 54f9217 + 0ee5cd5 commit 3424e02

File tree

12 files changed

+1193
-155
lines changed

12 files changed

+1193
-155
lines changed

docs/shared-drives.md

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,10 +676,63 @@ drive.
676676
The following routes manage share-by-link permissions scoped to files inside a
677677
shared drive:
678678

679+
- `GET /sharings/drives/:id/permissions?ids=...`
679680
- `POST /sharings/drives/:id/permissions`
680681
- `PATCH /sharings/drives/:id/permissions/:perm-id`
681682
- `DELETE /sharings/drives/:id/permissions/:perm-id`
682683

684+
### GET /sharings/drives/:id/permissions?ids=...
685+
686+
Lists the share-by-link permissions for the requested file or folder IDs inside
687+
the shared drive.
688+
689+
Authorization rules:
690+
691+
- The shared-drive owner can list all matching links.
692+
- A write-capable recipient can list writable and read-only links.
693+
- A read-only recipient only sees read-only links.
694+
695+
Validation:
696+
697+
- `ids` is required and must be a comma-separated list of file or folder IDs.
698+
- Every requested ID must belong to the shared drive.
699+
700+
Status codes:
701+
702+
- `200 OK` listed
703+
- `403 Forbidden` caller cannot access shared-drive permissions
704+
- `422 Unprocessable Entity` missing or invalid `ids`
705+
706+
### POST /sharings/drives/:id/permissions
707+
708+
Creates a share-by-link permission for one file or folder in the shared drive.
709+
The request body uses the same JSON:API shape as [`POST /permissions`](permissions.md#post-permissions).
710+
711+
Authorization rules:
712+
713+
- The shared-drive owner can create a link.
714+
- A write-capable recipient can create a link.
715+
- A read-only recipient cannot create a link.
716+
717+
Validation:
718+
719+
- The permission set must target exactly one file or folder.
720+
- The target type must be `io.cozy.files`.
721+
- Selectors are not supported.
722+
- The target must belong to the shared drive and must be readable by the
723+
caller.
724+
- Only one share-by-link permission can exist per target. A second creation
725+
attempt on the same target returns a conflict, regardless of which member
726+
created the existing link.
727+
728+
Status codes:
729+
730+
- `200 OK` created
731+
- `400 Bad Request` invalid permission set or invalid target
732+
- `403 Forbidden` caller lacks access to the target or is read-only on the
733+
shared drive
734+
- `409 Conflict` a share-by-link permission already exists for this target
735+
683736
### PATCH /sharings/drives/:id/permissions/:perm-id
684737

685738
Updates an existing share-by-link permission.
@@ -696,12 +749,19 @@ Allowed updates:
696749

697750
- `password`
698751
- `expires_at`
752+
- `permissions` (same target only)
699753

700754
Validation:
701755

702756
- `password` must be a string (empty string clears the password).
703757
- `expires_at` must be a string (empty string clears expiration, otherwise
704758
RFC3339 date-time).
759+
- `permissions`, when provided, must still target the same file or folder
760+
inside the shared drive.
761+
- A write-capable creator or the owner can promote a read-only link to a
762+
writable link if their current token grants those verbs.
763+
- A read-only shared-drive recipient cannot patch a permission set to add
764+
writable verbs.
705765

706766
Status codes:
707767

@@ -721,6 +781,7 @@ Authorization rules:
721781
- The shared-drive owner can revoke any share-by-link permission.
722782
- The creator of a share-by-link permission can revoke the permission they
723783
created.
784+
- A read-only shared-drive recipient cannot revoke a permission.
724785
- Public share tokens (`share`, `share-preview`) cannot revoke permissions.
725786

726787
Status codes:
@@ -731,6 +792,23 @@ Status codes:
731792
to this shared drive
732793
- `404 Not Found` permission ID does not exist
733794

795+
## Delegated email sharing
796+
797+
Members of a shared drive add new recipients through the sharing API, not
798+
through a drive-specific route:
799+
800+
- `POST /sharings/:sharing-id/recipients`
801+
802+
When that request is sent from a recipient Cozy, the stack delegates the
803+
operation to the owner Cozy internally.
804+
805+
Authorization rules:
806+
807+
- The shared-drive owner can add recipients as for any other sharing.
808+
- A write-capable recipient can invite read-write or read-only recipients.
809+
- A read-only recipient can invite only read-only recipients.
810+
- A read-only recipient receives `403 Forbidden` for a read-write invite.
811+
734812

735813
## Versions
736814

docs/sharing.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,9 +1046,13 @@ Content-Type: application/vnd.api+json
10461046
### POST /sharings/:sharing-id/recipients/delegated
10471047

10481048
This is an internal route for the stack. It is called by the recipient cozy on
1049-
the owner cozy to add recipients and groups to the sharing (`open_sharing:
1050-
true` only). Data for direct recipients should contain an email address but if
1051-
it is not known, an instance URL can also be provided.
1049+
the owner cozy to add recipients and groups to the sharing. It is used for
1050+
delegated recipient additions from recipient Cozys, including shared-drive
1051+
recipient invitations. Data for direct recipients should contain an email
1052+
address but if it is not known, an instance URL can also be provided. For
1053+
shared drives, each delegated recipient or group can also carry a `read_only`
1054+
flag, and the owner applies the drive-specific reshare rules when processing
1055+
the request.
10521056

10531057
#### Request
10541058

model/permission/permissions.go

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ func updateAppSet(db prefixer.Prefixer, doc *Permission, typ, docType, slug stri
492492
return doc, nil
493493
}
494494

495-
func checkSetPermissions(set Set, parent *Permission) error {
495+
func CheckSetPermissions(set Set, parent *Permission) error {
496496
if parent.Type != TypeWebapp &&
497497
parent.Type != TypeKonnector &&
498498
parent.Type != TypeOauth &&
@@ -557,7 +557,7 @@ func CreateShareSet(
557557
) (*Permission, error) {
558558
set := subdoc.Permissions
559559
if !skipValidation {
560-
if err := checkSetPermissions(set, parent); err != nil {
560+
if err := CheckSetPermissions(set, parent); err != nil {
561561
return nil, err
562562
}
563563
}
@@ -723,6 +723,76 @@ func GetPermissionsForIDs(db prefixer.Prefixer, doctype string, ids []string) (m
723723
return result, nil
724724
}
725725

726+
// GetShareByLinkPermissionsForTargets returns share-by-link permission docs for
727+
// the given target documents. Results are grouped by target ID. The underlying
728+
// view emits one row per rule/value, so permissions are deduplicated by
729+
// permission ID within each target.
730+
func GetShareByLinkPermissionsForTargets(
731+
db prefixer.Prefixer,
732+
doctype string,
733+
ids []string,
734+
) (map[string][]*Permission, error) {
735+
if len(ids) == 0 {
736+
return map[string][]*Permission{}, nil
737+
}
738+
739+
keys := make([]interface{}, len(ids))
740+
for i, id := range ids {
741+
keys[i] = []string{doctype, "_id", id}
742+
}
743+
744+
var res struct {
745+
Rows []struct {
746+
ID string `json:"id"`
747+
Key []string `json:"key"`
748+
Doc json.RawMessage `json:"doc"`
749+
} `json:"rows"`
750+
}
751+
err := couchdb.ExecView(db, couchdb.PermissionsShareByDocView, &couchdb.ViewRequest{
752+
Keys: keys,
753+
IncludeDocs: true,
754+
}, &res)
755+
if err != nil {
756+
return nil, err
757+
}
758+
759+
permsByTarget := make(map[string][]*Permission, len(keys))
760+
seen := make(map[string]struct{}, len(res.Rows))
761+
for _, row := range res.Rows {
762+
if len(row.Key) < 3 || len(row.Doc) == 0 {
763+
continue
764+
}
765+
766+
targetID := row.Key[2]
767+
seenKey := targetID + "\x00" + row.ID
768+
if _, ok := seen[seenKey]; ok {
769+
continue
770+
}
771+
seen[seenKey] = struct{}{}
772+
773+
perm := &Permission{}
774+
if err := json.Unmarshal(row.Doc, perm); err != nil {
775+
return nil, err
776+
}
777+
permsByTarget[targetID] = append(permsByTarget[targetID], perm)
778+
}
779+
780+
return permsByTarget, nil
781+
}
782+
783+
// GetShareByLinkPermissionsForTarget returns share-by-link permission docs for a single target document.
784+
func GetShareByLinkPermissionsForTarget(
785+
db prefixer.Prefixer,
786+
doctype string,
787+
id string,
788+
) ([]*Permission, error) {
789+
permsByTarget, err := GetShareByLinkPermissionsForTargets(db, doctype, []string{id})
790+
if err != nil {
791+
return nil, err
792+
}
793+
return permsByTarget[id], nil
794+
}
795+
726796
// GetPermissionsByDoctype returns the list of all permissions of the given
727797
// type (shared-with-me by example) that have at least one rule for the given
728798
// doctype. The cursor will be modified in place.

model/permission/permissions_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -369,27 +369,27 @@ func TestShareSetPermissions(t *testing.T) {
369369
setEvents := Set{Rule{Type: "io.cozy.events"}}
370370

371371
parent := &Permission{Type: TypeCLI, Permissions: setEvents}
372-
err := checkSetPermissions(setFiles, parent)
372+
err := CheckSetPermissions(setFiles, parent)
373373
assert.Error(t, err)
374374

375375
parent.Type = TypeWebapp
376-
err = checkSetPermissions(setFiles, parent)
376+
err = CheckSetPermissions(setFiles, parent)
377377
assert.Error(t, err)
378378

379379
parent.Permissions = setFiles
380-
err = checkSetPermissions(setFiles, parent)
380+
err = CheckSetPermissions(setFiles, parent)
381381
assert.NoError(t, err)
382382

383383
parent.Type = TypeShareInteract
384-
err = checkSetPermissions(setFiles, parent)
384+
err = CheckSetPermissions(setFiles, parent)
385385
assert.NoError(t, err)
386386

387387
parent.Type = TypeWebapp
388-
err = checkSetPermissions(setFilesWildCard, parent)
388+
err = CheckSetPermissions(setFilesWildCard, parent)
389389
assert.Error(t, err)
390390

391391
parent.Permissions = setFilesWildCard
392-
err = checkSetPermissions(setFilesWildCard, parent)
392+
err = CheckSetPermissions(setFilesWildCard, parent)
393393
assert.NoError(t, err)
394394

395395
// share-interact subset ignores selector/values and only checks type+verbs,
@@ -408,18 +408,18 @@ func TestShareSetPermissions(t *testing.T) {
408408
Values: []string{"drive-root-id"},
409409
}},
410410
}
411-
err = checkSetPermissions(childFileRule, parent)
411+
err = CheckSetPermissions(childFileRule, parent)
412412
assert.NoError(t, err)
413413

414414
// For non share-interact parents, values remain part of subset checks.
415415
parent.Type = TypeWebapp
416-
err = checkSetPermissions(childFileRule, parent)
416+
err = CheckSetPermissions(childFileRule, parent)
417417
assert.Error(t, err)
418418

419419
// share-interact still enforces verbs coverage.
420420
parent.Type = TypeShareInteract
421421
childFileRule[0].Verbs = Verbs(POST)
422-
err = checkSetPermissions(childFileRule, parent)
422+
err = CheckSetPermissions(childFileRule, parent)
423423
assert.Error(t, err)
424424
}
425425

model/sharing/member.go

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -374,10 +374,14 @@ func (s *Sharing) SendDelegated(inst *instance.Instance, api *APIDelegateAddCont
374374
ParseError: ParseRequestError,
375375
}
376376
res, err := request.Req(opts)
377+
originalRes, originalErr := res, err
377378
if res != nil && res.StatusCode/100 == 4 {
378379
res, err = RefreshToken(inst, res, err, s, &s.Members[0], c, opts, body)
379380
}
380381
if err != nil {
382+
if originalRes != nil && originalRes.StatusCode == http.StatusForbidden {
383+
return preserveDelegatedRequestError(originalRes.StatusCode, originalErr)
384+
}
381385
return err
382386
}
383387
defer res.Body.Close()
@@ -433,8 +437,33 @@ func (s *Sharing) SendDelegated(inst *instance.Instance, api *APIDelegateAddCont
433437
return s.SendInvitationsToMembers(inst, api.members, states)
434438
}
435439

436-
// AddDelegatedContact adds a contact on the owner cozy, but for a contact from
437-
// a recipient (open_sharing: true only)
440+
func preserveDelegatedRequestError(status int, err error) error {
441+
reqErr, ok := err.(*request.Error)
442+
if !ok {
443+
return jsonapi.NewError(status, http.StatusText(status))
444+
}
445+
446+
var payload struct {
447+
Errors jsonapi.ErrorList `json:"errors"`
448+
}
449+
if parseErr := json.Unmarshal([]byte(reqErr.Detail), &payload); parseErr == nil && len(payload.Errors) > 0 {
450+
if payload.Errors[0].Status == 0 {
451+
payload.Errors[0].Status = status
452+
}
453+
if payload.Errors[0].Title == "" {
454+
payload.Errors[0].Title = http.StatusText(status)
455+
}
456+
return payload.Errors[0]
457+
}
458+
459+
if reqErr.Detail != "" {
460+
return jsonapi.NewError(status, reqErr.Detail)
461+
}
462+
return jsonapi.NewError(status, http.StatusText(status))
463+
}
464+
465+
// AddDelegatedContact adds a contact on the owner cozy for a contact proposed
466+
// by a recipient.
438467
func (s *Sharing) AddDelegatedContact(inst *instance.Instance, m Member) (string, error) {
439468
m.Status = MemberStatusPendingInvitation
440469
if m.Instance != "" || m.Email == "" {

web/permissions/permissions.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ type CreateShareByLinkOptions struct {
138138
ValidatePermissions ValidatePermissionsFn
139139
// controls whether CreateShareSet's standard permission validation is skipped.
140140
SkipValidation bool
141+
// overrides the app slug stored in metadata.
142+
CreatorSlug *string
141143
// overrides the creator domain stored in metadata.
142144
CreatorDomain *string
143145
}
@@ -171,9 +173,12 @@ func HandleCreateShareByLink(c echo.Context, inst *instance.Instance, opts Creat
171173
}
172174

173175
var slug string
176+
if opts.CreatorSlug != nil {
177+
slug = *opts.CreatorSlug
178+
}
174179
sourceID := parent.SourceID
175180
// Check if the permission is linked to an OAuth Client
176-
if parent.Client != nil {
181+
if slug == "" && parent.Client != nil {
177182
oauthClient := parent.Client.(*oauth.Client)
178183
if slug = oauth.GetLinkedAppSlug(oauthClient.SoftwareID); slug != "" {
179184
// Changing the sourceID from the OAuth clientID to the classic
@@ -185,8 +190,12 @@ func HandleCreateShareByLink(c echo.Context, inst *instance.Instance, opts Creat
185190
// Getting the slug from the token if it has not been retrieved before
186191
// with the linkedapp
187192
if slug == "" {
188-
claims := c.Get("claims").(permission.Claims)
189-
slug = claims.Subject
193+
if claims, ok := c.Get("claims").(permission.Claims); ok {
194+
switch claims.AudienceString() {
195+
case consts.AppAudience, consts.KonnectorAudience, consts.AccessTokenAudience:
196+
slug = claims.Subject
197+
}
198+
}
190199
}
191200

192201
names := strings.Split(c.QueryParam("codes"), ",")

0 commit comments

Comments
 (0)