[Bug]: Unlisted breaking change in v2.6.0 with virtual folders returned by extrenal hooks

[Mathieu-COSYNS]

⚠️ This issue respects the following points: ⚠️

• This is a bug, not a question or a configuration issue.
• This issue is not already reported on Github (I've searched it).

Bug description

In v2.6.0:

• Virtual folder must be created before being returned by the external auth hook (in user.virtual_folders).
• External auth hook must include the virtual folder id.

If the external auth hook does not provide a virtual folder id SFTPGo will:

• Error with NOT NULL constraint failed: users_folders_mapping.folder_id if no virtual folder with the same name exist
• Error with error creating root directory "/virtual_path" for user "test": mkdir : no such file or directory when listing the virtual folder via SSH, if a virtual folder with the same name exist
• Work fine in the web UI (???)

Is this new behavior expected ? If the answer is yes, is it possible to add this change in the Backward incompatible changes list and create an error clearly state that user external auth hook need to return virtual folders with ids. I personally would prefer to keep the old behavior. This new behavior will create a dependency both way between my external auth and SFTPGo.

Steps to reproduce

I created a repo with the smallest config to reproduce the issue (https://github.com/Mathieu-COSYNS/sftpgo-external-auth-virtual-folder-issue)

TLDR;

1. Create a external hook that return a user config with a virtual folder without id
2. Try to login with a user that use the external hook

and/or

1. Create a external hook that return a user config with a virtual folder without id
2. Create a virtual folder with the same name that the one returned by the hook
3. Try to login with a user that use the external hook

Expected behavior

When returning a virtual folder from a external auth hook, SFTPGo creates the virtual folder if no virtual folder with the same name exist.

SFTPGo version

v2.6.0

Data provider

any

Installation method

Community Docker image

Configuration

Checkout (https://github.com/Mathieu-COSYNS/sftpgo-external-auth-virtual-folder-issue)

Relevant log output

No response

What are you using SFTPGo for?

Private user, home usecase (home backup/VPS)

Additional info

Relevant code changes from SFTPGo v2.5.6 changed in v2.6.0

sftpgo/openapi/openapi.yaml

Line 5872 in a6a92f0

description: 'A virtual folder is a mapping between a SFTPGo virtual path and a filesystem path outside the user home directory. The specified paths must be absolute and the virtual path cannot be "/", it must be a sub directory. The parent directory for the specified virtual path must exist. SFTPGo will try to automatically create any missing parent directory for the configured virtual folders at user login.'

sftpgo/internal/dataprovider/sqlcommon.go

Lines 2339 to 2350 in a6a92f0

	func sqlCommonAddOrUpdateFolder(ctx context.Context, baseFolder *vfs.BaseVirtualFolder, usedQuotaSize int64,
	usedQuotaFiles int, lastQuotaUpdate int64, dbHandle sqlQuerier,
	) error {
	fsConfig, err := json.Marshal(baseFolder.FsConfig)
	if err != nil {
	return err
	}
	q := getUpsertFolderQuery()
	_, err = dbHandle.ExecContext(ctx, q, baseFolder.MappedPath, usedQuotaSize, usedQuotaFiles,
	lastQuotaUpdate, baseFolder.Name, baseFolder.Description, fsConfig)
	return err
	}

sftpgo/internal/dataprovider/sqlcommon.go

Line 2574 in a6a92f0

err := sqlCommonAddOrUpdateFolder(ctx, &vfolder.BaseVirtualFolder, 0, 0, 0, dbHandle)

[drakkan]

Updated release notes, thanks for noticing. See #1349

I think it should work if you return an existing virtual folder by name.

[Mathieu-COSYNS]

Hi, thank you for the quick reply. I didn't knew about #1349. I was hopping to still be able to create virtual folders on the fly when returning user configuration. It's a bit annoying but I will adapt my approach for my external auth hook.

I think it should work if you return an existing virtual folder by name.

As I was explaining above, it does work for the Web UI but not for SSH (and FTP, WebDAV) for those you need the virtual folder id. This behavior should be consistent.

Error with error creating root directory "/virtual_path" for user "test": mkdir : no such file or directory when listing the virtual folder via SSH, if a virtual folder with the same name exist

One approach could be to change the user/group create/update API virtual_folders to take only virtual folders virtual_path, quota_size, quota_files, folder_id instead of full virtual folders configuration since setting other fields will/should not have any impact.

Should we continue this in another issue or reopen this one ?

[drakkan]

I did a quick test to confirm, returning an existing folder name is enough, the id is not required.

The OpenAPI document should be updated in several places to clarify the fields required for adding/updating and the fields returned in GETs.
If you want to help us and feel comfortable with our CLA feel free to send a PR. The PR should be complete and working to be accepted, we have currently no time for reviewing and iterating or pushing back on PRs (this is often more work than doing the change ourself). Thanks for understanding and for using SFTPGo