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

[dashboard] redirect anonymous users to www. #3991

Closed
wants to merge 1 commit into from

Conversation

AlexTugarev
Copy link
Member

in SaaS context we want to redirect not logged in users from the root URL with no additional context provided to the website URL.

fixes #3898

in SaaS context we want to redirect not logged in users from the root URL with no additional context provided to the website URL.

fixes #3898
@@ -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

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:

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

@ghuntley
Copy link
Contributor

ghuntley commented Apr 26, 2021

/werft run

👍 started the job as gitpod-build-at-redirect-www.8

@@ -71,6 +71,12 @@ function App() {
}
}, [localStorage.theme]);

if (window.location.pathname === '/' && window.location.hash === '' && !hasLoggedInBefore()) {
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))) {

@svenefftinge
Copy link
Member

svenefftinge commented Apr 26, 2021

/werft run

👍 started the job as gitpod-build-at-redirect-www.9

@svenefftinge
Copy link
Member

continuing here #4070

@svenefftinge svenefftinge deleted the at/redirect-www branch April 26, 2021 13:57
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.

[dashboard] New and w/o cxt? Redirect to www.gitpod.io
4 participants