Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[microservice] Add auth-mode ldap to enable authentication based on LDAP #35

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
*/
package de.muenchen.oss.ezldap.spring;

import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.ldap.core.LdapTemplate;
Expand All @@ -44,7 +46,8 @@
public class LdapConfiguration {

@Bean
LdapContextSource ldapContextSource(final EzLdapConfigurationProperties props) {
@ConditionalOnMissingBean(name = "ezldapQueryContextSource")
LdapContextSource ezldapQueryContextSource(final EzLdapConfigurationProperties props) {
final LdapContextSource contextSource = new LdapContextSource();
contextSource.setUrl(props.getLdap().getUrl());
contextSource.setUserDn(props.getLdap().getUserDn());
Expand All @@ -54,16 +57,17 @@ LdapContextSource ldapContextSource(final EzLdapConfigurationProperties props) {
return contextSource;
}

@Bean
LdapTemplate ldapTemplate(final LdapContextSource ldapContextSource) {
@Bean("ezldapLdapTemplate")
LdapTemplate ezldapLdapTemplate(@Qualifier("ezldapQueryContextSource") final LdapContextSource ldapContextSource) {
return new LdapTemplate(ldapContextSource);
}

@Bean
LdapService ldapService(final LdapTemplate template, final EzLdapConfigurationProperties props) {
LdapService ldapService(@Qualifier("ezldapLdapTemplate") final LdapTemplate template, final EzLdapConfigurationProperties props) {
final LdapBaseUserAttributesMapper ldapBaseUserAttributesMapper = new LdapBaseUserAttributesMapper();
final LdapOuAttributesMapper ldapOuAttributesMapper = new LdapOuAttributesMapper();
final LdapUserAttributesMapper ldapUserAttributesMapper = new LdapUserAttributesMapper(ldapBaseUserAttributesMapper);
final LdapUserAttributesMapper ldapUserAttributesMapper = new LdapUserAttributesMapper(
ldapBaseUserAttributesMapper);
return new LdapService(template, ldapUserAttributesMapper, ldapBaseUserAttributesMapper, ldapOuAttributesMapper,
new DtoMapperImpl(), props.getLdap().getUserSearchBase(), props.getLdap().getOuSearchBase());
}
Expand Down
14 changes: 13 additions & 1 deletion microservice/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
THE SOFTWARE.

-->
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<parent>
Expand Down Expand Up @@ -61,6 +63,11 @@
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-security</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.security</groupId>
<artifactId>spring-security-ldap</artifactId>
</dependency>


<!-- Testing -->
<dependency>
Expand All @@ -73,6 +80,11 @@
<artifactId>spring-boot-starter-test</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-testcontainers</artifactId>
<scope>test</scope>
</dependency>


<!-- Logging -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public class AppConfigurationProperties {

public enum AuthMode {
NONE,
LDAP,
BASIC;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,33 @@
*/
package de.muenchen.oss.ezldap.config;

import java.util.Collections;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.ldap.core.AuthenticationSource;
import org.springframework.ldap.core.support.LdapContextSource;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.authentication.AuthenticationProvider;
import org.springframework.security.authentication.ProviderManager;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.config.Customizer;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.http.SessionCreationPolicy;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.userdetails.User;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.provisioning.InMemoryUserDetailsManager;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.authentication.AnonymousAuthenticationFilter;

import de.muenchen.oss.ezldap.config.AppConfigurationProperties.AuthMode;
import de.muenchen.oss.ezldap.security.BasicAuthPassthroughFilter;
import de.muenchen.oss.ezldap.spring.props.EzLdapConfigurationProperties;
import lombok.extern.slf4j.Slf4j;

Expand All @@ -49,32 +63,88 @@
public class SecurityConfiguration {

@Bean
SecurityFilterChain securityFilterChain(HttpSecurity http, EzLdapConfigurationProperties configProps, AppConfigurationProperties appProps)
throws Exception {
SecurityFilterChain securityFilterChain(HttpSecurity http, EzLdapConfigurationProperties configProps,
AppConfigurationProperties appProps, @Autowired(required = false) AuthenticationManager authenticationManager) throws Exception {
if (AuthMode.NONE.equals(appProps.getAuthMode())) {
log.info("Bootstrapping Spring Security filter chain for auth-mode 'none' ...");
http
.authorizeHttpRequests(authz -> authz.anyRequest().permitAll());
http.sessionManagement(sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
http.authorizeHttpRequests(authz -> authz.anyRequest().permitAll());
http.sessionManagement(
sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
} else if (AuthMode.LDAP.equals(appProps.getAuthMode())) {
log.info("Bootstrapping Spring Security filter chain for auth-mode 'ldap' ...");
configureMatchers(http);
http.sessionManagement(
sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
http.addFilterBefore(new BasicAuthPassthroughFilter(authenticationManager, "/v1/**"), AnonymousAuthenticationFilter.class);
Comment on lines +66 to +78
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure AuthenticationManager is properly injected for LDAP authentication

In the securityFilterChain method, during LDAP authentication mode, the authenticationManager is essential for adding the BasicAuthPassthroughFilter. However, it is autowired with required = false, which may result in a null value if no AuthenticationManager bean is available. This could lead to a NullPointerException at runtime.

Suggested Improvement:

  • Remove required = false to ensure AuthenticationManager is always injected when LDAP authentication is enabled.
  • Ensure that an AuthenticationManager bean is defined and available in the application context when app.auth-mode is set to ldap.

Apply this change to enforce the presence of AuthenticationManager:

 SecurityFilterChain securityFilterChain(HttpSecurity http, EzLdapConfigurationProperties configProps,
-        AppConfigurationProperties appProps, @Autowired(required = false) AuthenticationManager authenticationManager) throws Exception {
+        AppConfigurationProperties appProps, AuthenticationManager authenticationManager) throws Exception {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
SecurityFilterChain securityFilterChain(HttpSecurity http, EzLdapConfigurationProperties configProps,
AppConfigurationProperties appProps, @Autowired(required = false) AuthenticationManager authenticationManager) throws Exception {
if (AuthMode.NONE.equals(appProps.getAuthMode())) {
log.info("Bootstrapping Spring Security filter chain for auth-mode 'none' ...");
http
.authorizeHttpRequests(authz -> authz.anyRequest().permitAll());
http.sessionManagement(sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
http.authorizeHttpRequests(authz -> authz.anyRequest().permitAll());
http.sessionManagement(
sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
} else if (AuthMode.LDAP.equals(appProps.getAuthMode())) {
log.info("Bootstrapping Spring Security filter chain for auth-mode 'ldap' ...");
configureMatchers(http);
http.sessionManagement(
sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
http.addFilterBefore(new BasicAuthPassthroughFilter(authenticationManager, "/v1/**"), AnonymousAuthenticationFilter.class);
SecurityFilterChain securityFilterChain(HttpSecurity http, EzLdapConfigurationProperties configProps,
AppConfigurationProperties appProps, AuthenticationManager authenticationManager) throws Exception {
if (AuthMode.NONE.equals(appProps.getAuthMode())) {
log.info("Bootstrapping Spring Security filter chain for auth-mode 'none' ...");
http.authorizeHttpRequests(authz -> authz.anyRequest().permitAll());
http.sessionManagement(
sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
} else if (AuthMode.LDAP.equals(appProps.getAuthMode())) {
log.info("Bootstrapping Spring Security filter chain for auth-mode 'ldap' ...");
configureMatchers(http);
http.sessionManagement(
sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
http.addFilterBefore(new BasicAuthPassthroughFilter(authenticationManager, "/v1/**"), AnonymousAuthenticationFilter.class);

} else {
log.info("Bootstrapping Spring Security filter chain for auth-mode 'basic' ...");
http
.authorizeHttpRequests(authz -> {
authz.requestMatchers("/openapi/v3/**", "/swagger-ui/**").permitAll();
authz.requestMatchers("/actuator/prometheus", "/actuator/info", "/actuator/health/**").permitAll();
authz.anyRequest().authenticated();
});
http.sessionManagement(sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
configureMatchers(http);
http.sessionManagement(
sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
http.httpBasic(Customizer.withDefaults());
}
return http.build();
}

private void configureMatchers(HttpSecurity http) throws Exception {
http.authorizeHttpRequests(authz -> {
authz.requestMatchers("/openapi/v3/**", "/swagger-ui/**").permitAll();
authz.requestMatchers("/actuator/prometheus", "/actuator/info", "/actuator/health/**").permitAll();
authz.anyRequest().authenticated();
});
}

@Bean
@ConditionalOnProperty(name = "app.auth-mode", havingValue = "basic")
InMemoryUserDetailsManager userDetailsService(AppConfigurationProperties appProps) {
UserDetails userDetails = User.withUsername(appProps.getBasicAuth().getUser()).password(appProps.getBasicAuth().getPassword()).roles("USER").build();
UserDetails userDetails = User.withUsername(appProps.getBasicAuth().getUser())
.password(appProps.getBasicAuth().getPassword()).roles("USER").build();
return new InMemoryUserDetailsManager(userDetails);
}

@Bean
@ConditionalOnProperty(name = "app.auth-mode", havingValue = "ldap")
AuthenticationManager authenticationManager() {
ProviderManager providerManager = new ProviderManager(Collections.singletonList(new AuthenticationProvider() {

@Override
public boolean supports(Class<?> authentication) {
return UsernamePasswordAuthenticationToken.class.isAssignableFrom(authentication);
}

@Override
public Authentication authenticate(Authentication authentication) throws AuthenticationException {
UsernamePasswordAuthenticationToken token = (UsernamePasswordAuthenticationToken) authentication;
return new UsernamePasswordAuthenticationToken(token.getPrincipal(), token.getCredentials(), Collections.emptyList());
}
}));
providerManager.setEraseCredentialsAfterAuthentication(false);
return providerManager;
}
Comment on lines +105 to +123
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security Vulnerability: Authentication provider does not validate credentials

The AuthenticationProvider in the authenticationManager() method returns an authenticated UsernamePasswordAuthenticationToken without verifying the user's credentials. This means any username and password combination will be accepted, posing a critical security risk.

Proposed Fix: Implement LDAP authentication using Spring Security

Utilize Spring Security's LDAP authentication mechanisms to validate user credentials against your LDAP server. Here's how you can modify the authenticationManager() method:

 @Bean
 @ConditionalOnProperty(name = "app.auth-mode", havingValue = "ldap")
-AuthenticationManager authenticationManager() {
-    ProviderManager providerManager = new ProviderManager(Collections.singletonList(new AuthenticationProvider() {
-
-        @Override
-        public boolean supports(Class<?> authentication) {
-            return UsernamePasswordAuthenticationToken.class.isAssignableFrom(authentication);
-        }
-
-        @Override
-        public Authentication authenticate(Authentication authentication) throws AuthenticationException {
-            UsernamePasswordAuthenticationToken token = (UsernamePasswordAuthenticationToken) authentication;
-            return new UsernamePasswordAuthenticationToken(token.getPrincipal(), token.getCredentials(), Collections.emptyList());
-        }
-    }));
-    providerManager.setEraseCredentialsAfterAuthentication(false);
-    return providerManager;
+AuthenticationManager authenticationManager(LdapContextSource contextSource) {
+    BindAuthenticator bindAuthenticator = new BindAuthenticator(contextSource);
+    // Configure search settings if necessary
+    LdapAuthenticationProvider ldapAuthenticationProvider = new LdapAuthenticationProvider(bindAuthenticator);
+    ProviderManager providerManager = new ProviderManager(Collections.singletonList(ldapAuthenticationProvider));
+    return providerManager;
 }
  • Explanation:
    • BindAuthenticator: Authenticates users by binding to the LDAP directory with the provided credentials.
    • LdapAuthenticationProvider: Processes authentication requests using the BindAuthenticator.
    • Configure additional settings like user DN patterns or search filters if required.

Committable suggestion skipped: line range outside the PR's diff.


@Bean(name = "ezldapQueryContextSource")
@ConditionalOnProperty(name = "app.auth-mode", havingValue = "ldap")
LdapContextSource ezldapQueryContextSource(final EzLdapConfigurationProperties props) {
final LdapContextSource contextSource = new LdapContextSource();
contextSource.setUrl(props.getLdap().getUrl());
contextSource.setAuthenticationSource(new AuthenticationSource() {

@Override
public String getPrincipal() {
UsernamePasswordAuthenticationToken auth = (UsernamePasswordAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
return auth.getPrincipal().toString();
}

@Override
public String getCredentials() {
UsernamePasswordAuthenticationToken auth = (UsernamePasswordAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
return auth.getCredentials().toString();
}
});
log.info(
"Initiating LDAP context-source with url='{}' and SpringSecurityAuthenticationSource for Web LDAP authentication credentials passthrough to LDAP queries.",
props.getLdap().getUrl());
return contextSource;
}
Comment on lines +125 to +148
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Handle potential NullPointerException in AuthenticationSource

The custom AuthenticationSource in the ezldapQueryContextSource bean retrieves the authentication from the SecurityContextHolder. If the security context does not contain an authentication (e.g., if the user is not authenticated), calling getPrincipal() or getCredentials() will result in a NullPointerException.

Suggested Improvement:

Add null checks and type checks to safely handle missing or invalid authentication:

 contextSource.setAuthenticationSource(new AuthenticationSource() {

     @Override
     public String getPrincipal() {
-        UsernamePasswordAuthenticationToken auth = (UsernamePasswordAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
-        return auth.getPrincipal().toString();
+        Authentication auth = SecurityContextHolder.getContext().getAuthentication();
+        if (auth instanceof UsernamePasswordAuthenticationToken) {
+            Object principal = auth.getPrincipal();
+            return principal != null ? principal.toString() : null;
+        } else {
+            log.warn("No authentication principal found in SecurityContext");
+            return null;
+        }
     }

     @Override
     public String getCredentials() {
-        UsernamePasswordAuthenticationToken auth = (UsernamePasswordAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
-        return auth.getCredentials().toString();
+        Authentication auth = SecurityContextHolder.getContext().getAuthentication();
+        if (auth instanceof UsernamePasswordAuthenticationToken) {
+            Object credentials = auth.getCredentials();
+            return credentials != null ? credentials.toString() : null;
+        } else {
+            log.warn("No authentication credentials found in SecurityContext");
+            return null;
+        }
     }
 });
  • Explanation:
    • Checks if auth is an instance of UsernamePasswordAuthenticationToken.
    • Logs a warning if authentication is missing or of an unexpected type.
    • Prevents NullPointerException by handling null values gracefully.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Bean(name = "ezldapQueryContextSource")
@ConditionalOnProperty(name = "app.auth-mode", havingValue = "ldap")
LdapContextSource ezldapQueryContextSource(final EzLdapConfigurationProperties props) {
final LdapContextSource contextSource = new LdapContextSource();
contextSource.setUrl(props.getLdap().getUrl());
contextSource.setAuthenticationSource(new AuthenticationSource() {
@Override
public String getPrincipal() {
UsernamePasswordAuthenticationToken auth = (UsernamePasswordAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
return auth.getPrincipal().toString();
}
@Override
public String getCredentials() {
UsernamePasswordAuthenticationToken auth = (UsernamePasswordAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
return auth.getCredentials().toString();
}
});
log.info(
"Initiating LDAP context-source with url='{}' and SpringSecurityAuthenticationSource for Web LDAP authentication credentials passthrough to LDAP queries.",
props.getLdap().getUrl());
return contextSource;
}
@Bean(name = "ezldapQueryContextSource")
@ConditionalOnProperty(name = "app.auth-mode", havingValue = "ldap")
LdapContextSource ezldapQueryContextSource(final EzLdapConfigurationProperties props) {
final LdapContextSource contextSource = new LdapContextSource();
contextSource.setUrl(props.getLdap().getUrl());
contextSource.setAuthenticationSource(new AuthenticationSource() {
@Override
public String getPrincipal() {
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
if (auth instanceof UsernamePasswordAuthenticationToken) {
Object principal = auth.getPrincipal();
return principal != null ? principal.toString() : null;
} else {
log.warn("No authentication principal found in SecurityContext");
return null;
}
}
@Override
public String getCredentials() {
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
if (auth instanceof UsernamePasswordAuthenticationToken) {
Object credentials = auth.getCredentials();
return credentials != null ? credentials.toString() : null;
} else {
log.warn("No authentication credentials found in SecurityContext");
return null;
}
}
});
log.info(
"Initiating LDAP context-source with url='{}' and SpringSecurityAuthenticationSource for Web LDAP authentication credentials passthrough to LDAP queries.",
props.getLdap().getUrl());
return contextSource;
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,8 @@ public class SpringdocsSwaggerConfig {

@Bean
GroupedOpenApi publicApi(OpenApiCustomizer globalResponseCodeCustomiser) {
return GroupedOpenApi.builder()
.group("ezLDAP")
.packagesToScan("de.muenchen.oss.ezldap.spring.rest.v1")
.addOpenApiCustomizer(globalResponseCodeCustomiser)
.build();
return GroupedOpenApi.builder().group("ezLDAP").packagesToScan("de.muenchen.oss.ezldap.spring.rest.v1")
.addOpenApiCustomizer(globalResponseCodeCustomiser).build();
}

@Bean
Expand All @@ -73,10 +70,13 @@ OpenApiCustomizer globalResponseCodeCustomiser(AppConfigurationProperties appPro

if (AuthMode.BASIC.equals(appProps.getAuthMode())) {
openApi.addSecurityItem(new SecurityRequirement().addList("basicAuth"))
.components(new Components()
.addSecuritySchemes("basicAuth", new SecurityScheme()
.type(SecurityScheme.Type.HTTP)
.scheme("basic")));
.components(new Components().addSecuritySchemes("basicAuth",
new SecurityScheme().type(SecurityScheme.Type.HTTP).scheme("basic")));
}
if (AuthMode.LDAP.equals(appProps.getAuthMode())) {
openApi.addSecurityItem(new SecurityRequirement().addList("basicAuthLdap"))
.components(new Components().addSecuritySchemes("basicAuthLdap",
new SecurityScheme().type(SecurityScheme.Type.HTTP).scheme("basic").description("Authenticate using LDAP")));
Comment on lines +73 to +79
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Add handling for NONE and default auth modes in security configuration

The verification confirms that AuthMode enum has three values: NONE, LDAP, and BASIC, but the SpringdocsSwaggerConfig only handles BASIC and LDAP explicitly. The NONE mode and potential unknown modes (through enum extension) are not handled. Add explicit handling:

  • microservice/src/main/java/de/muenchen/oss/ezldap/config/SpringdocsSwaggerConfig.java: Add an else branch after the BASIC and LDAP checks to handle NONE and any future auth modes, ensuring security configuration is appropriately set or explicitly disabled.
🔗 Analysis chain

Consider handling default/unknown auth modes

The current implementation only handles BASIC and LDAP modes explicitly. Consider adding a default case to maintain security when an unexpected auth mode is configured.

Let's verify all possible auth modes:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all possible AuthMode enum values to ensure complete coverage

# Search for AuthMode enum definition
ast-grep --pattern 'enum AuthMode {
  $$$
}'

Length of output: 46


Script:

#!/bin/bash
# Let's try a different approach to find the AuthMode enum using ripgrep
rg "enum.*AuthMode" -A 5

Length of output: 637


Script:

#!/bin/bash
# Let's check how AuthMode is handled in SpringdocsSwaggerConfig
rg "AuthMode" microservice/src/main/java/de/muenchen/oss/ezldap/config/SpringdocsSwaggerConfig.java -B 2 -A 2

Length of output: 951

}
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* The MIT License
* Copyright © 2024 Landeshauptstadt München | it@M
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package de.muenchen.oss.ezldap.security;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Base64;

import javax.naming.InvalidNameException;
import javax.naming.ldap.LdapName;

import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter;

import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

/**
* @author michael.prankl
*/
public class BasicAuthPassthroughFilter extends AbstractAuthenticationProcessingFilter {

public BasicAuthPassthroughFilter(AuthenticationManager authenticationManager, String filter) {
super(filter);
this.setAuthenticationManager(authenticationManager);
}

@Override
public Authentication attemptAuthentication(HttpServletRequest request, HttpServletResponse response) {

String header = request.getHeader("Authorization");

if (header != null && header.startsWith("Basic ")) {
String[] tokens = extractAndDecodeHeader(header);
String username = tokens[0];
// Validate username as a valid DN
try {
new LdapName(username);
} catch (InvalidNameException e) {
throw new BadCredentialsException("Invalid username format: " + e.getMessage(), e);
}
Comment on lines +63 to +67
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid exposing internal error details in exceptions

Throwing a BadCredentialsException with the underlying exception message may reveal sensitive information. It's safer to provide a generic error message to the client and log the detailed exception on the server side.

Apply this diff to modify the exception handling:

try {
    new LdapName(username);
} catch (InvalidNameException e) {
-    throw new BadCredentialsException("Invalid username format: " + e.getMessage(), e);
+    logger.warn("Invalid username format for user DN", e);
+    throw new BadCredentialsException("Invalid credentials");
}

Committable suggestion skipped: line range outside the PR's diff.

String password = tokens[1];

UsernamePasswordAuthenticationToken authRequest = new UsernamePasswordAuthenticationToken(username, password);
return getAuthenticationManager().authenticate(authRequest);
} else {
throw new BadCredentialsException("Missing basic auth header");
}
}
Comment on lines +55 to +75
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle missing or invalid Authorization header appropriately

Throwing a BadCredentialsException when the Authorization header is missing may not be the best approach. Instead, consider initiating the authentication entry point to prompt for credentials without exposing the internal exception.

Apply this diff to handle the missing header:

-} else {
-    throw new BadCredentialsException("Missing basic auth header");
-}
+} else {
+    authenticationEntryPoint.commence(request, response, new InsufficientAuthenticationException("Authorization header missing"));
+    return null;
+}

Ensure that an AuthenticationEntryPoint is configured to handle unauthorized access responses appropriately.

Committable suggestion skipped: line range outside the PR's diff.


@Override
protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain,
Authentication authResult) throws IOException, ServletException {

SecurityContextHolder.getContext().setAuthentication(authResult);
chain.doFilter(request, response);
}
Comment on lines +78 to +83
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure the security context is cleared upon authentication failure

If an exception occurs during chain.doFilter(request, response);, the security context might retain the previous authentication. Consider adding a finally block to clear the security context to prevent potential security issues.

Apply this diff to clear the security context on failure:

@Override
protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain,
        Authentication authResult) throws IOException, ServletException {

    SecurityContextHolder.getContext().setAuthentication(authResult);
    try {
        chain.doFilter(request, response);
    } finally {
        SecurityContextHolder.clearContext();
    }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain,
Authentication authResult) throws IOException, ServletException {
SecurityContextHolder.getContext().setAuthentication(authResult);
chain.doFilter(request, response);
}
protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain,
Authentication authResult) throws IOException, ServletException {
SecurityContextHolder.getContext().setAuthentication(authResult);
try {
chain.doFilter(request, response);
} finally {
SecurityContextHolder.clearContext();
}
}


private String[] extractAndDecodeHeader(String header) {
byte[] base64Token = header.substring(6).getBytes(StandardCharsets.UTF_8);
byte[] decoded;
try {
decoded = Base64.getDecoder().decode(base64Token);
} catch (IllegalArgumentException e) {
throw new BadCredentialsException("Failed to decode basic authentication token");
}
String token = new String(decoded, StandardCharsets.UTF_8);
int delim = token.indexOf(":");
if (delim == -1) {
throw new BadCredentialsException("Invalid basic authentication token");
}
return new String[] { token.substring(0, delim), token.substring(delim + 1) };
}

}
Loading
Loading