Add GET authentication #2428
Add GET authentication #2428
Conversation
There was a problem hiding this comment.
This looks great!
Maybe we should add the ability to generate keys/tokens? Then you could give out tokens to people which would let you revoke them without having to change your password.
For example we could have something like:
$ code-server --generate-token
<token here>
And it would insert into a tokens array in the config.yaml.
Maybe out of scope for this PR though.
| @@ -147,6 +148,10 @@ const options: Options<Required<Args>> = { | |||
| port: { type: "number", description: "" }, | |||
|
|
|||
| socket: { type: "string", path: true, description: "Path to a socket (bind-addr will be ignored)." }, | |||
| "enable-get-requests": { | |||
There was a problem hiding this comment.
What do you think about naming this enable-get-auth or something with auth in the name?
There was a problem hiding this comment.
If we went the token route we could call it enable-token-auth or something.
There was a problem hiding this comment.
That seems quick and concise! I might try that. :) Yeah, I agree that tokens seem to be safer.
| @@ -125,6 +125,10 @@ const main = async (args: DefaultedArgs): Promise<void> => { | |||
| logger.info(` - Authentication is disabled ${args.link ? "(disabled by --link)" : ""}`) | |||
| } | |||
|
|
|||
| if (args["enable-get-requests"]) { | |||
| logger.info(` - Login via GET is enabled ${args.auth === AuthType.None ? "(however auth is disabled)" : ""}`) | |||
There was a problem hiding this comment.
Should we just skip this output entirely if auth is disabled?
There was a problem hiding this comment.
I thought that it might seem intrusive to people who are just using normal POST passwords.
Co-authored-by: Asher <ash@coder.com>
Co-authored-by: Asher <ash@coder.com>
|
Should GET passwords be completely gone if tokens are added? |
|
@JammSpread I feel absolutely terrible for doing this to you after you've written all of this (really great!) code. 😭 We had an internal discussion about whether we want to support additional authentication methods (such as GET tokens or GET passwords) and while we think it's a cool idea ultimately we came to the conclusion that these are a little out of code-server's scope and it would be more secure to rely on external authentication mechanisms. For example SSH or something like https://github.com/oauth2-proxy/oauth2-proxy. I'm sorry I didn't check whether we wanted to integrate this feature before reviewing! Please let me know if you have any questions/concerns/counterpoints. |
|
Oh that's fine. If that's what might be best for |
|
This patch would make it possible to call Perhaps you might want to rethink this PR once more 🤔 |
|
Upstream does have token-based authentication now so I think this could be done. We might need to add a new flag but otherwise there would be no overhead on our end. |
|
Cool. That would be fantastic! It would be great, if In the case of an integration into JupyterLab's launchpad the request for a token would then be added HERE. And this token would be passed to the code-server's launchpad-button-link HERE.
|
|
The way upstream does it is that you can pass I just realized there would be overhead though because code-server has its own routes so we would need to update our authentication to support the token. Unfortunate but I think since upstream supports token-based authentication that is a strong reason for us to support it as well. |
|
Do you have any idea what priority this token-based authentication has for |
|
Priority for implementation? I will add it to the backlog but I
have no idea if/when we will get to it.
|
Adds config option
enable-get-requeststo enable authentication in the URL.(i.e. logging in on someone going to a link like, 192.168.1.49:8080/login?pass=SamplePassword)
This pr adds both the password and pass queries with the password query taking higher precedence in the URL.
This may have functionality when sharing a workspace with someone trusted.
Issues that had mentioned this ability: