Skip to content

Basic auth flow #26

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

Closed
wants to merge 29 commits into from
Closed

Basic auth flow #26

wants to merge 29 commits into from

Conversation

msujew
Copy link

@msujew msujew commented Aug 11, 2021

Implements a basic auth flow. Shows a login page when the server has been started with --password=<password>. Inserting the correct password redirects to the application, while a wrong password brings the user back to the login.

@msujew msujew force-pushed the msujew/basic-auth branch from 343f447 to 61bc66c Compare August 16, 2021 12:33
@msujew msujew marked this pull request as ready for review August 17, 2021 08:30
@akosyakov
Copy link
Member

One more thing basic oauth should not be contributed to server.main.ts, only via server.ts. We are not going to use it in Gitpod and should not have unnecesray deps then.

@msujew msujew force-pushed the msujew/basic-auth branch from 61bc66c to 3c22b7d Compare August 23, 2021 12:20
@msujew
Copy link
Author

msujew commented Aug 23, 2021

@akosyakov Auth is now separate from the main implementation. Care to take another look? Also removed all additional dependencies.

@msujew msujew force-pushed the msujew/basic-auth branch from bc5c75e to 69807bc Compare August 23, 2021 14:38
@akosyakov
Copy link
Member

Can we revert to minimal change? I cannot really understand what was changed in server.main.ts.

The PR should be:

  • introduce a needed callback in server.main.ts
  • implement rest in server.ts
  • add login page

There is no need for unnecessary refactoring and so on.

@msujew msujew force-pushed the msujew/basic-auth branch 2 times, most recently from f0ea2d6 to 9c9fcf1 Compare August 31, 2021 12:26
@spoenemann spoenemann requested a review from akosyakov September 1, 2021 13:44
@akosyakov
Copy link
Member

Web sockets are not secured.

@akosyakov
Copy link
Member

@msujew Could you rebase please? Some minor comments:

  • for login screen can we center it, i.e like on google.com and add a comment about the password in server terminal?
  • there is no way to fix pop up on first reload about the form fields, do you know the reason?

@msujew
Copy link
Author

msujew commented Sep 6, 2021

there is no way to fix pop up on first reload about the form fields, do you know the reason?

Well, the alternative would be the redirect. I don't think there's a way around that, as that's something implemented by the browser, not by us.

@akosyakov
Copy link
Member

akosyakov commented Sep 10, 2021

We discussed it and decided to put it on hold, try without built-in auth for now. Please don't feel discouraged, contribution is good!

@akosyakov akosyakov changed the base branch from web-server to main September 21, 2021 07:23
@akosyakov akosyakov changed the base branch from main to web-server September 21, 2021 11:26
@akosyakov akosyakov force-pushed the main branch 3 times, most recently from acafaa0 to 539e157 Compare September 23, 2021 06:47
@rob-64
Copy link

rob-64 commented Oct 3, 2021

I was able to accomplish this with a Caddy reverse proxy in front and basic auth there.

@akosyakov
Copy link
Member

I'm closing it. The goal of the project just to provide easy to maintain open source server. Integration issues should be resolved by adopters or end users.

That said, it does not mean that there could not be guides showing how to securely setup the server. Anyone is welcomed to contribute them.

@akosyakov akosyakov closed this Oct 4, 2021
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.