-
Notifications
You must be signed in to change notification settings - Fork 28
AMM-1239 : Role based broken access control #96
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
Changes from all commits
bd187b2
abd507b
2bca084
ca91a89
58a9826
7be5c2d
ee52ddf
9e395ed
9917e0f
20542c6
d1cec38
a2bc624
9d74754
13b5013
9be3aa1
c690b3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package com.iemr.ecd.utils.advice.exception_handler; | ||
|
|
||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import org.springframework.security.access.AccessDeniedException; | ||
| import org.springframework.security.web.access.AccessDeniedHandler; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Map; | ||
|
|
||
| @Component | ||
| public class CustomAccessDeniedHandler implements AccessDeniedHandler { | ||
|
|
||
| private static final ObjectMapper mapper = new ObjectMapper(); | ||
| @Override | ||
| public void handle(HttpServletRequest request, | ||
| HttpServletResponse response, | ||
| AccessDeniedException accessDeniedException) throws IOException { | ||
| response.setStatus(HttpServletResponse.SC_FORBIDDEN); // 403 | ||
| response.setContentType("application/json"); | ||
| Map<String, String> errorResponse = Map.of("error" , "Forbidden", | ||
| "message","Access denied"); | ||
| response.getWriter().write(mapper.writeValueAsString(errorResponse)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,23 @@ | ||||||
| package com.iemr.ecd.utils.advice.exception_handler; | ||||||
|
|
||||||
| import java.io.IOException; | ||||||
|
|
||||||
| import org.springframework.security.core.AuthenticationException; | ||||||
| import org.springframework.security.web.AuthenticationEntryPoint; | ||||||
| import org.springframework.stereotype.Component; | ||||||
|
|
||||||
| import jakarta.servlet.http.HttpServletRequest; | ||||||
| import jakarta.servlet.http.HttpServletResponse; | ||||||
|
|
||||||
| @Component | ||||||
| public class CustomAuthenticationEntryPoint implements AuthenticationEntryPoint { | ||||||
|
|
||||||
| @Override | ||||||
| public void commence(HttpServletRequest request, | ||||||
| HttpServletResponse response, | ||||||
| AuthenticationException authException) throws IOException { | ||||||
| response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); // 401 | ||||||
| response.setContentType("application/json"); | ||||||
| response.getWriter().write("{\"error\": \"Unauthorized\", \"message\": \"" + authException.getMessage() + "\"}"); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security vulnerability: Information disclosure and JSON injection risk. This has the same security issues as
Apply the same fix pattern: - response.getWriter().write("{\"error\": \"Unauthorized\", \"message\": \"" + authException.getMessage() + "\"}");
+ response.getWriter().write("{\"error\": \"Unauthorized\", \"message\": \"Authentication required\"}");Or use a JSON library for proper serialization as suggested in the π Committable suggestion
Suggested change
π€ Prompt for AI AgentsPotential information leakage and JSON injection vulnerability. Similar to the Apply the same security fixes as recommended for -response.getWriter().write("{\"error\": \"Unauthorized\", \"message\": \"" + authException.getMessage() + "\"}");
+response.getWriter().write("{\"error\": \"Unauthorized\", \"message\": \"Authentication required\"}");Or use a proper JSON library for safe serialization. π€ Prompt for AI Agents |
||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| package com.iemr.ecd.utils.mapper; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Objects; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; | ||
| import org.springframework.security.core.GrantedAuthority; | ||
| import org.springframework.security.core.authority.SimpleGrantedAuthority; | ||
| import org.springframework.security.core.context.SecurityContextHolder; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.web.filter.OncePerRequestFilter; | ||
|
|
||
| import com.iemr.ecd.service.masters.MasterServiceImpl; | ||
| import com.iemr.ecd.utils.constants.Constants; | ||
| import com.iemr.ecd.utils.redis.RedisStorage; | ||
|
|
||
| import io.jsonwebtoken.Claims; | ||
| import io.jsonwebtoken.io.IOException; | ||
| import jakarta.servlet.FilterChain; | ||
| import jakarta.servlet.ServletException; | ||
| import jakarta.servlet.http.Cookie; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| @Component | ||
| public class RoleAuthenticationFilter extends OncePerRequestFilter { | ||
| Logger logger = LoggerFactory.getLogger(this.getClass().getSimpleName()); | ||
|
|
||
| @Autowired | ||
| private JwtUtil jwtUtil; | ||
|
|
||
| @Autowired | ||
| private RedisStorage redisService; | ||
|
|
||
| @Autowired | ||
| private MasterServiceImpl userService; | ||
|
|
||
| @Override | ||
| protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) | ||
| throws ServletException, IOException, java.io.IOException { | ||
| List<String> authRoles = null; | ||
| try { | ||
| String jwtFromCookie = CookieUtil.getJwtTokenFromCookie(request); | ||
| String jwtFromHeader = request.getHeader(Constants.JWT_TOKEN); | ||
|
|
||
| String jwtToken = jwtFromCookie != null ? jwtFromCookie : jwtFromHeader; | ||
| if(null == jwtToken || jwtToken.trim().isEmpty()) { | ||
| filterChain.doFilter(request, response); | ||
| return; | ||
| } | ||
| Claims extractAllClaims = jwtUtil.extractAllClaims(jwtToken); | ||
| if(null == extractAllClaims) { | ||
| filterChain.doFilter(request, response); | ||
| return; | ||
| } | ||
| Object userIdObj = extractAllClaims.get("userId"); | ||
| String userId = userIdObj != null ? userIdObj.toString() : null; | ||
| if (null == userId || userId.trim().isEmpty()) { | ||
| filterChain.doFilter(request, response); | ||
| return; | ||
| } | ||
| Long userIdLong; | ||
| try { | ||
| userIdLong=Long.valueOf(userId); | ||
| }catch (NumberFormatException ex) { | ||
| logger.warn("Invalid userId format: {}",userId); | ||
| filterChain.doFilter(request, response); | ||
| return; | ||
| } | ||
| authRoles = redisService.getUserRoleFromCache(userIdLong); | ||
| if (authRoles == null || authRoles.isEmpty()) { | ||
| List<String> roles = userService.getUserRoles(Long.valueOf(userId)); // assuming this returns multiple roles | ||
| authRoles = roles.stream() | ||
| .filter(Objects::nonNull) | ||
| .map(String::trim) | ||
| .map(role -> "ROLE_" + role.toUpperCase().replace(" ", "_")) | ||
| .collect(Collectors.toList()); | ||
| redisService.cacheUserRoles(Long.valueOf(userId), authRoles); | ||
| } | ||
|
|
||
| List<GrantedAuthority> authorities = authRoles.stream() | ||
| .map(SimpleGrantedAuthority::new) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(userId, null, authorities); | ||
| SecurityContextHolder.getContext().setAuthentication(auth); | ||
| } catch (Exception e) { | ||
| logger.error("Authentication filter error for request {}: {}", request.getRequestURI(), e.getMessage()); | ||
| SecurityContextHolder.clearContext(); | ||
| } finally { | ||
| filterChain.doFilter(request, response); | ||
| } | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't follow where this hasRole is checked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please verify In RoleAuthenticationFilter line no 66