10
\$\begingroup\$

I'm working on a experimental code which allows users to authorize using JWT's jjwt library. Here's what I have done so far on authentication and authorization flow.

  • Can I get some improvement ideas on this?
  • And what security issues can be found in this?
  • Any other best practices and suggestions are appreciated.
  • Is it a good idea to show specific error responses to client when token or token header is altered or invalid ?

User login

  • User enters login credentials. (User login page is being displayed to every user)
  • Authentication service validates user's existence and login credentials.
  • If user has successfully authenticated, then creates jwt, HttpSession objects for user and a cookie which has jwt as its value.

In client side

  • In client side (web browser), javascript reads this cookie and sends to server it's value (jwt) with every request as request header Authorization: Bearer 'jwt'

JWT interceptor

  • AuthenticationInterceptor works as the following snippets.

in preHandle() method

  • read the authorization header

    String header = request.getHeader("Authorization");
String request_url = request.getServletPath();

    - if header is not found, 

    // Authorisation header not found - unauthorised,
// redirects to another URL that creates appropriate response
if (header == null || header.isEmpty()) {
    LOGGER.debug("INTERCEPTOR-UNAUTHORIZED authorization header not found");
    response.sendRedirect(
            new StringBuilder()
            .append(request.getContextPath())
            .append("/api/user/authentication")
            .append("?e=3")
            .toString()
    );
    return false;
}

    - if header value malformed (does not contain Bearer string)

    // Authorisation header not invalid - unauthorised
// redirects to another URL that creates appropriate response
if (!header.startsWith("Bearer ")) {
    LOGGER.debug("INTERCEPTOR-UNAUTHORIZED authorization header not found or malformed");
    response.sendRedirect(
            new StringBuilder()
            .append(request.getContextPath())
            .append("/api/user/authentication")
            .append("?e=3")
            .toString()
    );
    return false;
}

    - when an invalid token found

    String cleanedHeader = header.replace("Bearer ", "").trim();
// Authorisation header value expired/invalid - unauthorised
// redirects to another URL that creates appropriate response
if (!getJwtProviderService().validateToken(cleanedHeader)) {
    LOGGER.debug("INTERCEPTOR-UNAUTHORIZED authorization token is not valid or altered");
    response.sendRedirect(
            new StringBuilder()
            .append(request.getContextPath())
            .append("/api/user/authentication")
            .append("?e=1")
            .toString()
    );
    return false;
}

    - Authorization part

        /**
 * method 'authenticateUser(HttpServletRequest, String)' will do the
 * validation part of jwt and http session of this request.
 *
 * if jwt is successfully validated and the http session exists, and
 * these details are matched, it will return 'VIA_SESSION_AND_TOKEN'
 * enum.
 *
 * if jwt is successfully validated but http session has expired, then
 * it will return 'VIA_AUTHENTICATED_TOKEN' enum.
 *
 * if jwt is invalid, it will return 'NOT_AUTHENTICATED' enum.
 */
AuthenticationOption authenticateUser = getJwtProviderService().authenticateUser(request, cleanedHeader);
// Authorisation header value expired/invalid/user not authenticated - unauthorised
// redirects to another URL that creates appropriate response
if (AuthenticationOption.NOT_AUTHENTICATED.equals(authenticateUser)) {
    LOGGER.debug("INTERCEPTOR-UNAUTHORIZED authorization token is not valid or altered");
    response.sendRedirect(
            new StringBuilder()
            .append(request.getContextPath())
            .append("/api/user/authentication")
            .append("?e=2")
            .toString()
    );
    return false;
}

switch (authenticateUser) {
    case VIA_SESSION_AND_TOKEN: {
        //
        LOGGER.debug("INTERCEPTOR-AUTHORIZED via token and session");
        String userType = getUserType(request);
        switch (userType) {
            case "ADMIN":
                return request_url.contains("/admin/") || request_url.contains("/api/") || request_url.contains("/user/");
            case "USER":
                return request_url.contains("/user/") || request_url.contains("/api/");
            default:
                return false;
        }
    }
    case VIA_AUTHENTICATED_TOKEN: {
        LOGGER.debug("INTERCEPTOR-AUTHORIZED via token");
        //user has authorised via token, then creates new http session
        boolean create = createNewSession(request, cleanedHeader);
        //if new session has successfully created then re-directs 
        //to same URL to execute authorization again.
        if (create) {
            response.sendRedirect(
                    new StringBuilder()
                    .append(request.getContextPath())
                    .append(request.getServletPath())
                    .toString()
            );
        }
    }
    default:
        return false;
}
}

  • read user role from session object (if session exists)

    private String getUserType(HttpServletRequest request) {
    AuthorizedUser user = (AuthorizedUser) request.getSession().getAttribute("_user_session");
    if (user == null) {
        return " ";
    }
    return user.getRole();
}

  • create new http session

    /**
 * this will create a new http session for request that already contains
 * valid jwt cookie but previous session was expired, new session will
 * create for carry out user's further actions.
 */
 private boolean createNewSession(HttpServletRequest request, String token) throws HibernateException, UnsupportedEncodingException {
    request.getSession().invalidate();
    Object serviceResponse = getAuthenticationAndAuthorizationService().createSessionForUserAuthenticatedViaToken(token);
    if (serviceResponse instanceof SuccessServiceMessage) {
        AuthorizedUser au = ((AuthorizedUser) ((SuccessServiceMessage) serviceResponse).getPayload());
        HttpSession user_session = request.getSession();
        user_session.setAttribute("_user_session", au);
        user_session.setMaxInactiveInterval(3600);

        return true;
    } else {
        return false;
    }
}

Full working code

    @Autowired
    private JwtProviderService jwtProviderService;

    @Autowired
    private AuthenticationAndAuthorizationService authenticationAndAuthorizationService;

/**
 *
 * @return
 */
    private JwtProviderService getJwtProviderService() {
        return this.jwtProviderService;
    }

/**
 *
 * @return
 */
    private AuthenticationAndAuthorizationService getAuthenticationAndAuthorizationService() {
        return this.authenticationAndAuthorizationService;
    }

    @Override
    public void postHandle(
            HttpServletRequest request,
            HttpServletResponse response,
            Object handler,
            ModelAndView modelAndView
    ) throws Exception {
         LOGGER.debug("INTERCEPTOR-Request post handler----------------------");
         LOGGER.debug("INTERCEPTOR-Request path: {}", request.getServletPath());
          LOGGER.debug("INTERCEPTOR-------------------------------------------");
    }

    @Override
    public void afterCompletion(
            HttpServletRequest request,
            HttpServletResponse response,
            Object handler,
            Exception ex
    ) throws Exception {
        LOGGER.debug("INTERCEPTOR-Request completed-------------------------");
        LOGGER.debug("INTERCEPTOR-Request path: {}", request.getServletPath());
    LOGGER.debug("INTERCEPTOR-------------------------------------------");
    }

    @Override
    public boolean preHandle(
            HttpServletRequest request,
            HttpServletResponse response,
            Object handler
    ) throws Exception {
        LOGGER.debug("INTERCEPTOR-A request has just hit----------------------");
        LOGGER.debug("INTERCEPTOR-Request path: {}", request.getServletPath());

        String header = request.getHeader("Authorization");
        String request_url = request.getServletPath();

        // Authorisation header not found - unauthorised,
        // redirects to another URL that creates appropriate response
        if (header == null || header.isEmpty()) {
            LOGGER.debug("INTERCEPTOR-UNAUTHORIZED authorization header not found");
            response.sendRedirect(
                    new StringBuilder()
                        .append(request.getContextPath())
                        .append("/api/user/authentication")
                        .append("?e=3")
                        .toString()
            );
            return false;
        }

        // Authorisation header not invalid - unauthorised
        // redirects to another URL that creates appropriate response
        if (!header.startsWith("Bearer ")) {
            LOGGER.debug("INTERCEPTOR-UNAUTHORIZED authorization header not found or malformed");
            response.sendRedirect(
                    new StringBuilder()
                        .append(request.getContextPath())
                        .append("/api/user/authentication")
                        .append("?e=3")
                        .toString()
            );
            return false;
        }

        String cleanedHeader = header.replace("Bearer ", "").trim();
        // Authorisation header value expired/invalid - unauthorised
        // redirects to another URL that creates appropriate response
        if (!getJwtProviderService().validateToken(cleanedHeader)) {
            LOGGER.debug("INTERCEPTOR-UNAUTHORIZED authorization token is not valid or altered");
            response.sendRedirect(
                    new StringBuilder()
                        .append(request.getContextPath())
                        .append("/api/user/authentication")
                        .append("?e=1")
                .toString()
        );
        return false;
    }

    /**
     * method 'authenticateUser(HttpServletRequest, String)' will do the
     * validation part of jwt and http session of this request.
     *
     * if jwt is successfully validated and the http session exists, and
     * these details are matched, it will return 'VIA_SESSION_AND_TOKEN'
     * enum.
     *
     * if jwt is successfully validated but http session has expired, then
     * it will return 'VIA_AUTHENTICATED_TOKEN' enum.
     *
     * if jwt is invalid, it will return 'NOT_AUTHENTICATED' enum.
     */
    AuthenticationOption authenticateUser = getJwtProviderService().authenticateUser(request, cleanedHeader);
    // Authorisation header value expired/invalid/user not authenticated - unauthorised
    // redirects to another URL that creates appropriate response
    if (AuthenticationOption.NOT_AUTHENTICATED.equals(authenticateUser)) {
        LOGGER.debug("INTERCEPTOR-UNAUTHORIZED authorization token is not valid or altered");
        response.sendRedirect(
                new StringBuilder()
                .append(request.getContextPath())
                .append("/api/user/authentication")
                .append("?e=2")
                .toString()
        );
        return false;
    }

    switch (authenticateUser) {
        case VIA_SESSION_AND_TOKEN: {
            //
            LOGGER.debug("INTERCEPTOR-AUTHORIZED via token and session");
            String userType = getUserType(request);
            switch (userType) {
                case "ADMIN":
                    return request_url.contains("/admin/") || request_url.contains("/api/") || request_url.contains("/user/");
                case "USER":
                    return request_url.contains("/user/") || request_url.contains("/api/");
                default:
                    return false;
            }
        }
        case VIA_AUTHENTICATED_TOKEN: {
            LOGGER.debug("INTERCEPTOR-AUTHORIZED via token");
            //user has authorised via token, then creates new http session
            boolean create = createNewSession(request, cleanedHeader);
            //if new session has successfully created then re-directs 
            //to same URL to execute authorization again.
            if (create) {
                response.sendRedirect(
                        new StringBuilder()
                        .append(request.getContextPath())
                        .append(request.getServletPath())
                        .toString()
                );
            }
        }
        default:
            return false;
    }
}

/**
 *
 * @param request
 * @return
 */
private String getUserType(HttpServletRequest request) {
    AuthorizedUser user = (AuthorizedUser) request.getSession().getAttribute("_user_session");
    if (user == null) {
        return " ";
    }
    return user.getRole();
}

/**
 *
 * this will create a new http session for request that already contains
 * valid jwt cookie but previous session was expired, new session will
 * create for carry out user's further actions.
 */
private boolean createNewSession(HttpServletRequest request, String token) throws HibernateException, UnsupportedEncodingException {
    request.getSession().invalidate();
    Object serviceResponse = getAuthenticationAndAuthorizationService().createSessionForUserAuthenticatedViaToken(token);
    if (serviceResponse instanceof SuccessServiceMessage) {
        AuthorizedUser au = ((AuthorizedUser) ((SuccessServiceMessage) serviceResponse).getPayload());
        HttpSession user_session = request.getSession();
        user_session.setAttribute("_user_session", au);
        user_session.setMaxInactiveInterval(3600);

        return true;
    } else {
        return false;
    }
}
Improve this question
\$\endgroup\$

1 Answer 1

Reset to default
1
\$\begingroup\$

Well , I can give you some suggestions ( though debatable)

  1. Log levels has to be chosen properly. I see use of debug in all places. Actually, the info level is good enough to be used for simple texts like LOGGER.debug("INTERCEPTOR-Request post handler----------------------"); . But the debug fits nice , if you have a piece of derived info in log - like LOGGER.debug("INTERCEPTOR-Request path: {}", request.getServletPath()); .
  2. Use of Exceptions are not seen much. One better approach than returning true / false would be throwing appropriate customized exceptions - reduces a lot of branching in the code [code-readability] because of if..else. Also, logging the stack-traces enables better backtracking in case of errors.
  3. In my view, the use of StringBuilder is to be enforced for concatenations more than 3 times or non-deterministic cases like loops only.
  4. One general rule which can be followed is the return-guarantee. If a method says, it will return a specific functional data-unit (not technical unit like integer/ string/ boolean - but say role / sessionId etc.) the method should ensure it returns the valid data. On any other case an exception should be used. For eg., In method getUserType() , the data returned is not always a user-id, but also a blank space.
  5. Make use of custom exceptions to handle specific error messages. Use additional error codes instead of error messages to enable translatable errors.

These came immediately in mind. Probably will update this answer later. Hope this helps you a bit.

Improve this answer
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.