[
https://issues.apache.org/jira/browse/DELTASPIKE-749?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14194736#comment-14194736
]
Ron Smeral commented on DELTASPIKE-749:
---------------------------------------
#1 Agreed, the AuthenticationListener should have e.g. window/session scope.
#2 The way I see it seems to be in contradiction to your point #2: IMHO,
AccessDecisionVoters should really do just that one thing which is voting and
nothing else, any other behaviour added to a voter makes the design dirty.
#3 AuthenticationListener is a custom view-specific component responsible for
handling login cases (success/fail/whatever). The exception handling is just an
added behaviour.
#4 You're right, it would be cleaner to observe some LoginFailedEvent instead
of handling the exception.
#5 Documenting both seems like an overkill to me, for such a simple thing which
most app developers will probably implement in a way most appropriate for their
application.
I also think including both, a generic CDI version and a PicketLink version is
too much, I suggest keeping just the PL version and adding a note, that it's
easy to implement this for any other framework by handling appropriate events
and making appropriate authorization check calls in the voter.
I suggest keeping the combination of #1, #2, #4:
A plain voter:
- the one in the PL example in the doc is wrong, since it's called
AdminAccessDecisionVoter, but only calls {{isLoggedIn()}}
- also, why use BeanProvider in the method body instead of an injection point,
when the bean is an injection target?
{code:java|title=AdminAccessDecisionVoter.java}
@SessionScoped //or @WindowScoped
public class AdminAccessDecisionVoter extends AbstractAccessDecisionVoter {
@Inject
RelationshipManager relMgr;
@Inject
Identity id;
@Inject
IdentityManager idm;
@Override
protected void checkPermission(AccessDecisionVoterContext context,
Set<SecurityViolation> violations) {
if(!BasicModel.hasRole(relMgr, id.getAccount(), BasicModel.getRole(idm,
"admin"))) {
violations.add(/*...*/);
}
}
}
{code}
And a view-specific observer like this:
{code:java|title=AuthenticationListener.java}
@SessionScoped
public class AuthenticationListener {
@Inject ViewNavigationHandler viewNavigationHandler;
@Inject ViewConfigResolver viewConfigResolver;
private Class<? extends ViewConfig> deniedPage;
public void rememberDeniedView(@Observes LoginFailedEvent evt) {
deniedPage =
viewConfigResolver.getViewConfigDescriptor(FacesContext.getCurrentInstance().getViewRoot().getViewId()).getConfigClass();
evt.handledAndContinue();
}
public void handleLoggedIn(@Observes LoggedInEvent event) {
if(deniedPage != null) {
viewNavigationHandler.navigateTo(deniedPage);
deniedPage = null;
}
viewNavigationHandler.navigateTo(Pages.Home.class);
}
}
{code}
> Doc: Security: Making intitially requested and secured page available for
> redirect after login
> ----------------------------------------------------------------------------------------------
>
> Key: DELTASPIKE-749
> URL: https://issues.apache.org/jira/browse/DELTASPIKE-749
> Project: DeltaSpike
> Issue Type: Bug
> Components: Documentation
> Reporter: Ron Smeral
> Priority: Minor
>
> http://deltaspike.apache.org/documentation/security.html#_making_intitially_requested_and_secured_page_available_for_redirect_after_login
> In _CDI Implementation to redirect the login to the first denied page_:
> * change Usuario to User
> * why use {{char[]}} for password? Is that some security measure, to prevent
> interned Strings of passwords hanging around in memory? If so, that should be
> noted, otherwise it should be changed to String, it's confusing.
> In CDI and PL implementations:
> * -the AdminAccessDecisionVoter should implement AccessDecisionVoter, not
> extend AbstractAccessDecisionVoter-
> * I think the {{AdminAccessDecisionVoter}} should be agnostic of the view
> layer and therefore shouldn't inject {{ViewConfigResolver}} and shouldn't
> keep the denied page itself.
> Maybe the listener could handle the {{AccessDeniedException}} instead:
> Basic voter:
> {code:java|title=AdminAccessDecisionVoter.java}
> @SessionScoped //or @WindowScoped
> public class AdminAccessDecisionVoter extends AbstractAccessDecisionVoter {
> @Override
> protected void checkPermission(AccessDecisionVoterContext context,
> Set<SecurityViolation> violations) {
> // voting stuff
> }
> }
> {code}
> The listener/holder/handler:
> {code:java|title=AuthenticationListener.java}
> @ExceptionHandler
> public class AuthenticationListener {
> @Inject ViewNavigationHandler viewNavigationHandler;
> @Inject ViewConfigResolver viewConfigResolver;
> private Class<? extends ViewConfig> deniedPage;
> public void rememberDeniedView(@BeforeHandles
> ExceptionEvent<ErrorViewAwareAccessDeniedException> evt) {
> deniedPage =
> viewConfigResolver.getViewConfigDescriptor(FacesContext.getCurrentInstance().getViewRoot().getViewId()).getConfigClass();
> evt.handledAndContinue();
> }
> public void handleLoggedIn(@Observes UserLoggedInEvent event) {
> if(deniedPage != null) {
> viewNavigationHandler.navigateTo(deniedPage);
> deniedPage = null;
> }
> viewNavigationHandler.navigateTo(Pages.Home.class);
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
