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.

Sign up for GitHub

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

[dashboard] redirect anonymous users to www. #3991

Closed
wants to merge 1 commit into from
Closed

[dashboard] redirect anonymous users to www. #3991

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion components/dashboard/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Menu from './components/Menu';
import { BrowserRouter } from "react-router-dom";
import { Redirect, Route, Switch } from "react-router";

import { Login } from './Login';
import { Login, hasLoggedInBefore } from './Login';
import { UserContext } from './user-context';
import { getGitpodService } from './service/service';
import { shouldSeeWhatsNew, WhatsNew } from './WhatsNew';
Expand Down Expand Up @@ -71,6 +71,12 @@ function App() {
}
}, [localStorage.theme]);

if (window.location.pathname === '/' && window.location.hash === '' && !hasLoggedInBefore()) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove the hasLoggedInBefore so that you are landing on the website when you've logged out.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just learned about the markLoggedIn which sets a HTTP cookie gitpod-user=loggedIn.

But we already have a session cookie gitpod-user=loggedIn, which is managed on login/logout.

Copy link
Member Author

Choose a reason for hiding this comment

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

hasLoggedInBefore checks for a Cookie which is never removed, gitpod-user=loggedIn used to reflect the session state.

WDYT of renaming the new Cookie, so it reflects that particular state? maybe something like "gitpod-logged-in-before". Maybe we can even set the provider host as value ;-)

Copy link
Member

Choose a reason for hiding this comment

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

The cookie should be called 'seen-gitpod' or so, as it just indicates the user knows gitpod (has seen the website or the app). Anyhow, we don't want to use it here, as the login state is enough. We really should redirect everyone to www once they are logged out.

Copy link
Member Author

@AlexTugarev AlexTugarev Apr 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Anyhow, we don't want to use it here, as the login state is enough. We really should redirect everyone to www once they are logged out.

Roger that. The thing is, that the "login state" is fetched asynchronously. So, removing of the gitpod-user=loggedIn (session) cookie check, would mean that the redirect might happen in any case.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is what used to work before:

gitpod/components/dashboard/src/components/create/index.tsx

Line 56 in 24f7b60

if (!document.cookie.match("gitpod-user=loggedIn")) {

if (window.location.host === "gitpod.io" || window.location.host === "gitpod-staging.com") {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: in development the URL is https://at-redirect-www.staging.gitpod-dev.com which won't match this logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

true. in the dashboard component we still don't have a proper solution for the URL mismatch in preview environments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe something like this?

Suggested change
if (window.location.host === "gitpod.io" || window.location.host === "gitpod-staging.com") {
if (["gitpod.io", "gitpod-staging.com", "gitpod-dev.com", "gitpod-io-dev.com"].some(suffix => window.location.host.endsWith(suffix))) {

window.location.href = `https://www.${window.location.host}`;
}
}

if (loading) {
return <Loading />
}
Expand Down