From 5b915c7aa60caaa68a755f934d200313d1aade49 Mon Sep 17 00:00:00 2001 From: Steve Hu Date: Mon, 28 Sep 2026 10:39:09 -0400 Subject: [PATCH 1/3] Reject empty Basic auth passwords in BasicAuthMiddleware Port the light-4j BasicAuthHandler fixes: reject empty passwords before local or LDAP validation, use a constant-time password comparison, handle short Authorization headers without exceptions, and stop logging credentials or submitted usernames. Refs networknt/light-4j#2804 Co-Authored-By: Claude Opus 5.5 --- .../security/BasicAuthMiddleware.java | 37 +++++++++++-------- .../security/BasicAuthMiddlewareTest.java | 26 +++++++++++++ src/test/resources/config/basic-auth.yml | 4 ++ 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java index 6227721..c43e1ec 100644 --- a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java +++ b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java @@ -14,6 +14,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.security.MessageDigest; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -68,19 +69,19 @@ public Status execute(LightLambdaExchange exchange) { return this.handleAnonymousAuth(exchange, requestPath, config); } // verify the header with the config file. assuming it is basic authentication first. - if (BASIC_PREFIX.equalsIgnoreCase(auth.substring(0, 5))) { + if (auth.regionMatches(true, 0, BASIC_PREFIX, 0, BASIC_PREFIX.length())) { // check if the length is greater than 6 for issue1513 if(auth.trim().length() == 5) { - LOG.error("Invalid/Unsupported authorization header {}", auth); - return new Status(INVALID_AUTHORIZATION_HEADER, auth); + LOG.error("Invalid/Unsupported Basic authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, BASIC_PREFIX); } else { return this.handleBasicAuth(exchange, requestPath, auth); } - } else if (BEARER_PREFIX.equalsIgnoreCase(auth.substring(0, 6))) { + } else if (auth.regionMatches(true, 0, BEARER_PREFIX, 0, BEARER_PREFIX.length())) { return this.handleBearerToken(exchange, requestPath, auth, config); } else { - LOG.error("Invalid/Unsupported authorization header {}", auth.substring(0, 10)); - return new Status(INVALID_AUTHORIZATION_HEADER, auth.substring(0, 10)); + LOG.error("Invalid/Unsupported authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, "unknown"); } } } @@ -156,11 +157,15 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, if (pos != -1) { String username = credentials.substring(0, pos); String password = credentials.substring(pos + 1); - if(LOG.isTraceEnabled()) LOG.trace("input username = {}, password = {}", username, StringUtils.maskHalfString(password)); UserAuth user = config.getUsers().get(username); // if user cannot be found in the config, return immediately. if (user == null) { - LOG.error("User '{}' is not found in the configuration file.", username); + LOG.error("Basic authentication failed: user is not configured."); + if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); + return new Status(INVALID_USERNAME_OR_PASSWORD); + } + if (StringUtils.isEmpty(password)) { + LOG.error("Basic authentication rejected an empty password."); if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(INVALID_USERNAME_OR_PASSWORD); } @@ -169,23 +174,23 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, && StringUtils.isEmpty(user.getPassword()) && config.isEnableAD()) { // Call LdapUtil with LDAP authentication and authorization given user is matched, password is empty, and AD is enabled. - if(LOG.isTraceEnabled()) LOG.trace("Call LdapUtil with LDAP authentication and authorization for user = {}", username); + if(LOG.isTraceEnabled()) LOG.trace("Call LdapUtil for Basic authentication."); if (!handleLdapAuth(user, password)) { if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(INVALID_USERNAME_OR_PASSWORD); } } else { - if(LOG.isTraceEnabled()) LOG.trace("Validate basic auth based on config username {} and password {}", user.getUsername(), StringUtils.maskHalfString(user.getPassword())); // if username matches config, password matches config, and path matches config, pass if (!(user.getUsername().equals(username) - && password.equals(user.getPassword()))) { - LOG.error("Invalid username or password with authorization header = {}", StringUtils.maskHalfString(auth)); + && user.getPassword() != null + && MessageDigest.isEqual(password.getBytes(UTF_8), user.getPassword().getBytes(UTF_8)))) { + LOG.error("Invalid Basic username or password."); if (LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(INVALID_USERNAME_OR_PASSWORD); } } // Here we have passed the authentication. Let's do the authorization with the paths. - if(LOG.isTraceEnabled()) LOG.trace("Username and password validation is done for user = {}", username); + if(LOG.isTraceEnabled()) LOG.trace("Basic username and password validation is done."); boolean match = false; for (String path : user.getPaths()) { if (requestPath.startsWith(path)) { @@ -194,14 +199,14 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, } } if (!match) { - LOG.error("Request path '{}' is not authorized for user '{}", requestPath, user.getUsername()); + LOG.error("Basic user is not authorized for the requested path."); if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(NOT_AUTHORIZED_REQUEST_PATH, requestPath, user.getUsername()); } } else { LOG.error("Invalid basic authentication header. It must be username:password base64 encode."); if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); - return new Status(INVALID_BASIC_HEADER, auth.substring(0, 10)); + return new Status(INVALID_BASIC_HEADER, BASIC_PREFIX); } return successMiddlewareStatus(); } @@ -235,7 +240,7 @@ private Status handleBearerToken(LightLambdaExchange exchange, String requestPat if (!config.isAllowBearerToken()) { LOG.error("Not a basic authentication header, and bearer token is not allowed."); if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); - return new Status(INVALID_BASIC_HEADER, auth.substring(0, 10)); + return new Status(INVALID_BASIC_HEADER, BEARER_PREFIX); } else { // bearer token is allowed, we need to validate it and check the allowed paths. UserAuth user = config.getUsers().get(BasicAuthConfig.BEARER); diff --git a/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java b/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java index 5f073e2..7ba555a 100644 --- a/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java +++ b/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java @@ -42,6 +42,32 @@ private static String encodeCredentials(String username, String password) { return encodeCredentialsFullFormat(username, password, ":"); } + @Test + public void testEmptyConfiguredPasswordIsRejected() { + Assertions.assertEquals("", BasicAuthConfig.load().getUsers().get("blankPassword").getPassword()); + var requestEvent = TestUtils.createTestRequestEvent(); + requestEvent.setPath("/v2/pet"); + requestEvent.getHeaders().put(HeaderKey.AUTHORIZATION, "BASIC " + encodeCredentials("blankPassword", "")); + var exchange = new LightLambdaExchange(new LambdaContext("empty-password"), null); + exchange.setInitialRequest(requestEvent); + Status status = new BasicAuthMiddleware("basic-auth").execute(exchange); + Assertions.assertEquals(401, status.getStatusCode()); + Assertions.assertEquals("ERR10047", status.getCode()); + } + + @Test + public void testShortMalformedAuthorizationHeadersAreRejected() { + for (String header : new String[] {"x", "Basic eA"}) { + var requestEvent = TestUtils.createTestRequestEvent(); + requestEvent.setPath("/v2/pet"); + requestEvent.getHeaders().put(HeaderKey.AUTHORIZATION, header); + var exchange = new LightLambdaExchange(new LambdaContext("short-header"), null); + exchange.setInitialRequest(requestEvent); + Status status = new BasicAuthMiddleware("basic-auth").execute(exchange); + Assertions.assertEquals(401, status.getStatusCode(), header); + } + } + /** * Test with right credentials but incorrect path. Expect 401 status code. */ diff --git a/src/test/resources/config/basic-auth.yml b/src/test/resources/config/basic-auth.yml index dd0b98a..fccf4d8 100644 --- a/src/test/resources/config/basic-auth.yml +++ b/src/test/resources/config/basic-auth.yml @@ -19,6 +19,10 @@ users: password: user1pass paths: - /v1/address + - username: blankPassword + password: "" + paths: + - /v2/pet - username: user2 password: CRYPT:2f9fefa2c00d6ead365a6a20967f235d:e84bb6cccc5c5887f0331186:98eb70096c4d354e88f6e7863cdccf22897a12c2d669e26c paths: From 404c00617b59dbd426cd2a6bfd19d15e05b6a06d Mon Sep 17 00:00:00 2001 From: Steve Hu Date: Fri, 2 Oct 2026 15:18:14 -0400 Subject: [PATCH 2/3] fixes #185 review findings --- .../security/AuthorizationScheme.java | 23 ++++ .../security/BasicAuthMiddleware.java | 54 ++++----- .../security/UnifiedSecurityMiddleware.java | 20 ++-- .../security/BasicAuthMiddlewareTest.java | 106 +++++++++++++++++- src/test/resources/config/basic-auth-ldap.yml | 4 + .../config/unified-security-review.yml | 5 + 6 files changed, 168 insertions(+), 44 deletions(-) create mode 100644 src/main/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationScheme.java create mode 100644 src/test/resources/config/unified-security-review.yml diff --git a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationScheme.java b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationScheme.java new file mode 100644 index 0000000..5b035b8 --- /dev/null +++ b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/AuthorizationScheme.java @@ -0,0 +1,23 @@ +package com.networknt.aws.lambda.handler.middleware.security; + +/** Scheme classification without retaining or exposing credential material. */ +enum AuthorizationScheme { + BASIC, BEARER, UNKNOWN; + + static AuthorizationScheme parse(String header) { + if (header == null) return UNKNOWN; + for (AuthorizationScheme scheme : new AuthorizationScheme[] {BASIC, BEARER}) { + String prefix = scheme.name(); + if (header.regionMatches(true, 0, prefix, 0, prefix.length()) + && (header.length() == prefix.length() || header.charAt(prefix.length()) == ' ')) { + return scheme; + } + } + return UNKNOWN; + } + + static boolean hasCredentials(String header, AuthorizationScheme scheme) { + return scheme != UNKNOWN && parse(header) == scheme && header.length() > scheme.name().length() + 1 + && !header.substring(scheme.name().length() + 1).trim().isEmpty(); + } +} diff --git a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java index c43e1ec..537d384 100644 --- a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java +++ b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java @@ -7,7 +7,6 @@ import com.networknt.utility.MapUtil; import com.networknt.basicauth.BasicAuthConfig; import com.networknt.basicauth.UserAuth; -import com.networknt.config.Config; import com.networknt.ldap.LdapUtil; import com.networknt.status.Status; import com.networknt.utility.StringUtils; @@ -15,7 +14,6 @@ import org.slf4j.LoggerFactory; import java.security.MessageDigest; -import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Optional; @@ -68,21 +66,14 @@ public Status execute(LightLambdaExchange exchange) { if(auth.trim().isEmpty()) { return this.handleAnonymousAuth(exchange, requestPath, config); } - // verify the header with the config file. assuming it is basic authentication first. - if (auth.regionMatches(true, 0, BASIC_PREFIX, 0, BASIC_PREFIX.length())) { - // check if the length is greater than 6 for issue1513 - if(auth.trim().length() == 5) { - LOG.error("Invalid/Unsupported Basic authorization header."); - return new Status(INVALID_AUTHORIZATION_HEADER, BASIC_PREFIX); - } else { - return this.handleBasicAuth(exchange, requestPath, auth); - } - } else if (auth.regionMatches(true, 0, BEARER_PREFIX, 0, BEARER_PREFIX.length())) { + AuthorizationScheme scheme = AuthorizationScheme.parse(auth); + if (scheme == AuthorizationScheme.BASIC) { + return this.handleBasicAuth(exchange, requestPath, auth); + } else if (scheme == AuthorizationScheme.BEARER) { return this.handleBearerToken(exchange, requestPath, auth, config); - } else { - LOG.error("Invalid/Unsupported authorization header."); - return new Status(INVALID_AUTHORIZATION_HEADER, "unknown"); } + LOG.error("Invalid/Unsupported authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, "unknown"); } } @@ -148,6 +139,10 @@ private Status handleAnonymousAuth(LightLambdaExchange exchange, String requestP * @return Status to indicate if an error or success. */ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, String auth) { + if (!AuthorizationScheme.hasCredentials(auth, AuthorizationScheme.BASIC)) { + LOG.error("Invalid/Unsupported Basic authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, BASIC_PREFIX); + } String credentials = auth.substring(6); int pos = credentials.indexOf(':'); if (pos == -1) { @@ -158,31 +153,23 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, String username = credentials.substring(0, pos); String password = credentials.substring(pos + 1); UserAuth user = config.getUsers().get(username); - // if user cannot be found in the config, return immediately. - if (user == null) { - LOG.error("Basic authentication failed: user is not configured."); - if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); - return new Status(INVALID_USERNAME_OR_PASSWORD); - } - if (StringUtils.isEmpty(password)) { - LOG.error("Basic authentication rejected an empty password."); + // Reserved users describe authorization policies, never Basic identities. + if (user == null || StringUtils.isEmpty(password) + || BasicAuthConfig.ANONYMOUS.equals(username) || BasicAuthConfig.BEARER.equals(username)) { + LOG.error("Invalid Basic username or password."); if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(INVALID_USERNAME_OR_PASSWORD); } - // At this point, we know the user is found in the config file. - if (username.equals(user.getUsername()) - && StringUtils.isEmpty(user.getPassword()) - && config.isEnableAD()) { - // Call LdapUtil with LDAP authentication and authorization given user is matched, password is empty, and AD is enabled. + if (StringUtils.isEmpty(user.getPassword()) && config.isEnableAD()) { + // Delegate configured identities without a local password to LDAP. if(LOG.isTraceEnabled()) LOG.trace("Call LdapUtil for Basic authentication."); if (!handleLdapAuth(user, password)) { if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(INVALID_USERNAME_OR_PASSWORD); } } else { - // if username matches config, password matches config, and path matches config, pass - if (!(user.getUsername().equals(username) - && user.getPassword() != null + // Compare the configured local password without early character mismatches. + if (!(user.getPassword() != null && MessageDigest.isEqual(password.getBytes(UTF_8), user.getPassword().getBytes(UTF_8)))) { LOG.error("Invalid Basic username or password."); if (LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); @@ -199,7 +186,8 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, } } if (!match) { - LOG.error("Basic user is not authorized for the requested path."); + LOG.error("Request path '{}' is not authorized for user '{}'", + requestPath.replace('\r', ' ').replace('\n', ' '), user.getUsername()); if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); return new Status(NOT_AUTHORIZED_REQUEST_PATH, requestPath, user.getUsername()); } @@ -216,7 +204,7 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, * @param user * @return true if Ldap auth success, false if Ldap auth failure */ - private static boolean handleLdapAuth(UserAuth user, String password) { + protected boolean handleLdapAuth(UserAuth user, String password) { boolean isAuthenticated = LdapUtil.authenticate(user.getUsername(), password); if (!isAuthenticated) { LOG.error("user '" + user.getUsername() + "' Ldap authentication failed"); diff --git a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/UnifiedSecurityMiddleware.java b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/UnifiedSecurityMiddleware.java index 667a82c..38d0e2f 100644 --- a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/UnifiedSecurityMiddleware.java +++ b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/UnifiedSecurityMiddleware.java @@ -88,13 +88,14 @@ public Status execute(LightLambdaExchange exchange) { return new Status(MISSING_AUTH_TOKEN); } else { String authorization = optionalAuth.get(); - // make sure that the length is greater than 5. - if (authorization.trim().length() <= 5) { - LOG.error("Invalid/Unsupported authorization header {}", authorization); - return new Status(INVALID_AUTHORIZATION_HEADER, authorization); + AuthorizationScheme scheme = AuthorizationScheme.parse(authorization); + if (!AuthorizationScheme.hasCredentials(authorization, scheme) + || scheme == AuthorizationScheme.UNKNOWN) { + LOG.error("Invalid/Unsupported authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, + scheme == AuthorizationScheme.UNKNOWN ? "unknown" : scheme.name()); } - // check if it is basic or bearer and handler it differently. - if (BASIC_PREFIX.equalsIgnoreCase(authorization.substring(0, 5))) { + if (scheme == AuthorizationScheme.BASIC) { Map handlers = Handler.getHandlers(); BasicAuthMiddleware handler = (BasicAuthMiddleware) handlers.get(BASIC_PREFIX.toLowerCase()); if (handler == null) { @@ -107,7 +108,7 @@ public Status execute(LightLambdaExchange exchange) { } return handler.handleBasicAuth(exchange, reqPath, authorization); } - } else if (BEARER_PREFIX.equalsIgnoreCase(authorization.substring(0, 6))) { + } else if (scheme == AuthorizationScheme.BEARER) { // in the case that a bearer token is used, there are three scenarios: both jwt and swt are true, only jwt is true and only swt is true // in the first case, we need to identify if the token is jwt or swt before calling the right handler to verify it. Map handlers = Handler.getHandlers(); @@ -173,9 +174,8 @@ public Status execute(LightLambdaExchange exchange) { } } } else { - String s = authorization.length() > 10 ? authorization.substring(0, 10) : authorization; - LOG.error("Invalid/Unsupported authorization header {}", s); - return new Status(INVALID_AUTHORIZATION_HEADER, s); + LOG.error("Invalid/Unsupported authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, "unknown"); } } } else if (pathPrefixAuth.isApikey()) { diff --git a/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java b/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java index 7ba555a..c13a492 100644 --- a/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java +++ b/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java @@ -11,12 +11,18 @@ import com.networknt.aws.lambda.handler.middleware.specification.OpenApiMiddleware; import com.networknt.aws.lambda.utility.HeaderKey; import com.networknt.basicauth.BasicAuthConfig; +import com.networknt.basicauth.UserAuth; +import com.networknt.aws.lambda.handler.middleware.security.UnifiedSecurityMiddleware; import com.networknt.status.Status; import com.networknt.utility.Constants; import com.networknt.utility.MapUtil; import org.apache.commons.codec.binary.Base64; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import org.slf4j.LoggerFactory; import java.util.Map; @@ -57,7 +63,7 @@ public void testEmptyConfiguredPasswordIsRejected() { @Test public void testShortMalformedAuthorizationHeadersAreRejected() { - for (String header : new String[] {"x", "Basic eA"}) { + for (String header : new String[] {"x", "Basic eA", "Bearer", "Bearer x", "BasicSecret", "Basic"}) { var requestEvent = TestUtils.createTestRequestEvent(); requestEvent.setPath("/v2/pet"); requestEvent.getHeaders().put(HeaderKey.AUTHORIZATION, header); @@ -65,9 +71,107 @@ public void testShortMalformedAuthorizationHeadersAreRejected() { exchange.setInitialRequest(requestEvent); Status status = new BasicAuthMiddleware("basic-auth").execute(exchange); Assertions.assertEquals(401, status.getStatusCode(), header); + Assertions.assertEquals(header.equals("Basic eA") || header.startsWith("Bearer") + ? "ERR10046" : "ERR12003", status.getCode(), header); } } + private static LightLambdaExchange exchangeFor(String header) { + var request = TestUtils.createTestRequestEvent(); + request.setPath("/v2/pet"); + request.getHeaders().put(HeaderKey.AUTHORIZATION, header); + var exchange = new LightLambdaExchange(new LambdaContext("security-review"), null); + exchange.setInitialRequest(request); + return exchange; + } + + private static class RecordingLdapMiddleware extends BasicAuthMiddleware { + int ldapCalls; + + RecordingLdapMiddleware() { + super("basic-auth-ldap"); + } + + @Override + protected boolean handleLdapAuth(UserAuth user, String password) { + ldapCalls++; + return true; + } + } + + @Test + public void testLdapRejectsEmptyPasswordsAndReservedUsersBeforeBind() { + Assertions.assertTrue(BasicAuthConfig.load("basic-auth-ldap").isEnableAD()); + var middleware = new RecordingLdapMiddleware(); + for (String[] credentials : new String[][] { + {"ldapUser", ""}, {"anonymous", "password"}, {"bearer", "password"}}) { + Status status = middleware.execute(exchangeFor("Basic " + encodeCredentials(credentials[0], credentials[1]))); + Assertions.assertEquals("ERR10047", status.getCode()); + Assertions.assertEquals(401, status.getStatusCode()); + } + Assertions.assertEquals(0, middleware.ldapCalls); + // Positive control proves this configuration actually dispatches ordinary identities to LDAP. + Assertions.assertEquals(200, middleware.execute(exchangeFor("Basic " + encodeCredentials("ldapUser", "password"))).getStatusCode()); + Assertions.assertEquals(1, middleware.ldapCalls); + } + + @Test + public void testPublicBasicEntryPointRejectsMalformedHeaders() { + var middleware = new BasicAuthMiddleware("basic-auth"); + for (String header : new String[] {null, "", "x", "Basic", "Basic ", "BasicSecret", "Bearer x"}) { + Status status = middleware.handleBasicAuth(exchangeFor("ignored"), "/v2/pet", header); + Assertions.assertEquals("ERR12003", status.getCode()); + Assertions.assertEquals(401, status.getStatusCode()); + } + } + + @Test + public void testUnifiedRejectsMalformedHeadersWithoutEchoingCredentials() { + var middleware = new UnifiedSecurityMiddleware("unified-security-review"); + for (String header : new String[] {"x", "abcde", "Secret credential-material", "Basic", "Basic ", "Bearer", "Bearer ", "BasicSecret"}) { + Status status = middleware.execute(exchangeFor(header)); + Assertions.assertEquals("ERR12003", status.getCode(), header); + Assertions.assertEquals(401, status.getStatusCode(), header); + Assertions.assertFalse(status.toString().contains(header), header); + } + } + + @Test + public void testAuthorizationLogsDoNotEchoHeadersAndSanitizeDeniedPath() { + Logger basicLogger = (Logger) LoggerFactory.getLogger(BasicAuthMiddleware.class); + Logger unifiedLogger = (Logger) LoggerFactory.getLogger(UnifiedSecurityMiddleware.class); + var appender = new ListAppender(); + appender.start(); + basicLogger.addAppender(appender); + unifiedLogger.addAppender(appender); + try { + var basic = new BasicAuthMiddleware("basic-auth"); + var unified = new UnifiedSecurityMiddleware("unified-security-review"); + for (String header : new String[] {"abcde", "Secret credential-material"}) { + basic.execute(exchangeFor(header)); + unified.execute(exchangeFor(header)); + Assertions.assertTrue(appender.list.stream().noneMatch(event -> event.getFormattedMessage().contains(header))); + } + var exchange = exchangeFor("Basic " + encodeCredentials("user1", "user1pass")); + exchange.getRequest().setPath("/denied\r\nforged-line"); + Status status = basic.execute(exchange); + Assertions.assertEquals("ERR10071", status.getCode()); + Assertions.assertTrue(appender.list.stream().anyMatch(event -> event.getFormattedMessage() + .equals("Request path '/denied forged-line' is not authorized for user 'user1'"))); + } finally { + basicLogger.detachAppender(appender); + unifiedLogger.detachAppender(appender); + appender.stop(); + } + } + + @Test + public void testMixedCaseBasicSchemeAndColonInPasswordParsing() { + var basic = new BasicAuthMiddleware("basic-auth"); + Assertions.assertEquals(200, basic.execute(exchangeFor("bAsIc " + encodeCredentials("user2", "password"))).getStatusCode()); + Assertions.assertEquals("ERR10047", basic.execute(exchangeFor("Basic " + encodeCredentials("user2", "password:extra"))).getCode()); + } + /** * Test with right credentials but incorrect path. Expect 401 status code. */ diff --git a/src/test/resources/config/basic-auth-ldap.yml b/src/test/resources/config/basic-auth-ldap.yml index e545859..3c44fd6 100644 --- a/src/test/resources/config/basic-auth-ldap.yml +++ b/src/test/resources/config/basic-auth-ldap.yml @@ -30,3 +30,7 @@ users: paths: - /v1/party - /info + + - username: bearer + paths: + - /v2/pet diff --git a/src/test/resources/config/unified-security-review.yml b/src/test/resources/config/unified-security-review.yml new file mode 100644 index 0000000..37cb9ea --- /dev/null +++ b/src/test/resources/config/unified-security-review.yml @@ -0,0 +1,5 @@ +enabled: true +pathPrefixAuths: + - prefix: /v2 + basic: true + jwt: true From 657b9442935b0cb7d8d1cc59605b7dd75e00fb03 Mon Sep 17 00:00:00 2001 From: Steve Hu Date: Fri, 2 Oct 2026 15:26:56 -0400 Subject: [PATCH 3/3] fixes #185 address copilot findings --- .../middleware/security/BasicAuthMiddleware.java | 11 ++++++++--- .../security/BasicAuthMiddlewareTest.java | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java index 537d384..949fb66 100644 --- a/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java +++ b/src/main/java/com/networknt/aws/lambda/handler/middleware/security/BasicAuthMiddleware.java @@ -67,6 +67,11 @@ public Status execute(LightLambdaExchange exchange) { return this.handleAnonymousAuth(exchange, requestPath, config); } AuthorizationScheme scheme = AuthorizationScheme.parse(auth); + // A bare scheme must not reach handleBearerToken, which only checks paths. + if (scheme != AuthorizationScheme.UNKNOWN && !AuthorizationScheme.hasCredentials(auth, scheme)) { + LOG.error("Invalid/Unsupported authorization header."); + return new Status(INVALID_AUTHORIZATION_HEADER, scheme.name()); + } if (scheme == AuthorizationScheme.BASIC) { return this.handleBasicAuth(exchange, requestPath, auth); } else if (scheme == AuthorizationScheme.BEARER) { @@ -186,10 +191,10 @@ public Status handleBasicAuth(LightLambdaExchange exchange, String requestPath, } } if (!match) { - LOG.error("Request path '{}' is not authorized for user '{}'", - requestPath.replace('\r', ' ').replace('\n', ' '), user.getUsername()); + String safePath = requestPath.replace('\r', ' ').replace('\n', ' '); + LOG.error("Request path '{}' is not authorized for user '{}'", safePath, user.getUsername()); if(LOG.isDebugEnabled()) LOG.debug("BasicAuthMiddleware.execute ends with an error."); - return new Status(NOT_AUTHORIZED_REQUEST_PATH, requestPath, user.getUsername()); + return new Status(NOT_AUTHORIZED_REQUEST_PATH, safePath, user.getUsername()); } } else { LOG.error("Invalid basic authentication header. It must be username:password base64 encode."); diff --git a/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java b/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java index c13a492..ae7046f 100644 --- a/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java +++ b/src/test/java/com/networknt/aws/lambda/middleware/security/BasicAuthMiddlewareTest.java @@ -71,11 +71,22 @@ public void testShortMalformedAuthorizationHeadersAreRejected() { exchange.setInitialRequest(requestEvent); Status status = new BasicAuthMiddleware("basic-auth").execute(exchange); Assertions.assertEquals(401, status.getStatusCode(), header); - Assertions.assertEquals(header.equals("Basic eA") || header.startsWith("Bearer") + Assertions.assertEquals(header.equals("Basic eA") || header.equals("Bearer x") ? "ERR10046" : "ERR12003", status.getCode(), header); } } + @Test + public void testBareBearerRejectedWhenBearerTokensAllowed() { + var middleware = new BasicAuthMiddleware("basic-auth-bearer"); + Assertions.assertTrue(BasicAuthConfig.load("basic-auth-bearer").isAllowBearerToken()); + for (String header : new String[] {"Bearer", "Bearer ", "bearer "}) { + Status status = middleware.execute(exchangeFor(header)); + Assertions.assertEquals(401, status.getStatusCode(), header); + Assertions.assertEquals("ERR12003", status.getCode(), header); + } + } + private static LightLambdaExchange exchangeFor(String header) { var request = TestUtils.createTestRequestEvent(); request.setPath("/v2/pet"); @@ -156,6 +167,8 @@ public void testAuthorizationLogsDoNotEchoHeadersAndSanitizeDeniedPath() { exchange.getRequest().setPath("/denied\r\nforged-line"); Status status = basic.execute(exchange); Assertions.assertEquals("ERR10071", status.getCode()); + Assertions.assertFalse(status.toString().contains("\r") || status.toString().contains("\n")); + Assertions.assertTrue(status.toString().contains("/denied forged-line")); Assertions.assertTrue(appender.list.stream().anyMatch(event -> event.getFormattedMessage() .equals("Request path '/denied forged-line' is not authorized for user 'user1'"))); } finally {