ICEfaces
  1. ICEfaces
  2. ICE-5151

Check isUserInRole on initial request

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Invalid
    • Affects Version/s: 1.8.1
    • Fix Version/s: 1.8.2-EE-Beta, 1.8.2-EE-GA
    • Component/s: None
    • Labels:
      None
    • Environment:
      -

      Description

      In certain cases, the user's role is not initialized until after the first request. The fix is to check isUserInRole on the initial request in ServletEnvironmentRequest.java:

      public boolean isUserInRole(String role) {
          
           if (initialRequest.isUserInRole(role)) {
               return true;
              }
              
              return authorization.isUserInRole(role);
          }

      There are 2 forums posts regarding this issue:

      http://www.icefaces.org/JForum/posts/list/13040.page
      http://www.icefaces.org/JForum/posts/list/12530.page

        Activity

        Hide
        Ted Goddard added a comment -

        The suggested fix is not viable in the ICEfaces 1.8 code: initialRequest is a temporary variable only. This avoids requests being referenced outside of their normal lifecycle (such as during push renders).

        An alternative suggested fix is to ensure that the Authorization implementation has the request available to it (provided in the following comment).

        Show
        Ted Goddard added a comment - The suggested fix is not viable in the ICEfaces 1.8 code: initialRequest is a temporary variable only. This avoids requests being referenced outside of their normal lifecycle (such as during push renders). An alternative suggested fix is to ensure that the Authorization implementation has the request available to it (provided in the following comment).
        Hide
        Ted Goddard added a comment -

        — src/com/icesoft/faces/webapp/http/servlet/SessionDispatcher.java (revision 19721)
        +++ src/com/icesoft/faces/webapp/http/servlet/SessionDispatcher.java (working copy)
        @@ -58,7 +58,7 @@
        try

        { CurrentSessionBoundServer.attach(lookupServer(session)); //put the request in the pool of active request in case HttpServletRequest.isUserInRole need to be called - addRequest(request); + addRequest(session, request); super.service(request, response); }

        finally

        { //remove the request from the active requests pool @@ -119,8 +119,8 @@ sessionBoundServers.remove(session.getId()); }
        • private void addRequest(HttpServletRequest request) {
        • String key = request.getRequestedSessionId();
          + private void addRequest(HttpSession session, HttpServletRequest request) {
          + String key = session.getId();
          synchronized (activeRequests) {
          if (activeRequests.containsKey(key)) {
          List requests = (List) activeRequests.get(key);
        Show
        Ted Goddard added a comment - — src/com/icesoft/faces/webapp/http/servlet/SessionDispatcher.java (revision 19721) +++ src/com/icesoft/faces/webapp/http/servlet/SessionDispatcher.java (working copy) @@ -58,7 +58,7 @@ try { CurrentSessionBoundServer.attach(lookupServer(session)); //put the request in the pool of active request in case HttpServletRequest.isUserInRole need to be called - addRequest(request); + addRequest(session, request); super.service(request, response); } finally { //remove the request from the active requests pool @@ -119,8 +119,8 @@ sessionBoundServers.remove(session.getId()); } private void addRequest(HttpServletRequest request) { String key = request.getRequestedSessionId(); + private void addRequest(HttpSession session, HttpServletRequest request) { + String key = session.getId(); synchronized (activeRequests) { if (activeRequests.containsKey(key)) { List requests = (List) activeRequests.get(key);
        Hide
        Ted Goddard added a comment -

        A similar fix is already in place in ICEfaces 1.8.2. Awaiting customer verification of 1.8.2.

        If 1.8.2 is shown to still exhibit the bug, the problem may be with the ordering of the checkSession() and addRequest() calls below:

        public void service(HttpServletRequest request, HttpServletResponse response) throws Exception {
        HttpSession session = request.getSession(true);
        checkSession(session);

        //attach session bound server to the current thread – this is a lock-free strategy
        String id = session.getId();
        try {
        //put the request in the pool of active request in case HttpServletRequest.isUserInRole need to be called
        addRequest(id, request);

        checkSession() establishes the isUserInRole() callbacks, but the request objects are not populated until the addRequest() call. Hence, it may be possible for another thread to invoke isUserInRole prior to addRequest(), however, this seems unlikely since this bug is only present on the first page view (hence the first request).

        Show
        Ted Goddard added a comment - A similar fix is already in place in ICEfaces 1.8.2. Awaiting customer verification of 1.8.2. If 1.8.2 is shown to still exhibit the bug, the problem may be with the ordering of the checkSession() and addRequest() calls below: public void service(HttpServletRequest request, HttpServletResponse response) throws Exception { HttpSession session = request.getSession(true); checkSession(session); //attach session bound server to the current thread – this is a lock-free strategy String id = session.getId(); try { //put the request in the pool of active request in case HttpServletRequest.isUserInRole need to be called addRequest(id, request); checkSession() establishes the isUserInRole() callbacks, but the request objects are not populated until the addRequest() call. Hence, it may be possible for another thread to invoke isUserInRole prior to addRequest(), however, this seems unlikely since this bug is only present on the first page view (hence the first request).
        Hide
        Ken Fyten added a comment -

        Customer confirms 1.8.2 already has the fix included, so this JIRA is redundant.

        Show
        Ken Fyten added a comment - Customer confirms 1.8.2 already has the fix included, so this JIRA is redundant.

          People

          • Assignee:
            Ted Goddard
            Reporter:
            Tyler Johnson
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: