You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2022/10/25 08:25:47 UTC
[GitHub] [activemq-artemis] brusdev opened a new pull request, #4270: ARTEMIS-4071 Fix erroneus audit log messages due to console logouts
brusdev opened a new pull request, #4270:
URL: https://github.com/apache/activemq-artemis/pull/4270
The HTTP redirection messages (300 – 399) must be ignored they don't mean an authentication failure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] TheCycoONE commented on a diff in pull request #4270: ARTEMIS-4071 Fix erroneus audit log messages due to console logouts
Posted by GitBox <gi...@apache.org>.
TheCycoONE commented on code in PR #4270:
URL: https://github.com/apache/activemq-artemis/pull/4270#discussion_r1011736748
##########
artemis-web/src/main/java/org/apache/activemq/artemis/component/AuthenticationFilter.java:
##########
@@ -43,14 +43,21 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
filterChain.doFilter(servletRequest, servletResponse);
if (AuditLogger.isAnyLoggingEnabled()) {
int status = ((Response) servletResponse).getStatus();
- //status 200 means that the user has been authenticated, anything else must be a failure
- if (status == 200) {
+ if (status < 300) {
Review Comment:
Does 100 continue need to be handled as well - would that ever reach here? I think in general I would feel better if < 200 was skipped like 300-399.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] brusdev commented on a diff in pull request #4270: ARTEMIS-4071 Fix erroneus audit log messages due to console logouts
Posted by GitBox <gi...@apache.org>.
brusdev commented on code in PR #4270:
URL: https://github.com/apache/activemq-artemis/pull/4270#discussion_r1011917616
##########
artemis-web/src/main/java/org/apache/activemq/artemis/component/AuthenticationFilter.java:
##########
@@ -43,14 +43,21 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
filterChain.doFilter(servletRequest, servletResponse);
if (AuditLogger.isAnyLoggingEnabled()) {
int status = ((Response) servletResponse).getStatus();
- //status 200 means that the user has been authenticated, anything else must be a failure
- if (status == 200) {
+ if (status < 300) {
Review Comment:
Status ranges updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] brusdev commented on pull request #4270: ARTEMIS-4071 Fix erroneus audit log messages due to console logouts
Posted by GitBox <gi...@apache.org>.
brusdev commented on PR #4270:
URL: https://github.com/apache/activemq-artemis/pull/4270#issuecomment-1300645284
@clebertsuconic adding a test would be very hard, I tested it manually
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4270: ARTEMIS-4071 Fix erroneus audit log messages due to console logouts
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4270:
URL: https://github.com/apache/activemq-artemis/pull/4270#issuecomment-1292150381
@brusdev I don't have how to validate this...
if you believe this is not possible to have a test added, and know this is ok by manually testing it.. please merge it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] brusdev merged pull request #4270: ARTEMIS-4071 Fix erroneus audit log messages due to console logouts
Posted by GitBox <gi...@apache.org>.
brusdev merged PR #4270:
URL: https://github.com/apache/activemq-artemis/pull/4270
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4270: ARTEMIS-4071 Fix erroneus audit log messages due to console logouts
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4270:
URL: https://github.com/apache/activemq-artemis/pull/4270#issuecomment-1302383523
@brusdev fine... merge when you feel comfortable then . LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] TheCycoONE commented on a diff in pull request #4270: ARTEMIS-4071 Fix erroneus audit log messages due to console logouts
Posted by GitBox <gi...@apache.org>.
TheCycoONE commented on code in PR #4270:
URL: https://github.com/apache/activemq-artemis/pull/4270#discussion_r1011736748
##########
artemis-web/src/main/java/org/apache/activemq/artemis/component/AuthenticationFilter.java:
##########
@@ -43,14 +43,21 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
filterChain.doFilter(servletRequest, servletResponse);
if (AuditLogger.isAnyLoggingEnabled()) {
int status = ((Response) servletResponse).getStatus();
- //status 200 means that the user has been authenticated, anything else must be a failure
- if (status == 200) {
+ if (status < 300) {
Review Comment:
Does 100 continue need to be handled as well - would that ever reach here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org