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 acookie
which hasjwt
as its value.
In client side
- In client side (web browser),
javascript
reads thiscookie
and sends to server it's value (jwt
) with every request as request headerAuthorization: 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;
}
}
1 Answer 1
Well , I can give you some suggestions ( though debatable)
- 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 - likeLOGGER.debug("INTERCEPTOR-Request path: {}", request.getServletPath());
. - 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 ofif..else
. Also, logging the stack-traces enables better backtracking in case of errors. - In my view, the use of StringBuilder is to be enforced for concatenations more than 3 times or non-deterministic cases like loops only.
- 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 methodgetUserType()
, the data returned is not always a user-id, but also a blank space. - 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.