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

Error views for CDIViewProvider don't know why it was loaded #98

Closed
vaadin-bot opened this issue Apr 8, 2014 · 6 comments
Closed

Error views for CDIViewProvider don't know why it was loaded #98

vaadin-bot opened this issue Apr 8, 2014 · 6 comments

Comments

@vaadin-bot
Copy link

Originally by @mstahv


Error view in cdi view provider is loaded when the view is not available, either at all or to e.g. due to a @RolesAllowed rule. For app developer it would be nice to know why.


Imported from https://dev.vaadin.com/ issue #13566

@vaadin-bot
Copy link
Author

Originally by viljoenp


I'll attach a small patch ( cdi-security.patch ) to show one way I think it can be handled without breaking backwards compatibility.

Personally I think Navigator should become an Interface in the long run though, that way a developer can implements his/her own security (and other) needs.

In addition to providing for custom needs , it can also provide a cleaner interface ( for example in the case of CDI you don't really want to add views to the navigator with addView(...)

summary of patch :

  1. throw a java.lang.SecurityException in CDIViewProvider.getView() if the user does not have access to the view.
  2. add SecurityListener to CDINavigator to handle Security events
  3. handle the java.lang.SecurityException in CDINavigator.navigateTo(), if no SecurityListeners are provided or none wants to handle the event the code will fallback to default behaviour (errorProvider or IllegalArgumentException if no errorProvider specified)
  4. Application uses CDINavigator instead of default Navigator

to handle security with a simple Notification:

        navigator.addSecurityListener(new SecurityListener() {

            @Override
            public boolean accessDenied(SecurityListener.SecurityEvent event) {
                Notification.show("Access Denied", event.getSecurityException().getMessage(), Notification.Type.ERROR_MESSAGE);
                // return true to say we handled the problem.
                return true;
            }
        });

@vaadin-bot
Copy link
Author

Originally by viljoenp


Attachment added: cdi-security.patch (20.4 KiB)
Enhancements to handle SecurityExceptions when access is denied via @RolesAllowed annotations

@vaadin-bot
Copy link
Author

Originally by viljoenp


The more I think about it the more I feel that the Navigator should change the way it handles failures. Even unknown views should be handled with an event rather than an ErrorViewPRovider.

A more generic NavigationFailure Listener where the event can have information about the failure , aka no view could be found, acccess denied to the view , or any unhandled exception during the navigation..

@vaadin-bot
Copy link
Author

Originally by @mstahv


Is something going to happen for this in any time soon?

@hesara
Copy link
Contributor

hesara commented Oct 10, 2017

We are currently not planning to add such API to the 1.x and 2.x branches of the Vaadin CDI add-on, but will rather look into how to handle this in Vaadin CDI 3.x.

What we can do for 1.x, though, is to open up the methods CDIViewProvider.getViewBean(String) and CDIViewProvider.parseViewName(String) to be protected. These changes should allow overriding the methods getViewName(String) and getView(String) and handling the access denied case in a custom subclass of CDIViewProvider in the application without having to copy the entire view provider implementation from the add-on.

Here is a simplified PoC of how a CustomCDIViewProvider could look like after the change, but the same change should also permit using an event based approach like presented in the earlier patch in this issue:

public class CustomCDIViewProvider extends CDIViewProvider {

    private View accessDeniedView;

    public View getAccessDeniedView() {
        return accessDeniedView;
    }

    public void setAccessDeniedView(View accessDeniedView) {
        this.accessDeniedView = accessDeniedView;
    }

    // ignore access check if an access denied view is defined - will be handled in getView() to support access denied view
    @Override
    public String getViewName(String viewAndParameters) {
        String name = parseViewName(viewAndParameters);
        ViewBean viewBean = getViewBean(name);

        if (viewBean == null) {
            return null;
        }

        if (isUserHavingAccessToView(viewBean) || getAccessDeniedView() != null) {
            if (viewBean.getBeanClass().isAnnotationPresent(CDIView.class)) {
                String specifiedViewName = Conventions
                        .deriveMappingForView(viewBean.getBeanClass());
                if (!specifiedViewName.isEmpty()) {
                    return specifiedViewName;
                }
            }
            return name;
        }
        return null;
    }

    @Override
    public View getView(String viewName) {
        ViewBean viewBean = getViewBean(viewName);
        if (viewBean != null && !isUserHavingAccessToView(viewBean)) {
            return getAccessDeniedView();
        }
        return super.getView(viewName);
    }

}

@tsuoanttila
Copy link
Contributor

Required APIs opened in 1.0.4, implementations for later versions in ticket #223

@tsuoanttila tsuoanttila added this to the Vaadin CDI 1.0.4 milestone Oct 13, 2017
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

No branches or pull requests

3 participants