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

Add simple form based login page to UI #2755

Open
wants to merge 1 commit into
base: master
from
Open

Conversation

@dain
Copy link
Member

dain commented Feb 7, 2020

No description provided.

@dain dain requested a review from electrum Feb 7, 2020
@cla-bot cla-bot bot added the cla-signed label Feb 7, 2020
@dain dain force-pushed the dain:ui-login branch 2 times, most recently from 3c2c3ca to 62709dd Feb 8, 2020
@dain dain mentioned this pull request Feb 8, 2020
@dain dain force-pushed the dain:ui-login branch from 62709dd to bcbb146 Feb 9, 2020
@dain dain force-pushed the dain:ui-login branch from bcbb146 to 2beb90c Feb 9, 2020
@@ -302,6 +302,16 @@
<artifactId>jjwt</artifactId>
</dependency>

<dependency>

This comment has been minimized.

Copy link
@electrum

electrum Feb 13, 2020

Member

Make these <scope>runtime</scope> and add a section header:

<!-- used by tests but also needed transitively -->
long sessionTimeoutNanos = config.getSessionTimeout().roundTo(NANOSECONDS);
this.jwtGenerator = username -> generateJwt(hmac, username, sessionTimeoutNanos);

this.passwordAuthenticatorManager = passwordAuthenticatorManager;

This comment has been minimized.

Copy link
@electrum

electrum Feb 13, 2020

Member

Check for null

return Optional.empty();
}

if (!isHttps(request)) {

This comment has been minimized.

Copy link
@electrum

electrum Feb 13, 2020

Member

We might want to fail here if a password was provided


private static Optional<Cookie> getAuthenticationCookie(HttpServletRequest request)
{
for (Cookie cookie : firstNonNull(request.getCookies(), new Cookie[0])) {

This comment has been minimized.

Copy link
@electrum

electrum Feb 13, 2020

Member

This could be

return stream(firstNonNull(request.getCookies(), new Cookie[0]))
        .filter(cookie -> cookie.getName().equals(PRESTO_UI_COOKIE))
        .findFirst();
private static String getRedirectLocation(HttpServletRequest request, String path, String queryParameter)
{
String proto = firstNonNull(emptyToNull(request.getHeader(X_FORWARDED_PROTO)), request.getScheme());
HttpUriBuilder builder = uriBuilder()

This comment has been minimized.

Copy link
@electrum

electrum Feb 13, 2020

Member

Use the JAX-RS URI builder

</div>
<input id="redirectPath" name="redirectPath" type="hidden">
<div class="form-group">
<button id="submit" type="submit" class="btn btn-primary btn-block">Log in</button>

This comment has been minimized.

Copy link
@electrum

electrum Feb 13, 2020

Member

Nit: single space

<li>
<span className="navbar-cluster-info">
<span className="text" id="logout">
<a className="btn btn-logout" href="logout">Logout</a>

This comment has been minimized.

Copy link
@electrum

electrum Feb 13, 2020

Member

Log Out


Response response = assertOk(client, getLoginHtmlLocation(baseUri));
String body = response.body().string();
assertTrue(body.contains("action=\"/ui/login"));

This comment has been minimized.

Copy link
@electrum

electrum Feb 13, 2020

Member
assertThat(body).contains("action=\"/ui/login\"");
assertTrue(body.contains("method=\"post\""));

logIn(baseUri, client);
HttpCookie cookie = Iterables.getOnlyElement(cookieManager.getCookieStore().getCookies());

This comment has been minimized.

Copy link
@electrum

electrum Feb 13, 2020

Member

Static import

assertResponseCode(client, getLocation(baseUri, "/ui/api/unknown"), SC_NOT_FOUND);

assertRedirect(client, getLogoutLocation(baseUri), getLoginHtmlLocation(baseUri));
assertTrue(cookieManager.getCookieStore().getCookies().isEmpty());

This comment has been minimized.

Copy link
@electrum

electrum Feb 13, 2020

Member
assertThat(cookieManager.getCookieStore().getCookies()).isEmpty();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.