Skip to content

Conversation

SanderKondratjevNortal
Copy link

Signed-off-by: Sander Kondratjev [email protected]

Copy link

Comment on lines +86 to +87
} catch (IOException e) {
throw new BadCredentialsException("Unable to authenticate the Web eID authentication token", e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There can be two separate problem cases here, either an invalid token or a technical problem with the request reader. These should perhaps throw separate exceptions:

Suggested change
} catch (IOException e) {
throw new BadCredentialsException("Unable to authenticate the Web eID authentication token", e);
} catch (JacksonException e) {
throw new BadCredentialsException("Unable to parse the Web eID authentication token", e);
} catch (IOException e) {
throw new AuthenticationServiceException("I/O error while reading the Web eID authentication token", e);

private static final Logger LOG = LoggerFactory.getLogger(WebEidAjaxLoginProcessingFilter.class);
private final ObjectReader OBJECT_READER = new ObjectMapper().readerFor(AuthTokenDTO.class);
private final SecurityContextRepository securityContextRepository;
private final ObjectReader OBJECT_READER = new ObjectMapper().readerFor(WebEidAuthToken.class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private final ObjectReader OBJECT_READER = new ObjectMapper().readerFor(WebEidAuthToken.class);
private static final ObjectReader OBJECT_READER = new ObjectMapper().readerFor(WebEidAuthToken.class);

protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain, Authentication authResult) throws IOException, ServletException {
super.successfulAuthentication(request, response, chain, authResult);
securityContextRepository.saveContext(SecurityContextHolder.getContext(), request, response);
private WebEidAuthToken parseWebEidAuthToken(HttpServletRequest request) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private WebEidAuthToken parseWebEidAuthToken(HttpServletRequest request) {
private static WebEidAuthToken parseWebEidAuthToken(HttpServletRequest request) {

Comment on lines +69 to 73
final String contentType = request.getHeader(HttpHeaders.CONTENT_TYPE);
if (contentType == null || !contentType.startsWith(MediaType.APPLICATION_JSON_VALUE)) {
LOG.warn("Content type not supported: {}", contentType);
throw new AuthenticationServiceException("Content type not supported: " + contentType);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's extract content type validation to separate method as well:

Suggested change
final String contentType = request.getHeader(HttpHeaders.CONTENT_TYPE);
if (contentType == null || !contentType.startsWith(MediaType.APPLICATION_JSON_VALUE)) {
LOG.warn("Content type not supported: {}", contentType);
throw new AuthenticationServiceException("Content type not supported: " + contentType);
}
requireJsonContentType(request);

requireJsonContentType() might look as follows (untested, please verify):

    private static void requireJsonContentType(HttpServletRequest request) {
        try {
            MediaType contentType = MediaType.parseMediaType(request.getContentType());
            if (!MediaType.APPLICATION_JSON.equalsTypeAndSubtype(contentType)) {
                LOG.warn("Content type not supported: {}", contentType);
                throw new AuthenticationServiceException("Content type not supported: " + contentType);
            }
        } catch (InvalidMediaTypeException e) {
            LOG.warn("Invalid content type", e);
            throw new AuthenticationServiceException("Invalid content type", e);
        }
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants