Skip to content

Commit

Permalink
Different approach to fix secured issue (#734)
Browse files Browse the repository at this point in the history
* Created BakeryUI with listener
* Added test for access denied view
  • Loading branch information
Tulio Garcia authored Feb 12, 2019
1 parent 8f8c9d7 commit a2b0739
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

import com.vaadin.flow.server.ServletHelper.RequestType;
import com.vaadin.flow.shared.ApplicationConstants;
import com.vaadin.starter.bakery.ui.views.errors.AccessDeniedView;
import com.vaadin.starter.bakery.ui.views.errors.CustomRouteNotFoundError;
import com.vaadin.starter.bakery.ui.views.login.LoginView;

/**
* SecurityUtils takes care of all such static operations that have to do with
Expand Down Expand Up @@ -50,13 +53,22 @@ public static String getUsername() {
* Checks if access is granted for the current user for the given secured view,
* defined by the view class.
*
* @param securedClass
* @param securedClass View class
* @return true if access is granted, false otherwise.
*/
public static boolean isAccessGranted(Class<?> securedClass) {
final boolean publicView = LoginView.class.equals(securedClass)
|| AccessDeniedView.class.equals(securedClass)
|| CustomRouteNotFoundError.class.equals(securedClass);

// Always allow access to public views
if (publicView) {
return true;
}

Authentication userAuthentication = SecurityContextHolder.getContext().getAuthentication();

// All views require authentication
// All other views require authentication
if (!isUserLoggedIn(userAuthentication)) {
return false;
}
Expand Down
27 changes: 27 additions & 0 deletions src/main/java/com/vaadin/starter/bakery/ui/BakeryUI.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.vaadin.starter.bakery.ui;

import com.vaadin.flow.component.UI;
import com.vaadin.flow.server.VaadinRequest;
import com.vaadin.starter.bakery.app.security.SecurityUtils;
import com.vaadin.starter.bakery.ui.components.OfflineBanner;
import com.vaadin.starter.bakery.ui.exceptions.AccessDeniedException;
import com.vaadin.starter.bakery.ui.views.login.LoginView;

public class BakeryUI extends UI {

protected void init(VaadinRequest request) {
add(new OfflineBanner());
addBeforeEnterListener(event -> {
final boolean accessGranted =
SecurityUtils.isAccessGranted(event.getNavigationTarget());
if (!accessGranted) {
if (SecurityUtils.isUserLoggedIn()) {
event.rerouteToError(AccessDeniedException.class);
}
else {
event.rerouteTo(LoginView.class);
}
}
});
}
}
35 changes: 12 additions & 23 deletions src/main/java/com/vaadin/starter/bakery/ui/MainView.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
package com.vaadin.starter.bakery.ui;

import static com.vaadin.starter.bakery.ui.utils.BakeryConst.PAGE_DASHBOARD;
import static com.vaadin.starter.bakery.ui.utils.BakeryConst.PAGE_PRODUCTS;
import static com.vaadin.starter.bakery.ui.utils.BakeryConst.PAGE_STOREFRONT;
import static com.vaadin.starter.bakery.ui.utils.BakeryConst.PAGE_USERS;
import static com.vaadin.starter.bakery.ui.utils.BakeryConst.TITLE_DASHBOARD;
import static com.vaadin.starter.bakery.ui.utils.BakeryConst.TITLE_LOGOUT;
import static com.vaadin.starter.bakery.ui.utils.BakeryConst.TITLE_PRODUCTS;
import static com.vaadin.starter.bakery.ui.utils.BakeryConst.TITLE_STOREFRONT;
import static com.vaadin.starter.bakery.ui.utils.BakeryConst.TITLE_USERS;
import static com.vaadin.starter.bakery.ui.utils.BakeryConst.VIEWPORT;

import com.vaadin.flow.component.HasElement;
import com.vaadin.flow.component.UI;
import com.vaadin.flow.component.applayout.AbstractAppRouterLayout;
Expand All @@ -10,19 +21,12 @@
import com.vaadin.flow.component.html.Span;
import com.vaadin.flow.component.icon.VaadinIcon;
import com.vaadin.flow.component.page.Viewport;
import com.vaadin.flow.router.BeforeEnterEvent;
import com.vaadin.flow.router.BeforeEnterObserver;
import com.vaadin.flow.server.PWA;
import com.vaadin.starter.bakery.app.security.SecurityUtils;
import com.vaadin.starter.bakery.ui.components.BakeryCookieConsent;
import com.vaadin.starter.bakery.ui.components.OfflineBanner;
import com.vaadin.starter.bakery.ui.exceptions.AccessDeniedException;
import com.vaadin.starter.bakery.ui.views.HasConfirmation;
import com.vaadin.starter.bakery.ui.views.admin.products.ProductsView;
import com.vaadin.starter.bakery.ui.views.admin.users.UsersView;
import com.vaadin.starter.bakery.ui.views.login.LoginView;

import static com.vaadin.starter.bakery.ui.utils.BakeryConst.*;


@Viewport(VIEWPORT)
Expand All @@ -31,7 +35,7 @@
backgroundColor = "#227aef", themeColor = "#227aef",
offlinePath = "offline-page.html",
offlineResources = {"images/offline-login-banner.jpg"})
public class MainView extends AbstractAppRouterLayout implements BeforeEnterObserver {
public class MainView extends AbstractAppRouterLayout {

private final ConfirmDialog confirmDialog;

Expand All @@ -43,7 +47,6 @@ public MainView() {

getElement().appendChild(confirmDialog.getElement());
getElement().appendChild(new BakeryCookieConsent().getElement());
getElement().addAttachListener(e -> UI.getCurrent().add(new OfflineBanner()));
}

@Override
Expand Down Expand Up @@ -78,20 +81,6 @@ private void setMenuItem(AppLayoutMenu menu, AppLayoutMenuItem menuItem) {
menu.addMenuItem(menuItem);
}

@Override
public void beforeEnter(BeforeEnterEvent event) {
final boolean accessGranted =
SecurityUtils.isAccessGranted(event.getNavigationTarget());
if (!accessGranted) {
if (SecurityUtils.isUserLoggedIn()) {
event.rerouteToError(AccessDeniedException.class);
}
else {
event.rerouteTo(LoginView.class);
}
}
}

@Override
public void showRouterLayoutContent(HasElement content) {
super.showRouterLayoutContent(content);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.vaadin.flow.router.HasErrorParameter;
import com.vaadin.flow.router.PageTitle;
import com.vaadin.flow.router.ParentLayout;
import com.vaadin.flow.router.Route;
import com.vaadin.flow.templatemodel.TemplateModel;
import com.vaadin.starter.bakery.ui.MainView;
import com.vaadin.starter.bakery.ui.exceptions.AccessDeniedException;
Expand All @@ -19,6 +20,7 @@
@HtmlImport("src/views/errors/access-denied-view.html")
@ParentLayout(MainView.class)
@PageTitle(BakeryConst.TITLE_ACCESS_DENIED)
@Route
public class AccessDeniedView extends PolymerTemplate<TemplateModel> implements HasErrorParameter<AccessDeniedException> {

@Override
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ server.compression.mime-types=application/json,application/xml,text/html,text/xm
security.basic.enabled=false
server.tomcat.uri-encoding=UTF-8
spring.jackson.serialization.write_dates_as_timestamps=false

server.servlet.context-parameters.UI=com.vaadin.starter.bakery.ui.BakeryUI
# Comment out if using anything else than H2 (e.g. MySQL or PostgreSQL)
spring.jpa.database-platform=org.hibernate.dialect.H2Dialect

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<link rel="import" href="../../../bower_components/polymer/polymer-element.html">
<link rel="import" href="../../../styles/shared-styles.html">

<dom-module id="access-denied-view">
<template>
Expand Down
40 changes: 32 additions & 8 deletions src/test/java/com/vaadin/starter/bakery/testbench/UsersViewIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,21 @@
import org.junit.Test;
import org.openqa.selenium.Keys;

import com.vaadin.flow.component.tabs.testbench.TabElement;
import com.vaadin.flow.component.textfield.testbench.PasswordFieldElement;
import com.vaadin.flow.component.textfield.testbench.TextFieldElement;
import com.vaadin.starter.bakery.testbench.elements.ui.StorefrontViewElement;
import com.vaadin.starter.bakery.testbench.elements.ui.UsersViewElement;
import com.vaadin.testbench.TestBenchElement;

public class UsersViewIT extends AbstractIT<UsersViewElement> {

private static Random r = new Random();

@Override
protected UsersViewElement openView() {
StorefrontViewElement storefront = openLoginView().login("[email protected]", "admin");
StorefrontViewElement storefront =
openLoginView().login("[email protected]", "admin");
return storefront.getMenu().navigateToUsers();
}

Expand All @@ -31,7 +34,8 @@ public void updatePassword() {

String uniqueEmail = "e" + r.nextInt() + "@vaadin.com";

createUser(usersView, uniqueEmail, "Paul", "Irwin", "Vaadin10", "baker");
createUser(
usersView, uniqueEmail, "Paul", "Irwin", "Vaadin10", "baker");

int rowNum = usersView.getGrid().getCell(uniqueEmail).getRow();
usersView.openRowForEditing(rowNum);
Expand Down Expand Up @@ -83,8 +87,9 @@ public void updatePassword() {
Assert.assertEquals("", password.getAttribute("value"));
}

private void createUser(UsersViewElement usersView, String email, String firstName, String lastName,
String password, String role) {
private void createUser(
UsersViewElement usersView, String email, String firstName,
String lastName, String password, String role) {
usersView.getSearchBar().getCreateNewButton().click();
Assert.assertTrue(usersView.isEditorOpen());

Expand Down Expand Up @@ -115,7 +120,8 @@ public void tryToUpdateLockedEntity() {
page.getEmailField().setValue("[email protected]");
page.getEditorSaveButton().click();

Assert.assertEquals(rowNum, page.getGrid().getCell("[email protected]").getRow());
Assert.assertEquals(
rowNum, page.getGrid().getCell("[email protected]").getRow());
}

@Test
Expand All @@ -130,9 +136,10 @@ public void tryToDeleteLockedEntity() {
page.getEditorDeleteButton().click();
page.getDeleteConfirmDialog().getConfirmButton().click();

Assert.assertEquals(rowNum, page.getGrid().getCell("[email protected]").getRow());
Assert.assertEquals(
rowNum, page.getGrid().getCell("[email protected]").getRow());
}

@Test
public void testCancelConfirmationMessage() {
UsersViewElement page = openView();
Expand All @@ -141,6 +148,23 @@ public void testCancelConfirmationMessage() {
page.getFirstName().sendKeys("Some name");
page.getEditorCancelButton().click();

Assert.assertThat(page.getDiscardConfirmDialog().getMessageText(), containsString("Discard changes"));
Assert.assertThat(
page.getDiscardConfirmDialog().getMessageText(),
containsString("Discard changes"));
}

@Test
public void accessDenied() {
StorefrontViewElement storefront =
openLoginView().login("[email protected]", "barista");
Assert.assertEquals(
3, storefront.getMenu().$(TabElement.class).all().size());

driver.get(APP_URL + "users");
TestBenchElement accessDeniedPage =
$("access-denied-view").waitForFirst();

Assert.assertEquals("Access denied",
accessDeniedPage.$("h3").first().getText());
}
}

0 comments on commit a2b0739

Please sign in to comment.