Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

login: Malformed login URL arg breaks login, sh and sync #53

Closed
Russtopia opened this issue Jun 19, 2020 · 2 comments · Fixed by #54
Closed

login: Malformed login URL arg breaks login, sh and sync #53

Russtopia opened this issue Jun 19, 2020 · 2 comments · Fixed by #54
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@Russtopia
Copy link

As a naive user I tried

coder login localhost:8080
coder login master.cdr.dev

The browser auto-launch of the login URL is malformed in the above cases (and the text on terminal if autolaunch fails)

If I specify the full URL eg.
coder login http://localhost:8080

the URL generated is correct, and subsequent coder sh and coder sync ops work.

Recommend adding logic to prepend http:// if not specified by the user.

@Russtopia Russtopia added enhancement New feature or request good first issue Good for newcomers labels Jun 19, 2020
@Russtopia Russtopia self-assigned this Jun 19, 2020
@Russtopia Russtopia changed the title Add smarter login URI parsing login: Refuse missing protocol in URL Jun 19, 2020
@Russtopia Russtopia changed the title login: Refuse missing protocol in URL login: Malformed login URL arg breaks login, sh and sync Jun 19, 2020
@wbobeirne
Copy link

I think it'd be better to have them specify a full URI, since it could be https and they may not have redirects setup.

@Russtopia
Copy link
Author

I think it'd be better to have them specify a full URI, since it could be https and they may not have redirects setup.

Sorry, didn't see this until after I merged, but that's what I ended up doing. It doesn't try to fix up the incomplete URI -- it rejects and and the help/usage is updated to show an example. I can improve on it more if you think it should be more explicit.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants