[ https://issues.apache.org/jira/browse/SOLR-13835?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16951179#comment-16951179 ]
Chris M. Hostetter commented on SOLR-13835: ------------------------------------------- Jan: genreally speaking [https://patch-diff.githubusercontent.com/raw/apache/lucene-solr/pull/946.patch] seems like an improvement, but as far as the larger issue of how this code is deciding what AuditEvents to fire based on the AuthorizationResponse.... i still have the same concern i mentioned before regarding he "non-200|202" block (which is unchanged in your patch): bq. ...automatically assumed to be EventType.UNAUTHORIZED ... that seems kind of brittle and error prone ... what if an AuthorizationPlugin returns a 500 status code? bq. (particularly since the AuthorizationPlugin API seems open enough (ie: there is no fixed enum of authResponse.statusCode values) that a custom plugin could return a lot of diff non-200/202 error codes that the AuditLogger would all report as "UNAUTHORIZED") As a concrete example: i can write an AuthorizationPlugin that recognizes a user does in fact have access to a resource, but returns {{new AuthorizationResponse(429)}} because of the current state of the system, causing a 429 error to be returned to external client -- but that will be reported to the AuditLogger plugin as {{EventType.UNAUTHORIZED == 403}} that seems brittle. Shouldn't the AuditLogger hooks be more explicit & restricted and only report each {{EventType}} when they know for a fact that situation / HTTP Status code occured, and have some more generic, and/or use a more generic {{EventType.UNKNOWN}} if there is no direct correlation between the AuthorizationResponse.statusCode and the predefined EventTypes? (or at the very least, use {{EventType.ERROR}} when there isn't a direct correlation? > HttpSolrCall produces incorrect extra AuditEvent on > AuthorizationResponse.PROMPT > -------------------------------------------------------------------------------- > > Key: SOLR-13835 > URL: https://issues.apache.org/jira/browse/SOLR-13835 > Project: Solr > Issue Type: Bug > Security Level: Public(Default Security Level. Issues are Public) > Components: Authentication, Authorization > Reporter: Chris M. Hostetter > Assignee: Jan Høydahl > Priority: Major > Time Spent: 10m > Remaining Estimate: 0h > > spinning this out of SOLR-13741... > {quote} > Wrt the REJECTED + UNAUTHORIZED events I see the same as you, and I believe > there is a code bug, not a test bug. In HttpSolrCall#471 in the > {{authorize()}} call, if authResponse == PROMPT, it will actually match both > blocks and emit two audit events: > [https://github.com/apache/lucene-solr/blob/26ede632e6259eb9d16861a3c0f782c9c8999762/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java#L475:L493] > > {code:java} > if (authResponse.statusCode == AuthorizationResponse.PROMPT.statusCode) {...} > if (!(authResponse.statusCode == HttpStatus.SC_ACCEPTED) && > !(authResponse.statusCode == HttpStatus.SC_OK)) {...} > {code} > When code==401, it is also true that code!=200. Intuitively there should be > both a sendErrora and return RETURN before line #484 in the first if block? > {quote} > This causes any and all {{REJECTED}} AuditEvent messages to be accompanied by > a coresponding {{UNAUTHORIZED}} AuditEvent. > It's not yet clear if, from the perspective of the external client, there are > any other bugs in behavior (TBD) -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org