Skip to content

refactor eosfs auth#5514

Merged
jessegeens merged 2 commits intorelease-3.7from
refactor/eosfs-auth
Mar 24, 2026
Merged

refactor eosfs auth#5514
jessegeens merged 2 commits intorelease-3.7from
refactor/eosfs-auth

Conversation

@jessegeens
Copy link
Copy Markdown
Contributor

@jessegeens jessegeens commented Feb 20, 2026

  • Use a (configurable) dedicated service account for accesses made by external accounts,
    instead of impersonating the owner or using a token
  • Renamed the different types of auth to be more clear (e.g. cboxAuth became systemAuth)
  • Added a InvalidAuthorization to be returned instead of an empty auth; because empty auth maps to the system user (which is a sudo'er)

@update-docs
Copy link
Copy Markdown

update-docs bot commented Feb 20, 2026

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@jessegeens jessegeens force-pushed the refactor/eosfs-auth branch 29 times, most recently from b1d65d3 to 3eb4372 Compare February 26, 2026 12:49
@jessegeens jessegeens force-pushed the refactor/eosfs-auth branch 5 times, most recently from 5a04607 to f11ca3f Compare February 26, 2026 14:15
@jessegeens jessegeens changed the title WIP: refactor eosfs auth refactor eosfs auth Feb 26, 2026
@jessegeens jessegeens force-pushed the refactor/eosfs-auth branch 3 times, most recently from 0c48f71 to e95b9c1 Compare March 2, 2026 11:03
@jessegeens jessegeens marked this pull request as ready for review March 2, 2026 13:22
@jessegeens jessegeens force-pushed the refactor/eosfs-auth branch 5 times, most recently from 34d2c4f to e07a916 Compare March 4, 2026 09:16
Comment thread pkg/storage/fs/eos/client/grpc/auth.go Outdated
Comment thread pkg/storage/fs/eos/auth.go
Comment thread pkg/storage/fs/eos/auth.go Outdated
return invalidAuth(), fmt.Errorf("eosfs: no user found in context")
}

if fs.conf.ForceSingleUserMode {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This code is repeated. Seems a good candidate for a split into another function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe we can even consider getting rid of it? I think this stems from when owncloud did everything under one user. But today I don't really see a use case for this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If that is the case and we don't have a reason to keep it, go ahead. @glpatcern, with your history knowledge, what you think?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, as far as I remember this was the operating model of ownCloud 10 and older, and we never used it. Let's drop it.

Comment thread pkg/storage/fs/eos/client/grpc/auth.go Outdated
Comment thread pkg/storage/fs/eos/auth.go
Comment thread pkg/storage/fs/eos/eosfs.go
Comment thread pkg/storage/fs/eos/file_info.go
Comment thread pkg/storage/fs/eos/file_ops.go
@jessegeens jessegeens force-pushed the refactor/eosfs-auth branch from e07a916 to e5327fa Compare March 6, 2026 09:38
@jessegeens jessegeens changed the base branch from master to release-3.7 March 24, 2026 12:16
Copy link
Copy Markdown
Contributor

@diocas diocas left a comment

Choose a reason for hiding this comment

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

Fine to merge, but I would still prefer to see this line once and used in different places:

nobody := uint64(65534)

I don't see why we need to create a cyclical dependency for this.

* Use a (configurable) dedicated service account for accesses made by external accounts,
  instead of impersonating the owner or using a token
* Renamed the different types of auth to be more clear (e.g. cboxAuth became systemAuth)
* Added a `InvalidAuthorization` to be returned instead of an empty auth; because empty auth maps to the system user (which is a sudo'er)
@jessegeens jessegeens force-pushed the refactor/eosfs-auth branch from e5327fa to d390ff2 Compare March 24, 2026 14:28
@glpatcern glpatcern requested a review from diocas March 24, 2026 14:56
@jessegeens jessegeens force-pushed the refactor/eosfs-auth branch from f6a8c1f to 6727694 Compare March 24, 2026 16:01
@jessegeens jessegeens merged commit cafd0dc into release-3.7 Mar 24, 2026
11 of 12 checks passed
@diocas diocas deleted the refactor/eosfs-auth branch March 24, 2026 18:25
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.

3 participants