ui: style login page #26066

couchand · 2018-05-24T21:35:42Z

Adds some initial styling to the login page and the login indicator in the navigation sidebar.

Sidebar:

Login page:

Login page with error:

Insecure mode (temporary design):

Closes #24939.

cockroach-teamcity · 2018-05-24T21:35:50Z

This change is

couchand · 2018-05-29T18:04:13Z

cc @josueeee for design review

vilterp · 2018-05-29T18:38:35Z

with a couple non-blocking nits. Tested locally.

Would be good to disable the button while the login api call is happening, and change the text to "signing in..." or something.

Spacing in the text box seems a bit excessive:

Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

pkg/ui/src/views/login/loginPage.tsx, line 67 at r3 (raw file):

    let message = "Invalid username or password.";
    if (error.message !== "Unauthorized") {

Can you add a comment noting why have to look at the response body (generated somewhere in the depths of grpc-gateway or net/http or somewhere), and not the status code?

I'm ok with doing this for now, but we should leave clues for our future selves to fix this.

Comments from Reviewable

vilterp · 2018-05-29T20:36:24Z

Also, what does the sidebar look like in insecure mode?

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

Comments from Reviewable

couchand · 2018-06-04T16:44:40Z

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

pkg/ui/src/views/login/loginPage.tsx, line 67 at r3 (raw file):

Previously, vilterp (Pete Vilter) wrote…

Can you add a comment noting why have to look at the response body (generated somewhere in the depths of grpc-gateway or net/http or somewhere), and not the status code?

I'm ok with doing this for now, but we should leave clues for our future selves to fix this.

It's just here:

cockroach/pkg/ui/src/util/api.ts

Line 160 in 2137b24

throw Error(res.statusText);

But now that I've made a bunch of these hacks all over maybe it's time I actually fix it.

Comments from Reviewable

couchand · 2018-06-04T21:25:05Z

Ready to go @vilterp PTAL

vilterp

Don't see insecure indicator in insecure mode. Otherwise looks good.

vilterp · 2018-06-05T20:22:26Z

pkg/ui/src/views/app/components/layoutSidebar/index.tsx

@@ -70,7 +71,13 @@ function LoginIndicator({ loginState, handleLogout }: LoginIndicatorProps) {
  }

  if (!loginState.loginEnabled()) {
-    return (<div className="login-indicator">Insecure mode</div>);
+    return (


I don't see this indicator when I start a cluster in insecure mode. Just blank space above the cockroach icon.

Are you sure you had the environment variable set? I am seeing it...

vilterp · 2018-06-05T20:31:07Z

pkg/server/server.go

@@ -1928,6 +1928,7 @@ func serveUIAssets(fileServer http.Handler, cfg Config) http.Handler {
 		tmplArgs := ui.IndexHTMLArgs{
 			ExperimentalUseLogin: cfg.EnableWebSessionAuthentication,
 			LoginEnabled:         cfg.RequireWebSession(),
+			Tag:                  build.GetInfo().Tag,


looks like this is what broke the ServeIndexHTML test in CI. It's pretty brittle — asserts the exact HTML that comes out. Not sure how to make that always pass, since the build tag will keep changing… Maybe can just sprintf it into the expected HTML.

Tests updated.

vilterp · 2018-06-05T20:33:03Z

pkg/ui/src/routes/login.tsx

 import LoginPage from "src/views/login/loginPage";

 export const LOGIN_PAGE = "/login";
+export const LOGOUT_PAGE = "/logout";
+
+export default function(store: Store<AdminUIState>): JSX.Element {


nit: I don't like exporting anonymous functions; prefer to give them names (I think you can still export them in the same statement; maybe need an export statement below). Always giving functions names makes them easier to grep for and find via stack traces (as if we ever have reasonable stack traces… 😢 but that's a separate issue)

Good point. Done.

vilterp · 2018-06-05T20:34:02Z

pkg/ui/src/views/app/components/layoutSidebar/index.tsx

+        >
+          { user[0] }
+        </div>
+        <Link to="/logout" onClick={handleLogout}>Sign Out</Link>


was gonna suggest that you make the whole thing clickable, but maybe the log out button should be pretty small so you don't click it accidentally.

Yep. Maybe one day we'll have a user menu of some kind?!

Release note: None

Add some initial styling to the login page. Contributes to cockroachdb#24939. Release note: None

Add some styling and clean up the error text on login page. Contributes to cockroachdb#24939. Release note: None

Release note: None

Adds some initial styling to the login indicator on the navigation sidebar. Release note: None

An explicit logout route can be useful, so this adds one. Release note: None

Some TypeScript errors slipped through due to cockroachdb#26255. This fixes them. Release note: None

This will get shown on the login page. Release note: None

This updates the login page styling to the latest designs. Release note: None

This is temporary styling for the insecure mode indicator. A final design will be implemented later, but this will hold us over for now. Release note: None

Release note: None

vilterp · 2018-06-07T18:02:19Z

Ah, started in insecure mode with the flag and I see the indicator now. LGTM!

couchand · 2018-06-07T18:02:19Z

bors r+

26066: ui: style login page r=couchand a=couchand Adds some initial styling to the login page and the login indicator in the navigation sidebar. Sidebar: <img width="710" alt="screen shot 2018-05-29 at 1 59 44 pm" src="https://user-images.githubusercontent.com/793969/40676560-110e3a7e-6349-11e8-812a-9c8e682f5670.png"> Login page: <img width="1274" alt="screen shot 2018-06-04 at 4 28 40 pm" src="https://user-images.githubusercontent.com/793969/40940739-c0c6eb10-6816-11e8-9b24-8a0b492239b3.png"> Login page with error: <img width="1277" alt="screen shot 2018-06-04 at 4 45 40 pm" src="https://user-images.githubusercontent.com/793969/40940744-c4cb2d66-6816-11e8-90d4-d8b3c52927b6.png"> Insecure mode (temporary design): <img width="161" alt="screen shot 2018-06-04 at 5 10 56 pm" src="https://user-images.githubusercontent.com/793969/40942388-016f9478-681c-11e8-929c-36d0b7aa7463.png"> Closes #24939. Co-authored-by: Andrew Couch <[email protected]>

vilterp · 2018-06-07T18:05:37Z

pkg/ui/src/views/login/loginPage.tsx

+              account access and password restoration.
+            </p>
+            <p className="aside">
+              <a href={docsURL("admin-ui-overview.html")} className="docs-link">


maybe change to https://www.cockroachlabs.com/docs/dev/admin-ui-access-and-navigate.html#secure-the-admin-ui in a followup?

@Amruta-Ranade will this link change?

craig · 2018-06-07T18:22:30Z

Build succeeded

GitHub CI (Cockroach)

couchand added the do-not-merge bors won't merge a PR with this label. label May 24, 2018

couchand requested a review from a team May 24, 2018 21:35

couchand force-pushed the feature/login-styling branch from 0e1c6a0 to 893f64d Compare May 29, 2018 18:01

couchand changed the title ~~[WIP] ui: style login page~~ ui: style login page May 29, 2018

couchand removed the do-not-merge bors won't merge a PR with this label. label May 29, 2018

couchand mentioned this pull request May 29, 2018

Web UI Authn & Authz (Tracking Issue) #25529

Closed

couchand force-pushed the feature/login-styling branch from 893f64d to 522ed74 Compare June 4, 2018 20:44

couchand requested review from a team June 4, 2018 20:44

couchand added the do-not-merge bors won't merge a PR with this label. label Jun 4, 2018

couchand force-pushed the feature/login-styling branch from 522ed74 to d43824c Compare June 4, 2018 21:21

couchand removed the do-not-merge bors won't merge a PR with this label. label Jun 4, 2018

vilterp approved these changes Jun 5, 2018

View reviewed changes

couchand added 11 commits June 7, 2018 11:04

ui: declosurify event handlers on login page

46c1b4f

Release note: None

ui: style login page

a9f74ae

Add some initial styling to the login page. Contributes to cockroachdb#24939. Release note: None

ui: error state for login page

b966f7c

Add some styling and clean up the error text on login page. Contributes to cockroachdb#24939. Release note: None

ui: add cockroach icon to login page

79b9dc4

Release note: None

ui: style navbar login indicator

4eaa236

Adds some initial styling to the login indicator on the navigation sidebar. Release note: None

ui: add logout route

9f6f519

An explicit logout route can be useful, so this adds one. Release note: None

ui: fix ts errors from previous changes

f1ef286

Some TypeScript errors slipped through due to cockroachdb#26255. This fixes them. Release note: None

ui, server: pass build tag to ui

8d20abc

This will get shown on the login page. Release note: None

ui: update styling of login page

31f60eb

This updates the login page styling to the latest designs. Release note: None

ui: style insecure mode indicator

6d064e0

This is temporary styling for the insecure mode indicator. A final design will be implemented later, but this will hold us over for now. Release note: None

ui: change copy on sign in button while waiting

ee1a743

Release note: None

craig bot requested review from a team June 7, 2018 16:06

craig bot requested a review from a team as a code owner June 7, 2018 16:06

craig bot requested review from a team June 7, 2018 16:06

couchand force-pushed the feature/login-styling branch from d43824c to de8d6f1 Compare June 7, 2018 16:06

couchand removed request for a team June 7, 2018 16:07

vilterp reviewed Jun 7, 2018

View reviewed changes

craig bot merged commit de8d6f1 into cockroachdb:master Jun 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ui: style login page #26066

ui: style login page #26066

couchand commented May 24, 2018 •

edited

Loading

cockroach-teamcity commented May 24, 2018

couchand commented May 29, 2018

vilterp commented May 29, 2018

vilterp commented May 29, 2018

couchand commented Jun 4, 2018

couchand commented Jun 4, 2018

vilterp left a comment

vilterp Jun 5, 2018

couchand Jun 7, 2018

vilterp Jun 5, 2018

couchand Jun 7, 2018

vilterp Jun 5, 2018

couchand Jun 7, 2018

vilterp Jun 5, 2018

couchand Jun 7, 2018

vilterp commented Jun 7, 2018

couchand commented Jun 7, 2018

vilterp Jun 7, 2018

vilterp Jun 7, 2018

craig bot commented Jun 7, 2018

ui: style login page #26066

ui: style login page #26066

Conversation

couchand commented May 24, 2018 • edited Loading

cockroach-teamcity commented May 24, 2018

couchand commented May 29, 2018

vilterp commented May 29, 2018

vilterp commented May 29, 2018

couchand commented Jun 4, 2018

couchand commented Jun 4, 2018

vilterp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vilterp commented Jun 7, 2018

couchand commented Jun 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

craig bot commented Jun 7, 2018

Build succeeded

couchand commented May 24, 2018 •

edited

Loading