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

Support upload saver without username/password #5455

Merged
merged 1 commit into from
Jan 31, 2021

Conversation

simonbaird
Copy link
Contributor

The default behaviour is unchanged, but if you write "yes" to
$:/UploadWithUrlOnly then it will assume it's possible to upload
with a blank username and password, as long as the host is set.

The motivation is to support a upload plugin compatible upload
service that uses some method to authenticate other than the legacy
upload plugin user/password params.

Without this patch, the user would need to enter something random in
the user and password fields for TW to decide the upload plugin can
be used.

The default behaviour is unchanged, but if you write "yes" to
$:/UploadWithUrlOnly then it will assume it's possible to upload
with a blank username and password, as long as the host is set.

The motivation is to support a upload plugin compatible upload
service that uses some method to authenticate other than the legacy
upload plugin user/password params.

Without this patch, the user would need to enter something random in
the user and password fields for TW to decide the upload plugin can
be used.
@simonbaird
Copy link
Contributor Author

The patch doesn't (yet) expose "$:/UploadWithUrlOnly" in the UI, so it's kind of like an undocumented feature, which is fine for my specific purposes, but I could expose it as a checkbox somewhere if that seems useful. Let me know your thoughts.

return false;
if (uploadWithUrlOnly === "yes") {
// The url is good enough. No need for a username and password.
// Assume the server uses some other kind of auth mechanism.
Copy link
Contributor

Choose a reason for hiding this comment

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

hmmmm, I'm OK, that there is an option, that upload is possible, if there is an other auth mechanism in place. But this setting seems to be a system tiddler. Wouldn't it be better, if the option would be a "startup parameter" for the server itself, like --listen xxx eg: --allow-upload-with-url-only=yes or something similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By "server" here I mean something like Tiddlyspot. Imagine something like Tiddlyspot where the user creates a site, the empty TW file is created with $:/UploadURL pre-populated, then the user visits the new site and clicks save. Right now, it won't work unless the username and password are non-empty. The username could be pre-populated also, but I don't know how to pre-populate an upload password. And since they will be ignored, it seems preferrable not to require them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the long term I'm thinking I could create a dedicated saver, but in the short term, this is a small patch that makes the existing upload.js work smoothly for this use case.

@simonbaird
Copy link
Contributor Author

Further context:

@pmario
Copy link
Contributor

pmario commented Jan 30, 2021

I'm probably the wrong one here, but I do sense a security risk. @Arlen22 @inmysocks What do you think

@Arlen22
Copy link
Contributor

Arlen22 commented Jan 30, 2021

Definitely don't expose it as a checkbox. You should still use the username and password if it is set.

Also it should be named $:/config/UploadUrl and $:/config/UploadWithUrlOnly.

Other than that nothing stands out, but then I don't really work with this part of things either.

@simonbaird
Copy link
Contributor Author

Thanks for the reviews. I'm happy to call this low priority at the moment.

Perhaps I will create a dedicated new saver with exactly the behavior I'm looking for, instead of shoe horning the existing one.

@Jermolene
Copy link
Owner

Hi @simonbaird it's great to hear you are working on a successor to TiddlySpot, and I'm excited to see your goals.

This kind of hidden setting is a reasonable solution.

To @Arlen22's point about the naming of the config tiddler, in this case I think it makes sense to use $:/UploadWithUrlOnly for consistency with the other existing settings of the upload saver (it was written before introducing the $:/config/ prefix).

@Jermolene Jermolene merged commit 12f1847 into Jermolene:master Jan 31, 2021
@simonbaird
Copy link
Contributor Author

Cool, thanks!

simonbaird added a commit to simonbaird/tiddlyhost that referenced this pull request Jan 31, 2021
Now that Jermolene/TiddlyWiki5#5455 is
merged we can use a standard empty file.
simonbaird added a commit to simonbaird/tiddlyhost that referenced this pull request Feb 1, 2021
Now that Jermolene/TiddlyWiki5#5455 is
merged we can use a standard empty file.
simonbaird added a commit to simonbaird/tiddlyhost that referenced this pull request Feb 1, 2021
Now that Jermolene/TiddlyWiki5#5455 is
merged we can use a standard empty file.
@simonbaird
Copy link
Contributor Author

simonbaird commented Feb 12, 2021

FYI https://tiddlyhost.com/

(I don't want to publicise it widely yet, but you're welcome to give it a gentle test.)

@Jermolene
Copy link
Owner

FYI https://tiddlyhost.com/

Awesome, it's just like old times, only much, much better!

I successfully created an account, created a wiki and edited it on several browsers.

https://jeremy-test.tiddlyhost.com

(I don't want to publicise it widely yet, but you're welcome to give it a gentle test.)

Great, do let us know when you're ready.

@morosanuae
Copy link
Contributor

I get the error "plan must exists" when I try to create an account. I don't know what that means.

@kookma
Copy link
Contributor

kookma commented Feb 13, 2021

@simonbaird wonderful. I created https://kookma.tiddlyhost.com/ and it works great!
As I wrote in discussion board simonbaird/tiddlyhost#10 (comment) for me tiddlyhost is a little slow.

@simonbaird
Copy link
Contributor Author

FYI https://github.com/simonbaird/tiddlyhost/wiki/Using-a-5.1.23-or-earlier-version-of-TiddlyWiki helps explains the motivation for this change.

simonbaird added a commit to simonbaird/tiddlyhost that referenced this pull request Apr 3, 2021
Now that Jermolene/TiddlyWiki5#5455 is
merged we can use a standard empty file.
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.

None yet

6 participants