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

Enhancement: Add support for --share-password option when --create-share-link is called. #3119

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

merclyn
Copy link

@merclyn merclyn commented Feb 13, 2025

I have added support for passing in a password that will be required when the share link is accessed. This will allow for tighter security around sharing file and directory links.

This comment has been minimized.

This comment has been minimized.

@abraunegg
Copy link
Owner

@merclyn
Please can you update:

  • man page to reflect the added CLI option
  • add the details in application-config-options.md to document the new option
  • update --create-share-link documentation to reflect the use of this option as well

Other Questions I have

  • Can this be set via 'share_password' in the config file? Does this potentially pose a security issue? If present as a config option, should it 100% always be ignored so that it can only be set via a CLI option? Same password for all links created?
  • Operational Conflicts ... the new option can be set / used anytime ... I feel this should throw an operation conflict if the user adds this without using --create-share-link as in the past folk arbitrarily add / use config options without reading when they should be used .. so IMHO if --share-password is used without --create-share-link - the application should throw an operational conflict error
  • If --share-password ' ' is used (that is, using a space as the password) should this be allowed? Should the password actually be something valid? This raises another question - password strength / complexity ... yes this is a user problem, however the aim here of this PR is to implement a level of security around the anonymous link that is being created - so having a level of security around the actual password to ensure that the user is not just creating a low bar hurdle (ie use the password 'password' or similar) that is easily guessed is avoided. The application currently has this function generateAlphanumericString() that automatically generates a 16 character random string, you can pass in a numeric value to get longer or shorter ... so perhaps negate having the user provide a password, and generate a random one - and have that printed out ? Something to think about option wise.
  • An alternative here to setting a password is also changing the scope to 'organisation' - as then this enforces people must login to view the file via the URL

@abraunegg abraunegg added this to the v2.5.5 milestone Feb 14, 2025
@abraunegg abraunegg added the In Progress Currently being worked on label Feb 14, 2025
@abraunegg abraunegg changed the title Add support for --share-password option when --create-share-link is called. Enhancement: Add support for --share-password option when --create-share-link is called. Feb 14, 2025
@merclyn
Copy link
Author

merclyn commented Feb 14, 2025

I am reviewing your comments and will implement the suggestions over the next few days.

I agree that the password should not be able to be ' ' since im pretty sure that would not be valid on the onedrive platform.

I also think that this password should not be saved inside the config file since that is generally not best practices for password policy.

as far as the organization scope (Instead of anonymous) It seemed like the API docs were saying that you needed to be inside the current organization. I will test this and possibly add another option and another pull request if my tests product good results.

This comment has been minimized.

Copy link

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

Unrecognized words (41)
bashisms
capitalisation
containerised
customisable
customisation
cuz
finalise
finalised
fullstop
gzipping
initialising
Ioctl
ISPs
kinda
Kubuntu
logfiles
Lubuntu
malware
meh
optimisations
preinstalled
prioritisation
prioritise
prolly
reparse
resynchronisation
sandboxing
segfault
shitload
Slackware
sourced
Sourcing
spamming
thingies
Thu
uninitialised
uninstallation
upgradable
utilisation
websites
Xubuntu
To accept these unrecognized words as correct, you could run the following commands

... in a clone of the git@github.com:merclyn/onedrive.git repository
on the master branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/main/apply.pl' |
perl - 'https://github.com/abraunegg/onedrive/actions/runs/13356069834/attempts/1'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Progress Currently being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants