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 for basic authentication or tokens for @updateURL #517

Closed
peanball opened this issue Jul 26, 2023 · 20 comments
Closed

Support for basic authentication or tokens for @updateURL #517

peanball opened this issue Jul 26, 2023 · 20 comments

Comments

@peanball
Copy link

This is a feature request, possibly a bug fix if the limitation is not intentional.

I have a script in a private GitHub Enterprise repo. Even "public" repositories there require a personal access token.

The URL is something like this: https://ghe_NOT_MY_ACTUAL_TOKEN@raw.github.example.com/peanball/main/userscripts/cool_script.js

Adding this URL to @updateURL should just keep fetching whatever current version is in main. This works great for my workflow.

Testing the same script with the same @updateURL in Firefox with Violentmonkey works fine.

Userscripts instead claims that the URL is unreachable.

image

I've browsed through the code a little and see that URLs are sanitised before being parsed as Swift URL. Possibly something goes amiss there?

@quoid
Copy link
Owner

quoid commented Jul 27, 2023

@peanball In the macOS Console it should log a reason as to why this update failed. At this line you will see a bunch of potential errors that can got logged. If you can provide that, it can be traced back as to why it is failing.

If you can provide me with a valid test url, I can try to find some time to debug this.

I took a quick look and it is failing at (4) in the above line. I am not entirely sure why. However, I can circumvent this with this url pattern, using the token that is generated when hitting the RAW button on a file. I have no idea how long these tokens last for or if this is a truly viable alternative.

https://raw.githubusercontent.com/quoid/someRepo/master/static/example.user.js?token=XYZ

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 27, 2023

https://user:pass@userjs.deno.dev/DEBUG.BasicAuthentication.user.js

I made a basic authentication user script URL and tested it, everything works fine both when installing from the URL and when updating the user script.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 27, 2023

My advice is to use an external tool (or a browser not logged into GitHub) to test that your address can be accessed correctly. For example in a shell tool:

curl -i https://user:pass@userjs.deno.dev/DEBUG.BasicAuthentication.user.js

You could try changing parts of the URL to get different responses (401, 404, 200), which are also confirmed in Userscripts.

So it looks like there is no issue to implement or fix here, I will close this issue, if you still have problems feel free to comment.

@ACTCD ACTCD closed this as completed Jul 27, 2023
@peanball
Copy link
Author

peanball commented Jul 27, 2023

It works in curl of course. I’ve made sure that the URL I’m trying is reasonable.

As I wrote, the same URL also works correctly (for adding the initial script via URL and updating via @updateURL) when testing in violentmonkey.

I’m offering to help debug/improve this and opened the issue as requested in the CONTRIBUTING file.

The GitHub UI raw token is valid for a few hours or days. I want to use a personal access token to distribute a company internal script for my team. So that short lived token will not work for our use case.

For completeness, I’ve also tried with a username and password, via: https://x-access-token:$TheToken@raw.github.example.com…

This also works in curl and is a valid “dummy” user name supported by GitHub.

Thanks for the hint with the console app. I will dive in deeper and probably start a debugging session to see what’s up.

Please reopen.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 27, 2023

@peanball Have you tried the demo URL address I provided? Does it work in your local Userscripts?

@ACTCD ACTCD reopened this Jul 27, 2023
@peanball
Copy link
Author

@ACTCD Your URL works fine in Userscripts.

With mine I'm getting a 404 ultimately.

My hunch is that somewhere in func getRemoteFileContents(_ url: String) -> String?, my URL is modified and may lose something.

My real file name is something like this: https://x-access-token:ghp_NOT_MY_ACTUAL_TOKEN@raw.github.tools.tld/CFN/logsearch-userscripts/main/LogSearch Annotations for CFN.js

I've tried escaping the spaces in file names with:

I end up with a 404 from the server when used in Userscripts. Curl works fine of course when escaped (i.e. with %20), but does not work with + either.

Unfortunately all of the intermediate URLs (e.g. checkedUrl, encodedUrl, solidUrl) are never logged so we can't observe what's happening in between. I will try to build a modified version and dig deeper.

Later today (it's just before 8 AM in Europe) I will provide a new link from a GitHub private repo that should exhibit the same issues. I'm not comfortable, even sharing a read-only public repo token from our company GitHub.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 27, 2023

@peanball

OK, now I've updated that URL with two new demos that contain spaces.

You could try them out and they still work with or without URL encode.

https://user:pass@userjs.deno.dev/DEBUG.Basic Authentication 1.user.js
https://user:pass@userjs.deno.dev/DEBUG.Basic%20Authentication%202.user.js

Let me be more straightforward, use the URL sample you provided directly (just replacing the domain name), and they still work fine:

https://x-access-token:ghp_NOT_MY_ACTUAL_TOKEN@userjs.deno.dev/CFN/logsearch-userscripts/main/LogSearch Annotations for CFN.js
https://x-access-token:ghp_NOT_MY_ACTUAL_TOKEN@userjs.deno.dev/CFN/logsearch-userscripts/main/LogSearch%20Annotations%20for%20CFN.js

@peanball
Copy link
Author

@ACTCD yes, those URLs work fine.

I will debug with our server and will get back. Please don't close just yet. I will either explain what it was and close or provide a PR to fix the behaviour.

@quoid
Copy link
Owner

quoid commented Jul 28, 2023

I tried getRemoteFileContents, and related code, outside of the context of the project using this format:

// @updateURL https://ghp_TOKEN@raw.githubusercontent.com/quoid/k21p/master/static/example.user.js

And it worked...:thinking: I am not sure why it is failing when running the same exact function within the project.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 28, 2023

I think I found the problem.

When the App requests the URL, even if the auth information is already included in the URL, it will be omitted in the first request.

After receiving the 401 status returned by the server, the request will be made again with auth information, and the request will be successful.

But when using the GitHub URL, client will get 404 directly when app make the first request, and the request fails.

Using curl will not have this problem, because the auth information is carried in the first request.

So the problem is that GitHub's behavior is not expected in traditional, it will directly return 404 instead of 401 when the auth is not carried.

This will be the exact issue we need to resolve. Attempts to control or customize this default request behavior. Make it able to carry the auth information the first request.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 28, 2023

After reading more material, I don't think we should attempt to implement this custom request behavior.

  • According to Apple documentation and support thread, this practice is not recommended.
  • According to Fetch Standard issue, the usage of auth within URLs seems to be unsupported and no longer recommended in wider use.
  • According to GitHub docs, this is not a recommended or applicable usage or scenario. (So GitHub also doesn't implement the challenge in the standard way)

Suggested alternative solution:

  • Create a custom reverse proxy and use query parameters for authentication.
  • Use a hosted provider or a custom relay that includes a standard challenge.

According to @quoid's external testing, it seems that the problem is not reproduced in the latest swift 5.8.1.
So maybe when the App is recompiled in the future of new Xcode version, the problem will be automatically resolved.

@peanball
Copy link
Author

Hi both, thank you for the thorough investigation. Adding the auth in the URL is by far not my favorite approach.

As @ACTCD says it’s not the primary auth mode for GitHub, which actually would be to set the Authorization: header. But Userscripts does not provide this capability.

If this is not resolved with Swift 5.8.1, or in general, would it be a consideration to add metadata support for bearer tokens?

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 28, 2023

@peanball This is not standard practice with user script, is uncommon, and causes other problems and possible confusion.

I personally recommend you to use a custom reverse proxy, it is easy, you can do all kinds of customization, and provides better security (you don't even need to expose any of your real GitHub tokens to users).

@peanball
Copy link
Author

I fully understand how I could make it work with alternative solutions. Of course I could set up a reverse proxy, put it into some S3 bucket, put all scripts on a public github.com repo, etc. The possibilities are endless.

This is not a support request but a feature request, if anything.

This issue is to assess, whether it could be possible to support hosting data from GitHub with personal access tokens as part of the URL. At the moment it is not. As you point out, GitHub does not behave how Apple would like HTTP to work.

The remaining question is: Do you want to support actual use cases people might be having, or rely on Apple that insists on the following cycle:

  • You have auth data, but don't send it unless asked.
  • HTTP 401 response, asking for auth data.
  • You now send the auth data.
  • HTTP 200 response with script data.

The question came up because many other tools (browsers, curl, violentmonkey and likely other user script managers) support the use case of "I have auth data, I send it right away" without issues.

I also understand Apple's point of view for suggesting not to mess with auth flow, as for 98% of cases it will be more reliable than app developers messing with HTTP headers they might not fully understand. I think we are in the 2% that do understand though.

As for exposing tokens: All users have regular access to these repos anyway. This is just a means of having Userscripts be able to read that same data without additional effort.

@peanball
Copy link
Author

peanball commented Jul 28, 2023

And it worked...🤔 I am not sure why it is failing when running the same exact function within the project.

@quoid, is it feasible that the URLSession.shared instance is behaving differently?

That said, in my brief experiment I didn't see any URLSessionConfiguration options that would force immediately sending basic auth information without auth challenge though. I don't have much experience with URLSession though.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 28, 2023

@peanball I guess Apple just followed the process by tradition and security considerations. But yes, maybe it should have provided a suggested solution earlier. I have no idea.

As additional information, I found that it seems that the auth information carried in the URL is always ignored in Safari, you are always asked to manually re-enter the username and password.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 29, 2023

I need to deny my previous guess that this issue might be fixed in swift 5.8.1.

According to the latest investigation, the swift 5.8.1 version has been used in the local Xcode.

In the external swift simulator, even if an earlier swift version is used, the problem cannot be reproduced.


In addition, I found that in the local App, it does not support Basic Authentication URL where :pass is completely omitted, but pass can be empty, that is, the colon cannot be omitted at the same time.

But in the external swift simulator, it is possible to omit the colon altogether. This is very strange, and I don't know why the two behaviors are so much different.


To make it easier to debug this issue, I made a new demo URL that completely mimics GitHub's behavior, returning a 404 directly when the token is incorrect, so that no one needs to test with their own GitHub account and token.

https://token@basic.deno.dev/DEBUG.BasicAuthentication.user.js
https://user:pass@basic.deno.dev/DEBUG.BasicAuthentication.user.js

UPDATE: Ok, now I know why the external swift emulator behaves differently, it seems that it actually uses curl internally, which explains why it behaves exactly like curl and not the Xcode compiled app.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 29, 2023

At this point, I believe all the mysteries have been solved.

In this project, basic authentication is currently dependent on Apple's implementation, and we don't want to make changes to that.

This issue is actually a request for support for a standalone feature for GitHub private repositories, which we are not ready for at the moment, but may consider in the future. Thank you for contributing to this and I will close this issue. Stay safe!

@ACTCD ACTCD closed this as completed Jul 29, 2023
@peanball
Copy link
Author

Understood and accepted. Thanks for the investigation.

@ACTCD
Copy link
Collaborator

ACTCD commented Jul 29, 2023

To describe and document the future vision for this feature, there is a new issue #521 for tracking.

@peanball If there are any ideas, please comment in the new issue, thanks.

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 a pull request may close this issue.

3 participants