Skip to content
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

[full-ci] fix an error when lock/unlock a file #4518

Merged
merged 3 commits into from Mar 7, 2024

Conversation

2403905
Copy link

@2403905 2403905 commented Feb 15, 2024

Description:
We fixed a bug when anonymous user with viewer role in public link of a folder can lock/unlock a file inside it

Related issue:
owncloud/ocis#7785
owncloud/ocis#7761

Related PR:
owncloud/ocis#8472

@2403905 2403905 marked this pull request as ready for review February 19, 2024 13:40
@2403905 2403905 requested review from labkode, a team and glpatcern as code owners February 19, 2024 13:40
@butonic
Copy link
Contributor

butonic commented Feb 19, 2024

shouldn't this be handled in the internal/grpc/services/storageprovider/storageprovider.go:

// SetLock puts a lock on the given reference
func (s *service) SetLock(ctx context.Context, req *provider.SetLockRequest) (*provider.SetLockResponse, error) {
	err := s.storage.SetLock(ctx, req.Ref, req.Lock)

	return &provider.SetLockResponse{
		Status: status.NewStatusFromErrType(ctx, "set lock", err),
	}, nil
}

// GetLock returns an existing lock on the given reference
func (s *service) GetLock(ctx context.Context, req *provider.GetLockRequest) (*provider.GetLockResponse, error) {
	lock, err := s.storage.GetLock(ctx, req.Ref)

	return &provider.GetLockResponse{
		Status: status.NewStatusFromErrType(ctx, "get lock", err),
		Lock:   lock,
	}, nil
}

// RefreshLock refreshes an existing lock on the given reference
func (s *service) RefreshLock(ctx context.Context, req *provider.RefreshLockRequest) (*provider.RefreshLockResponse, error) {
	err := s.storage.RefreshLock(ctx, req.Ref, req.Lock, req.ExistingLockId)

	return &provider.RefreshLockResponse{
		Status: status.NewStatusFromErrType(ctx, "refresh lock", err),
	}, nil
}

// Unlock removes an existing lock from the given reference
func (s *service) Unlock(ctx context.Context, req *provider.UnlockRequest) (*provider.UnlockResponse, error) {
	err := s.storage.Unlock(ctx, req.Ref, req.Lock)

	return &provider.UnlockResponse{
		Status: status.NewStatusFromErrType(ctx, "unlock", err),
	}, nil
}

If we only check the current users public share role on the HTTP API this will still be possible on the CS3 API.

@2403905
Copy link
Author

2403905 commented Feb 19, 2024

It's a fair point

@2403905 2403905 force-pushed the public-link-lock branch 5 times, most recently from 5e2acf6 to 219ec22 Compare February 22, 2024 20:19
Copy link
Contributor

@butonic butonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The switch should be extended and the errors should all be from the errors package.

internal/http/services/owncloud/ocdav/locks.go Outdated Show resolved Hide resolved
@2403905 2403905 requested a review from butonic February 26, 2024 19:49
@2403905 2403905 force-pushed the public-link-lock branch 2 times, most recently from 960dc70 to 5baf636 Compare March 5, 2024 13:40
Copy link
Contributor

@butonic butonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no failing tests? wow! but we should do an ocis bump full test run to verify ;-)

@2403905 2403905 merged commit 9da2e18 into cs3org:edge Mar 7, 2024
9 checks passed
rhafer pushed a commit to rhafer/reva that referenced this pull request Mar 12, 2024
* [full-ci] fix an error when lock/unlock a public shared file

* added errtype to http code mapping

* reworked the error handling

---------

Co-authored-by: Roman Perekhod <rperekhod@owncloud.com>
(cherry picked from commit 9da2e18)
rhafer pushed a commit that referenced this pull request Mar 12, 2024
* [full-ci] fix an error when lock/unlock a public shared file

* added errtype to http code mapping

* reworked the error handling

---------

Co-authored-by: Roman Perekhod <rperekhod@owncloud.com>
(cherry picked from commit 9da2e18)
@micbar micbar mentioned this pull request Mar 13, 2024
71 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants