Skip to content

Commit

Permalink
Process feedback for security code cleaning
Browse files Browse the repository at this point in the history 
Signed-off-by: Arjan Tijms <arjan.tijms@omnifish.ee>
  • Loading branch information
@arjantijms
arjantijms committed Apr 25, 2023
1 parent f7a3f29 commit 7634652
Showing 1 changed file with 50 additions and 53 deletions.
103 changes: 50 additions & 53 deletions ...rver/web/web-core/src/main/java/org/apache/catalina/authenticator/BasicAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@

package org.apache.catalina.authenticator;

import static java.util.Locale.ENGLISH;
import static java.util.logging.Level.FINE;
import static org.apache.catalina.authenticator.Constants.BASIC_METHOD;
import static org.apache.catalina.authenticator.Constants.REQ_SSOID_NOTE;

import java.io.IOException;
import java.nio.charset.Charset;
import java.security.Principal;
import java.util.Locale;
import java.util.logging.Level;

import org.apache.catalina.HttpRequest;
import org.apache.catalina.HttpResponse;
Expand All @@ -39,7 +42,6 @@
* @author Craig R. McClanahan
* @version $Revision: 1.7 $ $Date: 2007/05/05 05:31:52 $
*/

public class BasicAuthenticator extends AuthenticatorBase {

// --------------------------------------------------- Instance Variables
Expand All @@ -51,12 +53,18 @@ public class BasicAuthenticator extends AuthenticatorBase {

// ----------------------------------------------------------- Properties


/**
* Return descriptive information about this Valve implementation.
*/
@Override
public String getInfo() {
return (this.info);
return info;
}

@Override
protected String getAuthMethod() {
return HttpServletRequest.BASIC_AUTH;
}

// ------------------------------------------------------- Public Methods
Expand All @@ -73,61 +81,54 @@ public String getInfo() {
*/
@Override
public boolean authenticate(HttpRequest request, HttpResponse response, LoginConfig config) throws IOException {

// Have we already authenticated someone?
Principal principal = ((HttpServletRequest) request.getRequest()).getUserPrincipal();
if (principal != null) {
if (log.isLoggable(Level.FINE)) {
log.log(Level.FINE, "Already authenticated '" + principal.getName() + "'");
Principal existingPrincipal = ((HttpServletRequest) request.getRequest()).getUserPrincipal();
if (existingPrincipal != null) {
if (log.isLoggable(FINE)) {
log.log(FINE, "Already authenticated '" + existingPrincipal.getName() + "'");
}
return (true);

return true;
}

// Validate any credentials already included with this request
HttpServletResponse hres = (HttpServletResponse) response.getResponse();
HttpServletResponse httpServletResponse = (HttpServletResponse) response.getResponse();
String authorization = request.getAuthorization();

/*
* IASRI 4868073 String username = parseUsername(authorization); String password = parsePassword(authorization);
* principal = context.getRealm().authenticate(username, password); if (principal != null) { register(request, response,
* principal, Constants.BASIC_METHOD, username, password); return (true); }
*/
// BEGIN IASRI 4868073
// Only attempt to parse and validate the authorization if one was
// sent by the client. No reason to attempt to login with null
// authorization which must fail anyway. With basic auth this
// scenario always occurs first so this is a common case. This
// will also prevent logging the audit message for failure to
// authenticate null user (since login failures are always logged
// per psarc req).
// Only attempt to parse and validate the authorization if one was sent by the client.
//
// No reason to attempt to login with null authorization which must fail anyway.
// With basic authentication this scenario always occurs first so this is a common case. This
// will also prevent logging the audit message for failure to authenticate null user
// (since login failures are always logged per "psarc" request).

if (authorization != null) {
String username = parseUsername(authorization);
char[] password = parsePassword(authorization);
principal = context.getRealm().authenticate(username, password);
if (principal != null) {
register(request, response, principal, Constants.BASIC_METHOD, username, password);
String ssoId = (String) request.getNote(Constants.REQ_SSOID_NOTE);

Principal authenticatedPrincipal = context.getRealm().authenticate(username, password);

if (authenticatedPrincipal != null) {
register(request, response, authenticatedPrincipal, BASIC_METHOD, username, password);
String ssoId = (String) request.getNote(REQ_SSOID_NOTE);
if (ssoId != null) {
getSession(request, true);
}
return (true);

return true;
}
}
// END IASRI 4868073

// Send an "unauthorized" response and an appropriate challenge
String realmName = config.getRealmName();
if (realmName == null) {
realmName = REALM_NAME;
}
// if (debug >= 1)
// log("Challenging for realm '" + realmName + "'");
hres.setHeader(AUTH_HEADER_NAME, "Basic realm=\"" + realmName + "\"");
hres.sendError(HttpServletResponse.SC_UNAUTHORIZED);
// hres.flushBuffer();
return (false);

httpServletResponse.setHeader(AUTH_HEADER_NAME, "Basic realm=\"" + realmName + "\"");
httpServletResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED);

return false;
}

// ------------------------------------------------------ Protected Methods
Expand All @@ -138,22 +139,20 @@ public boolean authenticate(HttpRequest request, HttpResponse response, LoginCon
* @param authorization Authorization credentials from this request
*/
protected String parseUsername(String authorization) {

if ((authorization == null) || !authorization.toLowerCase(Locale.ENGLISH).startsWith("basic ")) {
return (null);
if (!isBasicAuthHeader(authorization)) {
return null;
}

authorization = authorization.substring(6).trim();

// Decode and parse the authorization credentials
String unencoded = new String(Base64.decode(authorization.getBytes(Charset.defaultCharset())));
int colon = unencoded.indexOf(':');
if (colon < 0) {
return (null);
return null;
}
String username = unencoded.substring(0, colon);
// String password = unencoded.substring(colon + 1).trim();
return (username);

return unencoded.substring(0, colon);
}

/**
Expand All @@ -162,26 +161,24 @@ protected String parseUsername(String authorization) {
* @param authorization Authorization credentials from this request
*/
protected char[] parsePassword(String authorization) {

if ((authorization == null) || !authorization.toLowerCase(Locale.ENGLISH).startsWith("basic ")) {
return (null);
if (!isBasicAuthHeader(authorization)) {
return null;
}

authorization = authorization.substring(6).trim();

// Decode and parse the authorization credentials
String unencoded = new String(Base64.decode(authorization.getBytes(Charset.defaultCharset())));
int colon = unencoded.indexOf(':');
if (colon < 0) {
return (null);
return null;
}
// String username = unencoded.substring(0, colon).trim();
char[] password = unencoded.substring(colon + 1).toCharArray();
return (password);

return unencoded.substring(colon + 1).toCharArray();
}

@Override
protected String getAuthMethod() {
return HttpServletRequest.BASIC_AUTH;
private static boolean isBasicAuthHeader(String authorization) {
return authorization != null && authorization.toLowerCase(ENGLISH).startsWith("basic ");
}

}

0 comments on commit 7634652

Please sign in to comment.