feat(guestlinks)!: auth-guest service by maki5 · Pull Request #3609 · opencloud-eu/opencloud

maki5 · 2026-09-28T07:13:08Z

Guest Auth service:

New guestauth service — part of the default service set, no OC_ADD_RUN_SERVICES needed.
Event consumer — subscribes to share events (ShareCreated, ShareRemoved, ShareExpired) and reacts to them.
Token creation on share creation — when a guest-link share is created, the service generates a secure random token, stores it, and publishes a GuestTokenCreated message so the invitation link can be sent later.
Cleanup — tokens are removed when the share is removed or expires.
Redeem endpoint — POST /graph/v1beta1/guestInvitations/redeem exchanges a token for a session cookie. Tokens are single-use: a token that was already redeemed, is invalid, or whose share no longer exists/is expired is rejected.
Persistent storage — one JSON record per share in a directory tree, behind an abstraction so the backend can be replaced later.
Proxy integration — the redeem route is registered as a public (unauthenticated) route so guests can reach it.

Breaking change

Existing installations will not start until AUTH_GUEST_JWT_SECRET is set
(the new auth-guest service is part of the default service set and requires
it; there is intentionally no fallback to OC_JWT_SECRET).

Migration:

set AUTH_GUEST_JWT_SECRET=<random secret> in the environment, or
regenerate config so it is created (new installs get it via opencloud init).

closes: #3069
dependent on: opencloud-eu/libre-graph-api#76

codacy-production · 2026-09-28T07:24:32Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 253 complexity

Metric Results

Complexity 253

View in Codacy

🟢 Coverage 29.25% diff coverage · +0.08% coverage variation

Metric Results

Coverage variation ✅ +0.08% coverage variation (-1.00%)

Diff coverage ✅ 29.25% diff coverage

View coverage diff in Codacy

Coverage variation details

Coverable lines Covered lines Coverage

Common ancestor commit (8e51430) 89530 21781 24.33%

Head commit (aeb11b0) 90725 (+1195) 22143 (+362) 24.41% (+0.08%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

Coverable lines Covered lines Diff coverage

Pull request (#3609) 1200 351 29.25%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

aduffeck

Just a few things to refine. Please also add license headers to the new files.

aduffeck · 2026-09-28T11:50:47Z

+
+type FileStorage struct {
+	root string
+	mu   sync.Mutex


To make the service scalable (on a RWX storage) we should use actual posix file locks instead of a Mutex here I think.

aduffeck · 2026-09-28T13:32:46Z

+
+type Token struct {
+	ShareIDHash string
+	SecretHash  string


Why do we need the SecretHash field? it's always simply a sha256sum of the secret, so it looks redundant to me. Also the candidate.SecretHash != secretHash check in line 65 can never be false.
I think I'd rather replace it with a SecretHash() or HashedSecret() function which returns a (cached?) hash of the secret.

aduffeck · 2026-09-28T13:59:06Z

+	Sharer       *user.UserId
+	ItemID       *provider.ResourceId
+	ResourceName string
+	Token        string


This event should include the grantee's email adress so the notification service doesn't have to look up the share anymore.

aduffeck · 2026-09-28T14:22:02Z

+}
+
+func (s *FileStorage) path(shareIDHash string) string {
+	return filepath.Join(s.root, shareIDHash[:2], shareIDHash[2:4], shareIDHash[4:]+".json")


This could panic when verifying a malformed token and we need to be careful with path traversials.
Maybe filepath.Clean() plus verifying the length of the segments would do the trick?

aduffeck · 2026-09-28T14:29:33Z

+	Redeemed    bool      `json:"redeemed"`
+}
+
+type Storage interface {


Could we name this differently? The implementations don't just store, they also include business logic (redeeming tokens now, probably thins like adding pin verification in the future), so maybe something like Backend or similar would be more appropriate.

Not sure if this is better, but in reva these kinds of interfaces are called Manager I think.

aduffeck · 2026-09-28T14:34:11Z

+// Validate validates the config
+func Validate(cfg *config.Config) error {
+	if cfg.TokenManager == nil || cfg.TokenManager.JWTSecret == "" {
+		return shared.MissingJWTTokenError(cfg.Service.Name)


If I'm not mistaken this will break existing installations? We'll have to make sure to bump the version and write according release notes when merging this.

aduffeck · 2026-09-28T14:38:18Z

+		ShareID:     shareID,
+		ShareIDHash: tok.ShareIDHash,
+		SecretHash:  tok.SecretHash,
+		Expiry:      utils.TSToTime(share.GetExpiration()),


The token expiry shouldn't be the share expiration date but time.Now().Add(30 * time.Minute) according to #2513

aduffeck · 2026-09-28T14:40:55Z

+				log.Debug().Err(err).Msg("token already redeemed")
+				w.WriteHeader(http.StatusConflict)
+			case errors.Is(err, guestauth.ErrExpired):
+				log.Debug().Err(err).Msg("token expired")


The errors should follow what was discussed in #2517 (comment) and e.g. return a 401 with something like

{ "error_type": "token_expired", "message": "Your token has expired.", "share_id": "<the-share-id>", }

in the body here.

aduffeck · 2026-09-28T14:45:54Z

+
+	Service Service `yaml:"-"`
+
+	LogLevel string `yaml:"loglevel" env:"OC_LOG_LEVEL;GUESTAUTH_LOG_LEVEL" desc:"The log level. Valid values are: 'panic', 'fatal', 'error', 'warn', 'info', 'debug', 'trace'." introductionVersion:"1.0.0"`


Suggested change

LogLevel string `yaml:"loglevel" env:"OC_LOG_LEVEL;GUESTAUTH_LOG_LEVEL" desc:"The log level. Valid values are: 'panic', 'fatal', 'error', 'warn', 'info', 'debug', 'trace'." introductionVersion:"1.0.0"`

LogLevel string `yaml:"loglevel" env:"OC_LOG_LEVEL;GUESTAUTH_LOG_LEVEL" desc:"The log level. Valid values are: 'panic', 'fatal', 'error', 'warn', 'info', 'debug', 'trace'." introductionVersion:"%%NEXT%%"`

Same for all the other new env vars that are being introduced here.

aduffeck · 2026-09-28T14:48:27Z

+	defer s.mu.Unlock()
+
+	p := s.path(shareIDHash)
+	if _, err := os.Stat(p); err != nil {


Maybe handle not exists from os.Remove() instead of the stat?

dragotin · 2026-09-28T15:13:08Z

I would suggest that the new service is named auth-guest. We already have quite a bunch of auth-* named services that deal with different types of authentication.

maki5 · 2026-09-29T08:08:14Z

@aduffeck about the license headers, I found just this one:

// Copyright 2026 OpenCloud GmbH <mail@opencloud.eu>
// SPDX-License-Identifier: Apache-2.0

should it be used or there is somewhere some other header? also to which files it should be applied? all .go files from the service?

aduffeck · 2026-09-29T11:16:15Z

@aduffeck about the license headers, I found just this one:
// Copyright 2026 OpenCloud GmbH <mail@opencloud.eu>
// SPDX-License-Identifier: Apache-2.0
should it be used or there is somewhere some other header? also to which files it should be applied? all .go files from the service?

Right, this one to all new source files we add.

aduffeck · 2026-10-01T09:11:26Z

@@ -74,13 +97,30 @@ func (s *FileManager) Redeem(shareIDHash string) error {
 	return s.add(rec)
 }

+func (s *FileManager) lockStore() (*flock.Flock, error) {


Could we just lock the particular record that's being mutated instead of the global store lock?

rhafer · 2026-10-01T11:18:41Z

 		cfg.Groups.Commons = cfg.Commons
 		return groups.Execute(cfg.Groups)
 	})
+	if opts.Config.Commons != nil && opts.Config.Commons.EnableGuestLinks {


This doesn't currently seem to work. The opts.Config.Commons struct is still nil here (unless the yaml config file has a section named shared).

AFAICS this is a bug in the top-level config parser ("pkg/config/parser/parse.go") which does not initialize cfg.Commons in EnsureDefaults().

rhafer · 2026-10-01T14:07:30Z

Could we maybe add the diagram (the mermaid source): #2513 (comment) to the README?

maki5 · 2026-10-01T14:19:32Z

Could we maybe add the diagram (the mermaid source): #2513 (comment) to the README?

I will add it, but I believe it describes the whole flow not just the service, also probably it will need to be updated with renewal endpoint flow but this will be part of renewal PR

… be based on interfaces

…lementation

…ency with libre-graph-api

…pired and shareRemoved events, common config loading fix

aduffeck · 2026-10-06T07:02:27Z

/cc @kulmann @v-scharf jfyi we're renaming the GRAPH_ENABLE_GUEST_INVITES environment variable to OC_ENABLE_GUEST_LINKS with this PR, so you'll have to adapt your docker-compose.yml and .woodpecker.star in web accordingly when you pull this change in.

maki5 requested review from aduffeck, pbleser-oc and rhafer September 28, 2026 07:13

maki5 added Status:Needs-Review Component:Backend labels Sep 28, 2026

maki5 marked this pull request as ready for review September 28, 2026 09:09

rhafer added the Type:Enhancement label Sep 28, 2026

rhafer reviewed Sep 28, 2026

View reviewed changes

Comment thread services/guestauth/pkg/server/http/server.go Outdated

aduffeck requested changes Sep 28, 2026

View reviewed changes

rhafer reviewed Sep 29, 2026

View reviewed changes

Comment thread services/guestauth/pkg/config/defaults/defaultconfig.go Outdated

maki5 force-pushed the feat/guestauth_service branch from e8ce608 to 3ffdb78 Compare September 29, 2026 14:42

maki5 changed the title ~~feat(guestlinks): Guest Auth service~~ feat(guestlinks)!: add auth-guest service Sep 29, 2026

maki5 added the Type:Breaking-Change label Sep 29, 2026

maki5 changed the title ~~feat(guestlinks)!: add auth-guest service~~ feat(guestlinks)!: auth-guest service Sep 29, 2026

maki5 requested review from aduffeck and rhafer September 29, 2026 15:03

rhafer mentioned this pull request Sep 29, 2026

Add "guestlinks" authmanager opencloud-eu/reva#822

Merged

maki5 mentioned this pull request Sep 29, 2026

feat(authguest): added redeem endpoint for auth-guest service opencloud-eu/libre-graph-api#76

Merged

rhafer mentioned this pull request Sep 29, 2026

guestlinks: implement auth-middleware for Guestlink cookies #3618

Merged

aduffeck reviewed Oct 1, 2026

View reviewed changes

rhafer reviewed Oct 1, 2026

View reviewed changes

maki5 force-pushed the feat/guestauth_service branch from 72774b6 to 456c8ae Compare October 1, 2026 13:45

maki5 force-pushed the feat/guestauth_service branch from 9a960da to 39784fe Compare October 5, 2026 12:15

feat(guestauth): added guestauth service skeleton

19fc7df

maki5 added 22 commits October 6, 2026 08:07

feat(guestauth): register gusetauth service into opencloud config

a1cb335

feat(guestauth): register guestauth proxy

402a0c0

feat(guestauth): added mocks for tests, made services dependencies to…

6ea563d

… be based on interfaces

feat(guestauth): cookie set always to true due to codacy mention

45bc705

feat(guestauth): removed uneeded config field

d359a7d

feat(guestauth): upd service readme

fdf52e4

feat(guestauth): upd version tags for env vars

2106f60

feat(guestauth): renamed Storage interface and dependencies to Manager

b4aa47c

feat(guestauth): moved SecretHash to a method instead of field in struct

e5c3be7

feat(guetauth): upd the redeem route

b920654

feat(guestauth): upd token record expiry proporety value on add

9088273

feat(guestauth): upd redeem response error format

eac68b9

feat(guestauth): used file lock instead of mutex in file strorage imp…

cf77e47

…lementation

feat(guestauth): grantee email in GuestTokenCreated event

f86e339

feat(guestauth): guestauth service rename to auth-guest

cbf3ee8

feat(guestauth): prefix for cookie

d084709

feat(guestauth): aligned readme

e17a9a3

feat(guestauth): updated redeem endpoint response and dependencies

340851d

feat(guestauth): global flag to disable the guest links feature

a1a8221

feat(guestauth): renaming guest ivitations to guest links for consist…

c5879ca

…ency with libre-graph-api

feat(guestauth): upd file manager lock, addtional filetrs for shareEx…

2c89825

…pired and shareRemoved events, common config loading fix

feat(guestauth): upd readme with guest links flow diagram

c1dc12d

aduffeck force-pushed the feat/guestauth_service branch from 39784fe to c1dc12d Compare October 6, 2026 06:08

Adapt to renamed env var about guest links

aeb11b0

aduffeck approved these changes Oct 6, 2026

View reviewed changes

aduffeck enabled auto-merge October 6, 2026 07:04

aduffeck merged commit 6ff4794 into opencloud-eu:main Oct 6, 2026
67 of 68 checks passed

openclouders mentioned this pull request Oct 6, 2026

🎉 Release 9.0.0 #3665

Open

dschmidt mentioned this pull request Oct 6, 2026

Push based thumbnailer architecture #3397

Open


		Service Service `yaml:"-"`

		LogLevel string `yaml:"loglevel" env:"OC_LOG_LEVEL;GUESTAUTH_LOG_LEVEL" desc:"The log level. Valid values are: 'panic', 'fatal', 'error', 'warn', 'info', 'debug', 'trace'." introductionVersion:"1.0.0"`

Conversation

maki5 commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking change

Uh oh!

codacy-production Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

Uh oh!

aduffeck left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dragotin commented Sep 28, 2026

Uh oh!

maki5 commented Sep 29, 2026

Uh oh!

aduffeck commented Sep 29, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhafer commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maki5 commented Oct 1, 2026

Uh oh!

aduffeck commented Oct 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maki5 commented Sep 28, 2026 •

edited

Loading

codacy-production Bot commented Sep 28, 2026 •

edited

Loading

rhafer commented Oct 1, 2026 •

edited

Loading