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

feat(drivers): add fine-grained control for link signing #3924

Merged
merged 13 commits into from
Mar 24, 2023

Conversation

SugarNekoE
Copy link
Contributor

Provide link signature authentication option for Local Driver.
Reuse the original Sign function for link signature.
Clear the sign parameter for URLs that don't require a signature.

Season Shi added 6 commits March 22, 2023 23:17
NOT TESTED: TokenKV Function
Add File based KV func
I found that the original Sign function is enough to complete the link signature, and only need to add simple configuration items to meet the requirements.
@welcome
Copy link

welcome bot commented Mar 22, 2023

Thanks for opening this pull request! Please check out our contributing guidelines.

@SugarNekoE SugarNekoE changed the title Add Sign Link option for Local Driver feet(drivers): add sign link option for Local Driver Mar 22, 2023
@SugarNekoE SugarNekoE changed the title feet(drivers): add sign link option for Local Driver feat(drivers): add sign link option for Local Driver Mar 22, 2023
@SugarNekoE
Copy link
Contributor Author

After testing, only proxy url works, I will push a new commit to support direct url. (Tonight)

@xhofe
Copy link
Collaborator

xhofe commented Mar 23, 2023

I can't understand why you do this?

@SugarNekoE
Copy link
Contributor Author

Something similar to UpYun's Token anti-leech, because the existing Local storage can obtain files without any verification, which is not safe. I think it can be easily implemented by using the existing Sign function.

But obviously last night I only did the part of the Proxy link, and the Direct link mode still lacks protection.

In addition, in a special case, the link will be generated as a proxy link, but the proxy link function is not enabled, and the Sign parameter is also generated, although it is never been used.

(For me, this problem occurs when accessing AList directly through Localhost.)

So I added a judgement to clean up the redundant Sign parameter after generation.

@xhofe
Copy link
Collaborator

xhofe commented Mar 23, 2023

I don't think it's necessary to add sign for a single special driver, beacuse there is a global config to control this.

If you want more fine-grained control, add enable_sign to the basic storage struct maybe a better choice which make us can enable sign for every driver rather than just for LocalStorage.

@SugarNekoE
Copy link
Contributor Author

ok i'll try it tonight.

SugarNekoE and others added 2 commits March 23, 2023 18:01
Can enable sign for every driver now.

Bug: When sign enabled, in download page, Copy link doesn't contain a sign.

(Not done yet)
@SugarNekoE SugarNekoE changed the title feat(drivers): add sign link option for Local Driver feat(drivers): add fine-grained control for link signing Mar 23, 2023
Response of fsread function does not contain sign.
@SugarNekoE
Copy link
Contributor Author

It's done! Tested on Local Driver and Aliyundrive Driver.

public/dist/README.md Outdated Show resolved Hide resolved
server/handles/fsread.go Show resolved Hide resolved
server/handles/fsread.go Outdated Show resolved Hide resolved
SugarNekoE and others added 3 commits March 24, 2023 21:46
- Add back public/dist/README.md

- Enable sign when DownProxyUrl is enabled

- Merge needSign() to isEncrypt() in fsread.go
@SugarNekoE
Copy link
Contributor Author

All fixed.

@xhofe xhofe merged commit 1123630 into AlistGo:main Mar 24, 2023
@welcome
Copy link

welcome bot commented Mar 24, 2023

Congrats on merging your first pull request! We here at behavior bot are proud of you!

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.

2 participants